Bug 34494 - Table tmp_holdsqueue fails to be created for MySQL 8
Summary: Table tmp_holdsqueue fails to be created for MySQL 8
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low blocker (vote)
Assignee: Kyle M Hall
QA Contact: Victor Grousset/tuxayo
URL:
Keywords:
Depends on: 28966
Blocks:
  Show dependency treegraph
 
Reported: 2023-08-08 11:10 UTC by Kyle M Hall
Modified: 2023-11-02 19:55 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 34494: Table tmp_holdsqueue fails to be created for MySQL 8 (2.27 KB, patch)
2023-08-08 11:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34494: Table tmp_holdsqueue fails to be created for MySQL 8 (2.31 KB, patch)
2023-08-08 13:05 UTC, David Nind
Details | Diff | Splinter Review
Bug 34494: Table tmp_holdsqueue fails to be created for MySQL 8 (2.37 KB, patch)
2023-08-15 07:06 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 34494: (QA follow-up) fiks spelin (937 bytes, patch)
2023-08-15 07:06 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2023-08-08 11:10:37 UTC
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.
Comment 1 Kyle M Hall 2023-08-08 11:19:57 UTC
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!
Comment 2 David Nind 2023-08-08 13:05:51 UTC
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>
Comment 3 David Nind 2023-08-08 13:16:00 UTC
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.
Comment 4 Victor Grousset/tuxayo 2023-08-15 07:06:12 UTC
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>
Comment 5 Victor Grousset/tuxayo 2023-08-15 07:06:15 UTC
Created attachment 154418 [details] [review]
Bug 34494: (QA follow-up) fiks spelin
Comment 6 Victor Grousset/tuxayo 2023-08-15 07:53:50 UTC
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?
Comment 7 Tomás Cohen Arazi 2023-08-15 08:09:47 UTC
Testing the upgrade before 28966 (using 328baf44b457bc34cd93ffad8128b79d4ee30a42).
Comment 8 Tomás Cohen Arazi 2023-08-15 08:19:11 UTC
Upgrading works correctly.
Comment 9 Tomás Cohen Arazi 2023-08-15 08:25:09 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 10 Victor Grousset/tuxayo 2023-08-16 02:57:54 UTC
(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?
Comment 11 Fridolin Somers 2023-08-17 19:30:24 UTC
Depends on Bug 28966 not in 23.05.x