It appears that MySQL 8 rejects creating any table where the primary key is nullable. The table tmp_holdsqueue has a nullable pk ( itemnumber ) but there is no reason for this column to be nullable ( generating a holds queue entry with no itemnumber is not possible ) so it make sense to just remove the nullability for MySQL compatiability.
Created attachment 154315 [details] [review] Bug 34494: Table tmp_holdsqueue fails to be created for MySQL 8 It appears that MySQL 8 rejects creating any table where the primary key is nullable. The table tmp_holdsqueue has a nullable pk ( itemnumber ) but there is no reason for this column to be nullable ( generating a holds queue entry with no itemnumber is not possible ) so it make sense to just remove the nullability for MySQL compatiability. Test Plan: 1) Using MySQL 8.0, attempt to create a database from kohastructure.sql If you use ktd, you can try: DB_IMAGE=mysql:8.0 ktd up 2) Note the failure to create the table 3) Apply this patch 4) Repeat step 1 5) The table is created!
Created attachment 154317 [details] [review] Bug 34494: Table tmp_holdsqueue fails to be created for MySQL 8 It appears that MySQL 8 rejects creating any table where the primary key is nullable. The table tmp_holdsqueue has a nullable pk ( itemnumber ) but there is no reason for this column to be nullable ( generating a holds queue entry with no itemnumber is not possible ) so it make sense to just remove the nullability for MySQL compatiability. Test Plan: 1) Using MySQL 8.0, attempt to create a database from kohastructure.sql If you use ktd, you can try: DB_IMAGE=mysql:8.0 ktd up 2) Note the failure to create the table 3) Apply this patch 4) Repeat step 1 5) The table is created! Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. Before applying the patch, testing with MySQL 8.0 as the database fails, as expected (exits during start up - edit .env, change DB_IMAGE=mysql:8.0, ktd up). 2. Shut down ktd, then change database back to DB_IMAGE=mariadb:10.5 - KTD should start up as normal. Apply the patch. 3. Shut down KTD, change database to MySQL as in step 1, start up should now be successful. 4. Probably an easier way, but this worked for me when testing.
Created attachment 154417 [details] [review] Bug 34494: Table tmp_holdsqueue fails to be created for MySQL 8 It appears that MySQL 8 rejects creating any table where the primary key is nullable. The table tmp_holdsqueue has a nullable pk ( itemnumber ) but there is no reason for this column to be nullable ( generating a holds queue entry with no itemnumber is not possible ) so it make sense to just remove the nullability for MySQL compatiability. Test Plan: 1) Using MySQL 8.0, attempt to create a database from kohastructure.sql If you use ktd, you can try: DB_IMAGE=mysql:8.0 ktd up 2) Note the failure to create the table 3) Apply this patch 4) Repeat step 1 5) The table is created! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 154418 [details] [review] Bug 34494: (QA follow-up) fiks spelin
weird, with and without the patch I'm hitting this error with MySQL8 before getting to the part relevant to this ticket: koha-koha-1 | DBIx::Class::Storage::DBI::catch {...} (): DBI Connection failed: DBI connect('database=koha_kohadev;host=db;port=3306','koha_kohadev',...) failed: Authentication plugin 'caching_sha2_password' reported error: Authentication requires secure connection. at /kohadevbox/koha/Koha/Database.pm line 91. at /kohadevbox/koha/Koha/Database.pm line 139 koha-koha-1 exited with code 1 Does that speak to anyone? ktd repo, koha code, ktd images up to date **Anyway**, it works for Kyle and David and the change is trivial, there is enough confidence to not block on my dev env issue. And with the patch, it still works for me with MariaDB. And this will be the CI which fails on the right issue so it should be fixed. So: Makes sense, QA script happy[1], code looks good, passing QA :) [1] waived this perltidy warning: ``` $dbh->do(q{ ALTER TABLE tmp_holdsqueue CHANGE COLUMN itemnumber `itemnumber` int(11) NOT NULL }); ``` ↓↓↓↓↓ ``` $dbh->do( q{ ALTER TABLE tmp_holdsqueue CHANGE COLUMN itemnumber `itemnumber` int(11) NOT NULL } ); ``` The proposed code is following our two examples: https://wiki.koha-community.org/wiki/Database_updates minor tangent question: @Kyle, @Tomas Maybe they should be changed or it's not a big deal to have this warning on each new db rev?
Testing the upgrade before 28966 (using 328baf44b457bc34cd93ffad8128b79d4ee30a42).
Upgrading works correctly.
Pushed to master for 23.11. Nice work everyone, thanks!
(In reply to Tomás Cohen Arazi from comment #7) > Testing the upgrade before 28966 (using > 328baf44b457bc34cd93ffad8128b79d4ee30a42). Thanks, so the idea is if possible, spin a Koha on the version just before the problem was created. And then upgrade it to main/master + the patch to see if the combination works, did I get it right?
Depends on Bug 28966 not in 23.05.x