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
Switching version to main as I thing the RM and RMaints filter on that so what would direct to the wrong person once passed QA.
*** Bug 40125 has been marked as a duplicate of this bug. ***
Moving to FQA. The comment of Victor needs attention. Just wondering why you did not change the status yourself, Victor?
Another instance of this happening reported on Mattermost Chat (from 23.11 to 25.05) https://chat.koha-community.org/koha-community/pl/f98s7x3b1fb95pe4y4yg1imako
(In reply to Paul Derscheid from comment #0) > 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. We already have a QA check it seems: https://gitlab.com/koha-community/qa-test-tools/-/blob/main/QohA/File/Perl.pm?ref_type=heads#L176 176 push @forbidden_patterns, 177 { 178 pattern => qr{RENAME\s+COLUMN}i, 179 error => "RENAME COLUMN is not supported in MariaDB < 10.5.2 (see bug 40292)" 180 } 181 if $self->path =~ m|installer/data/mysql/atomicupdate/|; Isn't it working correctly?
*** Bug 40767 has been marked as a duplicate of this bug. ***
Created attachment 187428 [details] [review] Bug 40292: Use Test::NoWarnings
Created attachment 187438 [details] [review] Bug 40292: Use Test::NoWarnings Amended-by: Jonathan Druart Added + 1 to the plan
Created attachment 187439 [details] [review] Bug 40292: Add a specific error if a previous upgrade failed We can reach an inconsistent state if the upgrade DB failed and new_record_id has been created but renamed. I don't think we should try to recover from this state, but have a specific error message in this case. We will guide people asking for help.
Created attachment 187440 [details] [review] Bug 40292: Rename new_record_id if exists If some installs didn't notice the column was missing (not renamed) we can still do it now.
Here is my try to move this blocker forward
Created attachment 187441 [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> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 187442 [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> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 187443 [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> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 187444 [details] [review] Bug 40292: Use Test::NoWarnings Amended-by: Jonathan Druart Added + 1 to the plan Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 187445 [details] [review] Bug 40292: Add a specific error if a previous upgrade failed We can reach an inconsistent state if the upgrade DB failed and new_record_id has been created but renamed. I don't think we should try to recover from this state, but have a specific error message in this case. We will guide people asking for help. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 187446 [details] [review] Bug 40292: Rename new_record_id if exists If some installs didn't notice the column was missing (not renamed) we can still do it now. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 187447 [details] [review] Bug 40292: (QA follow-up) Tidy db_revs/241200018.pl, atomicupdate/bug_40292.pl Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Thanks for working on this @joubu!
Should we give sysadmins a hint? If record_id has been dropped already, it seems like changing new_record_id to record_id is the only thing to do ?
Idea to squash here?
(In reply to Marcel de Rooy from comment #39) > Should we give sysadmins a hint? If record_id has been dropped already, it > seems like changing new_record_id to record_id is the only thing to do ? Maybe, I didn't want to provide SQL queries without knowing exactly what happened.
[FAIL] xt/check_db_revs_sql_syntax.t FAIL forbidden_patterns forbidden pattern: Use https:// instead of http:// for URLs (line 18)
Created attachment 188061 [details] [review] Bug 40292: Use Koha::Devel::Files to list dbrevs For reusability and to limit files to the git index
(In reply to Marcel de Rooy from comment #42) > [FAIL] xt/check_db_revs_sql_syntax.t > FAIL forbidden_patterns > forbidden pattern: Use https:// instead of http:// for URLs > (line 18) Fixed in the last patch (that does more than that).
(In reply to Jonathan Druart from comment #44) > (In reply to Marcel de Rooy from comment #42) > > [FAIL] xt/check_db_revs_sql_syntax.t > > FAIL forbidden_patterns > > forbidden pattern: Use https:// instead of http:// for URLs > > (line 18) > > Fixed in the last patch (that does more than that). I welcome enhancing code. But can we squash this now? How do we want to backport etc?
(In reply to Marcel de Rooy from comment #45) > (In reply to Jonathan Druart from comment #44) > > (In reply to Marcel de Rooy from comment #42) > > > [FAIL] xt/check_db_revs_sql_syntax.t > > > FAIL forbidden_patterns > > > forbidden pattern: Use https:// instead of http:// for URLs > > > (line 18) > > > > Fixed in the last patch (that does more than that). > > I welcome enhancing code. But can we squash this now? How do we want to > backport etc? Better to not squash if we want to backport then. Tests should not process files that are not in the git repository, so this is a bug fix that should be part of this bug report IMO. You are correct that Koha::Devel::Files is not in all supported branches, but this specific patch can be skipped if needed.
Created attachment 188758 [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> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 188759 [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> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 188760 [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> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 188761 [details] [review] Bug 40292: Use Test::NoWarnings Amended-by: Jonathan Druart Added + 1 to the plan Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 188762 [details] [review] Bug 40292: Add a specific error if a previous upgrade failed We can reach an inconsistent state if the upgrade DB failed and new_record_id has been created but renamed. I don't think we should try to recover from this state, but have a specific error message in this case. We will guide people asking for help. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 188763 [details] [review] Bug 40292: Rename new_record_id if exists If some installs didn't notice the column was missing (not renamed) we can still do it now. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 188764 [details] [review] Bug 40292: (QA follow-up) Tidy db_revs/241200018.pl, atomicupdate/bug_40292.pl Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 188765 [details] [review] Bug 40292: Use Koha::Devel::Files to list dbrevs For reusability and to limit files to the git index Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
No need to keep this waiting any longer, although I think that we dont make it easier for the Rmaints here..
Nice work everyone! Pushed to main for 25.11
Nice work everyone! Pushed to 25.05.x