Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Installing Flux gives certain SQL errors #168

Closed
Vennren opened this issue Mar 31, 2018 · 2 comments · Fixed by #388
Closed

Installing Flux gives certain SQL errors #168

Vennren opened this issue Mar 31, 2018 · 2 comments · Fixed by #388

Comments

@Vennren
Copy link

Vennren commented Mar 31, 2018

Hello!

While installing Flux various errors show up along the way.
These updates to the tables are cause of the issue, for:
cp_itemshop.20081109093448.sql

ALTER TABLE cp_itemshop ADD use_existing TINYINT NOT NULL DEFAULT '0' AFTER info

Add the missing semi-colon to fix.

ALTER TABLE cp_itemshop ADD use_existing TINYINT NOT NULL DEFAULT '0' AFTER info;

cp_itemshop.20090104190020.sql

ALTER TABLE cp_itemshop ADD category INT(11) NULL AFTER nameid

Add the missing semi-colon to fix.

ALTER TABLE cp_itemshop ADD category INT(11) NULL AFTER nameid;

ALTER TABLE cp_redeemlog ADD credits_before INT( 10 ) NOT NULL , ADD credits_after INT( 10 )
NOT NULL

Should probably be the missing semi-colon to fix as below, however I ended up with dropping/readding the columns for my fresh install.

ALTER TABLE cp_redeemlog ADD credits_before INT( 10 ) NOT NULL , ADD credits_after INT( 10 )
NOT NULL

My solution for my installation:

ALTER TABLE cp_redeemlog DROP COLUMN credits_before , DROP COLUMN credits_after;
ALTER TABLE cp_redeemlog ADD credits_before INT( 10 ) NOT NULL , ADD credits_after INT( 10 ) NOT NULL;

With these modifications in place I was able to install everything. Perhaps newly updated tables can be added to the schemas for the creation of the cp_tables along with a newer date for these files so that the old sql files won't be re-iterated through.

Hopefully this might be of some use.

Kind regards,

Tranquility

@Daegaladh
Copy link
Member

Drop colum for reinstallation doesn't seem like a good idea, it would be better something like:

CREATE PROCEDURE Alter_Table()
BEGIN
  DECLARE _count INT;
  SET _count = (SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_NAME = 'cp_itemshop' AND COLUMN_NAME = 'use_existing');
  IF _count = 0 THEN
    ALTER TABLE `cp_itemshop` ADD `use_existing` TINYINT NOT NULL DEFAULT '0' AFTER `info`
  END IF;
END

@Visparu
Copy link

Visparu commented Jan 4, 2024

Unfortunately, this issue still persists and it haunted me while trying to set up a containerized deployment. Since the existence of the respective indices and columns is not checked within the PHP code, some indices are duplicated whenever a new FluxCP instance is created, and the column alterations fail.

The following scripts are problematic:

  • cp_charprefs.20081109093448.sql (Duplicates index for account_id, char_id)
  • cp_charprefs.20120816150540.sql (Duplicates index for char_id)
  • cp_itemshop.20081109093448.sql (Fails due to existing column use_existing)
  • cp_itemshop.20081128093449.sql (Duplicates index for nameid)
  • cp_itemshop.20090104190020.sql (Fails due to existing column category)
  • cp_redeemlog.20081001054354.sql (Fails due to existing columns credits_before and credits_after)
  • cp_redeemlog.20081109093448.sql (Duplicates index for nameid, account_id, char_id)
  • cp_xferlog.20081109093448.sql (Duplicates index for from_account_id, target_account_id, target_char_id)

For a quick workaround (which does solve the crashes without any data loss like having to drop columns), I have expanded on the approach of @Daegaladh, using stored procedures to check for the existence of these indices and columns. This will still bring up the installation screen whenever FluxCP is redeployed, but it will not fail when performing the installation.

So, here are the two types of snippets needed (Using the cp_itemshop table as an example):

One for the ALTER TABLE xxx ADD xxx scripts:

DROP PROCEDURE IF EXISTS alter_cp_itemshop_20090104190020;

DELIMITER //
CREATE PROCEDURE alter_cp_itemshop_20090104190020()
BEGIN
	IF (
		SELECT COUNT(*)
		FROM INFORMATION_SCHEMA.COLUMNS
		WHERE TABLE_NAME = 'cp_itemshop'
		AND COLUMN_NAME = 'category'
	) = 0
	THEN
		ALTER TABLE `cp_itemshop` ADD `category` INT(11) NULL AFTER `nameid`;
	END IF;
END
//

DELIMITER ;

CALL alter_cp_itemshop_20090104190020;

And one for the ALTER TABLE xxx ADD INDEX xxx scripts:

DROP PROCEDURE IF EXISTS alter_cp_itemshop_20081128093449;

DELIMITER //
CREATE PROCEDURE alter_cp_itemshop_20081128093449()
BEGIN
	IF (
		SELECT COUNT(*)
		FROM information_schema.statistics
		WHERE TABLE_NAME = 'cp_itemshop'
		AND INDEX_NAME = 'nameid'
	) = 0
	THEN
		ALTER TABLE `cp_itemshop` ADD INDEX `nameid` (`nameid`);
	END IF;
END
//

DELIMITER ;

CALL alter_cp_itemshop_20081128093449;

Just replace the contents of the existing scripts with these, altering the respective names where appropriate. That specifically includes the table names, index names, column names and procedure names. Keep in mind that some scripts change multiple indices or columns so you might need multiple IF statements.

The procedure names should be unique (though it will work either way since the procedure is dropped on script execution). I used a slightly altered version of the respective script file name.

The index names can just be set to the first column that is part of the index. That is MySQL's default behavior when no index name is specified but it does throw an error if that index already exists, instead of just adding an incrementing number to the end. This makes troubleshooting a little easier since duplicating an index should not be possible with this setup.

@Akkarinage if you'd like, I could create a pull request with these changes. Depends on whether you want to have a permanent solution for this inside the PHP code rather than use this workaround. It does preserve the independence of the code towards the SQL scripts though so I personally think it's quite an elegant solution.

Quick Edit: The DROP PROCEDURE calls were initially only meant for my testing so that I didn't have to drop them manually every time but I left them in since they don't break anything and allow for easier debugging if something does go wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

4 participants