Got this report on Mattermost for 25.05: Configurando koha-common (25.05.01-1) ... Upgrading database schema for biblio Upgrade to 24.12.00.042 [07:56:03]: Bug 39838 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39838) - Rename alias_id -> vendor_alias_id ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'COLUMN alias_id TO vendor_alias_id' at line 1 at /usr/share/koha/lib/C4/Installer.pm line 825 After investigation and confirmation, the usage of RENAME COLUMN is the problem. This syntax is unsupported in versions of MariaDB prior to 10.5.3, here's the relevant documentation for reference: mariadb dot com/docs/server/reference/sql-statements/data-definition/alter/alter-table#rename-column The solution is to fix the usage of RENAME COLUMN in the current db_revs and to write a coding guideline and a QA test to accompany it.
Created attachment 183706 [details] [review] Bug 40292: Prevent SQL syntax error when upgrading to 25.05 on MariaDB 10.3 - Change `RENAME COLUMN <OLD> TO <NEW>` to `CHANGE COLUMN <OLD> <NEW> <DATATYPE (required for this syntax)>`. To test (assuming ktd): 1. Checkout upstream/24.11.x 2. Start ktd with `KOHA_IMAGE=24.11-bullseye(-arm64) DB_IMAGE=mariadb:10.3 ktd up -d` 3. Checkout upstream/main 4. Run updatedatabase in the Koha container 5. Observe the error regarding the unsupported syntax. 6. Tear everything down again: ktd down 7. Repeat 1-2. 8. Once started up, checkout upstream/main 9. Apply the patch on top of it 10. Run updatedatabase in the Koha container 11. Observe a successful upgrade 12. Sign off Side Note: I tested this against upstream/25.05.x, but this patch is based on main.
> The solution is to fix the usage of RENAME COLUMN in the current db_revs and to write a coding guideline and a QA test to accompany it. For 2nd point: alternative/complement (much wider scope of the checks even if it's later in the workflow): Bug 40295 - Add CI job Koha_Main_MariaDB_update but with lowest MariaDB version (10.3)
we need a test for this (qa and ideally xt as well)
Created attachment 183747 [details] [review] Bug 40292: Prevent SQL syntax error when upgrading to 25.05 on MariaDB 10.3 - Change `RENAME COLUMN <OLD> TO <NEW>` to `CHANGE COLUMN <OLD> <NEW> <DATATYPE (required for this syntax)>`. To test (assuming ktd): 1. Checkout upstream/24.11.x 2. Start ktd with `KOHA_IMAGE=24.11-bullseye(-arm64) DB_IMAGE=mariadb:10.3 ktd up -d` 3. Checkout upstream/main 4. Run updatedatabase in the Koha container 5. Observe the error regarding the unsupported syntax. 6. Tear everything down again: ktd down 7. Repeat 1-2. 8. Once started up, checkout upstream/main 9. Apply the patch on top of it 10. Run updatedatabase in the Koha container 11. Observe a successful upgrade 12. Sign off Side Note: I tested this against upstream/25.05.x, but this patch is based on main. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 183748 [details] [review] Bug 40292: (follow-up) Add xt test to prevent future occurences Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 183749 [details] [review] Bug 40292: Prevent SQL syntax error when upgrading to 25.05 on MariaDB 10.3 - Change `RENAME COLUMN <OLD> TO <NEW>` to `CHANGE COLUMN <OLD> <NEW> <DATATYPE (required for this syntax)>`. To test (assuming ktd): 1. Checkout upstream/24.11.x 2. Start ktd with `KOHA_IMAGE=24.11-bullseye(-arm64) DB_IMAGE=mariadb:10.3 ktd up -d` 3. Checkout upstream/main 4. Run updatedatabase in the Koha container 5. Observe the error regarding the unsupported syntax. 6. Tear everything down again: ktd down 7. Repeat 1-2. 8. Once started up, checkout upstream/main 9. Apply the patch on top of it 10. Run updatedatabase in the Koha container 11. Observe a successful upgrade 12. Sign off Side Note: I tested this against upstream/25.05.x, but this patch is based on main. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 183750 [details] [review] Bug 40292: (follow-up) Add xt test to prevent future occurences Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Working on QA script addition
hold up! vendor_alias_id is loosing auto_increment after doing a diff on `describe aqbookseller_aliases;`
Thanks tuxayo, I overlooked that we need to seemingly specify everything as in mariadb com /docs/server/reference/sql-statements/data-definition/alter/alter-table#change-column
With such changes you should run misc4dev/run_tests.pl with --run-db-upgrade-only
Created attachment 183754 [details] [review] Bug 40292: Specify full datatypes as required for CHANGE COLUMN To test: 1. Follow the original test plan and compare the resulting tables to: MariaDB [koha_kohadev]> desc additional_field_values; +-----------+--------------+------+-----+---------+----------------+ | Field | Type | Null | Key | Default | Extra | +-----------+--------------+------+-----+---------+----------------+ | id | int(11) | NO | PRI | NULL | auto_increment | | field_id | int(11) | NO | MUL | NULL | | | record_id | int(11) | NO | | NULL | | | value | varchar(255) | NO | | | | +-----------+--------------+------+-----+---------+----------------+ 4 rows in set (0.003 sec) MariaDB [koha_kohadev]> desc aqbookseller_aliases; +-----------+--------------+------+-----+---------+----------------+ | Field | Type | Null | Key | Default | Extra | +-----------+--------------+------+-----+---------+----------------+ | alias_id | int(11) | NO | PRI | NULL | auto_increment | | vendor_id | int(11) | NO | MUL | NULL | | | alias | varchar(255) | NO | | NULL | | +-----------+--------------+------+-----+---------+----------------+ 3 rows in set (0.004 sec) MariaDB [koha_kohadev]> desc aqbookseller_interfaces; +---------------+--------------+------+-----+---------+----------------+ | Field | Type | Null | Key | Default | Extra | +---------------+--------------+------+-----+---------+----------------+ | interface_id | int(11) | NO | PRI | NULL | auto_increment | | vendor_id | int(11) | NO | MUL | NULL | | | type | varchar(80) | YES | | NULL | | | name | varchar(255) | NO | | NULL | | | uri | mediumtext | YES | | NULL | | | login | varchar(255) | YES | | NULL | | | password | mediumtext | YES | | NULL | | | account_email | mediumtext | YES | | NULL | | | notes | longtext | YES | | NULL | | +---------------+--------------+------+-----+---------+----------------+ 9 rows in set (0.003 sec) The Type field should be the only deviation from the previous version, Null and Extra should be identical. 2. Run misc4dev/run_tests.pl --run-db-upgrade-only 3. Sign off
Tests are also picky about COMMENT perl /kohadevbox/misc4dev/run_tests.pl --run-db-compare-only --compare-with v24.11.00 And CHANGE COLUMN also requires them ^^" Follow-up coming.
Created attachment 183775 [details] [review] Bug 40292: Specify full datatypes and comment as required for CHANGE COLUMN To test: 1. Follow the original test plan and compare the resulting tables to: MariaDB [koha_kohadev]> desc additional_field_values; +-----------+--------------+------+-----+---------+----------------+ | Field | Type | Null | Key | Default | Extra | +-----------+--------------+------+-----+---------+----------------+ | id | int(11) | NO | PRI | NULL | auto_increment | | field_id | int(11) | NO | MUL | NULL | | | record_id | int(11) | NO | | NULL | | | value | varchar(255) | NO | | | | +-----------+--------------+------+-----+---------+----------------+ 4 rows in set (0.003 sec) MariaDB [koha_kohadev]> desc aqbookseller_aliases; +-----------+--------------+------+-----+---------+----------------+ | Field | Type | Null | Key | Default | Extra | +-----------+--------------+------+-----+---------+----------------+ | alias_id | int(11) | NO | PRI | NULL | auto_increment | | vendor_id | int(11) | NO | MUL | NULL | | | alias | varchar(255) | NO | | NULL | | +-----------+--------------+------+-----+---------+----------------+ 3 rows in set (0.004 sec) MariaDB [koha_kohadev]> desc aqbookseller_interfaces; +---------------+--------------+------+-----+---------+----------------+ | Field | Type | Null | Key | Default | Extra | +---------------+--------------+------+-----+---------+----------------+ | interface_id | int(11) | NO | PRI | NULL | auto_increment | | vendor_id | int(11) | NO | MUL | NULL | | | type | varchar(80) | YES | | NULL | | | name | varchar(255) | NO | | NULL | | | uri | mediumtext | YES | | NULL | | | login | varchar(255) | YES | | NULL | | | password | mediumtext | YES | | NULL | | | account_email | mediumtext | YES | | NULL | | | notes | longtext | YES | | NULL | | +---------------+--------------+------+-----+---------+----------------+ 9 rows in set (0.003 sec) The Type field should be the only deviation from the previous version, Null and Extra should be identical. 2. Run misc4dev/run_tests.pl --run-db-upgrade-only 3. Sign off Co-authored-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 183777 [details] [review] Bug 40292: (follow-up) Add xt test to prevent future occurences Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 183778 [details] [review] Bug 40292: Prevent SQL syntax error when upgrading to 25.05 on MariaDB 10.3 - Change `RENAME COLUMN <OLD> TO <NEW>` to `CHANGE COLUMN <OLD> <NEW> <DATATYPE (required for this syntax)>`. To test (assuming ktd): 1. Checkout upstream/24.11.x 2. Start ktd with `KOHA_IMAGE=24.11-bullseye(-arm64) DB_IMAGE=mariadb:10.3 ktd up -d` 3. Checkout upstream/main 4. Run updatedatabase in the Koha container 5. Observe the error regarding the unsupported syntax. 6. Tear everything down again: ktd down 7. Repeat 1-2. 8. Once started up, checkout upstream/main 9. Apply the patch on top of it 10. Run updatedatabase in the Koha container 11. Observe a successful upgrade 12. Sign off Side Note: I tested this against upstream/25.05.x, but this patch is based on main. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 183779 [details] [review] Bug 40292: Specify full datatypes and comment as required for CHANGE COLUMN To test: 1. Follow the original test plan and compare the resulting tables to: MariaDB [koha_kohadev]> desc additional_field_values; +-----------+--------------+------+-----+---------+----------------+ | Field | Type | Null | Key | Default | Extra | +-----------+--------------+------+-----+---------+----------------+ | id | int(11) | NO | PRI | NULL | auto_increment | | field_id | int(11) | NO | MUL | NULL | | | record_id | int(11) | NO | | NULL | | | value | varchar(255) | NO | | | | +-----------+--------------+------+-----+---------+----------------+ 4 rows in set (0.003 sec) MariaDB [koha_kohadev]> desc aqbookseller_aliases; +-----------+--------------+------+-----+---------+----------------+ | Field | Type | Null | Key | Default | Extra | +-----------+--------------+------+-----+---------+----------------+ | alias_id | int(11) | NO | PRI | NULL | auto_increment | | vendor_id | int(11) | NO | MUL | NULL | | | alias | varchar(255) | NO | | NULL | | +-----------+--------------+------+-----+---------+----------------+ 3 rows in set (0.004 sec) MariaDB [koha_kohadev]> desc aqbookseller_interfaces; +---------------+--------------+------+-----+---------+----------------+ | Field | Type | Null | Key | Default | Extra | +---------------+--------------+------+-----+---------+----------------+ | interface_id | int(11) | NO | PRI | NULL | auto_increment | | vendor_id | int(11) | NO | MUL | NULL | | | type | varchar(80) | YES | | NULL | | | name | varchar(255) | NO | | NULL | | | uri | mediumtext | YES | | NULL | | | login | varchar(255) | YES | | NULL | | | password | mediumtext | YES | | NULL | | | account_email | mediumtext | YES | | NULL | | | notes | longtext | YES | | NULL | | +---------------+--------------+------+-----+---------+----------------+ 9 rows in set (0.003 sec) The Type field should be the only deviation from the previous version, Null and Extra should be identical. 2. Run perl ../misc4dev/run_tests.pl --run-db-compare-only --compare-with v24.05.00 3. Sign off Co-authored-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
additional_field_values.record_id was a bit weird because there is another db rev (241200045.pl) that changes the type again and would overwrite the full column definition with a correct one. Fixing mistakes that would have been in 241200018.pl Still fully fixed anyway. (commit amended) ----- The xt test was not tidy. commit amended.
This is what happens when hitting the bug. Apply the fix. And rerunning the upgrade: ---------------------- == 1. run the old upgrade == Upgrade to 24.12.00.018 [19:43:50]: Bug 38663 - Add additional fields to libraries Converting record_id from int(11) to VARCHAR(11)... ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'COLUMN new_record_id TO record_id' at line 1 at /kohadevbox/koha/C4/Installer.pm line 825 == 2. apply patches == == 3. retry to run the upgrade == kohadev-koha@kohadevbox:koha(testing-SO-QA)$ updatedatabase Upgrade to 24.12.00.018 [19:45:11]: Bug 38663 - Add additional fields to libraries No conversion needed for record_id column. ↑↑↑↑↑↑↑ that's where it leaves new_record_id [....] Upgrade to 24.12.00.045 [19:45:12]: Bug 38457 - Add additional fields to debit types ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Unknown column 'record_id' in 'additional_field_values' at /kohadevbox/koha/C4/Installer.pm line 825 ---------------------- - the temporary new_record_id is still hanging around with the data: blocking, fix should be obvious. - it's type is VARCHAR(11) and the conversion to VARCHAR(80) has been skipped. Non blocking and can end up staying like that. Causing a divergence with the DBIC schema files. I don't know what that will mess up with. Is there anything more to worry about? (and warranting more work, the above isn't IMHO) Admins should hopefully have rollbacked the upgrade and restored a backup instead of trying to guess what to fix. If they did anyway, then at least above is a warning about one non obvious thing. And they should look here for how to compare their database schema with kohastructure.sql https://gitlab.com/koha-community/koha-misc4dev/-/blob/e7657e08b73a0d7b5ef372c806ad14c32ceef667/run_tests.pl#L345