Upgrade to 23.12.00.044 [17:47:36]: Bug 36120 - Add pickup location to bookings Added column 'bookings.pickup_library_id' Set existing bookings pickup location to item homebranch ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Cannot change column 'pickup_library_id': used in a foreign key constraint 'bookings_ibfk_4' at /usr/share/koha/lib/C4/Installer.pm line 741
Created attachment 167376 [details] [review] Bug 37000: [Bug 36120 follow-up] Improve reliability of database update This patch adds a series of fallthroughs to ensure pickup_library_id is always set prior to adding the NOT NULL constraint. We initially only looked at items.homebranch but as that's a nullable field itself, we now look at items.holdingbranch before finally defaulting to the first available branch in the branches table in the worst case.
Created attachment 167563 [details] [review] Bug 37000: [Bug 36120 follow-up] Improve reliability of database update This patch adds a series of fallthroughs to ensure pickup_library_id is always set prior to adding the NOT NULL constraint. We initially only looked at items.homebranch but as that's a nullable field itself, we now look at items.holdingbranch before finally defaulting to the first available branch in the branches table in the worst case. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 167573 [details] [review] Bug 37000: (Bug 36120 follow-up) Improve reliability of database update This patch adds a series of fallthroughs to ensure pickup_library_id is always set prior to adding the NOT NULL constraint. We initially only looked at items.homebranch but as that's a nullable field itself, we now look at items.holdingbranch before finally defaulting to the first available branch in the branches table in the worst case. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Sorry, this doesn't completely fix the issue for production environments using MySQL. If strict_mode is off, MySQL won't allow you to change a nullable column to non-nullable if the column has a foreign key, even if there are no existing null values in the data. (see https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35687#c13) I was able to reproduce the error, even with this patch applied, with the following steps: 1. Shut down KTD 2. Switch to 23.11.x 3. Start up KTD with MySQL-8 (I did this using the built-in alias ku-my8) 4. Inside a kshell, edit /etc/koha/sites/kohadev/koha-conf.xml. Set <strict-sql-modes> to 0. 5. Switch to main and apply patch 6. restart_all 7. updatedatabase --> observe the original error Can the foreign key be added at the end of the dbrev, after the NOT NULL constraint is set, instead of the beginning? items.homebranch and items.holdingbranch already have a FK referencing branches.branchcode themselves, so it should be safe to fill in those values and add the new FK after. Incidentally, if I ran the database update a second time immediately after the first, the database appeared to successfully complete the update to 24.05; however, this dbrev silently did nothing because the pickup_library_id column already exists, and the foreign key never got added. Will upload a follow-up patch in a bit that I believe will address both of these issues...
Created attachment 167589 [details] [review] Bug 37000: (follow-up) Add foreign key last Certain configurations of MySQL will not allow a column to be changed from nullable to non-nullable if the column has a foreign key constraint. Add the foreign key constraint last to avoid issues from this.
Hope I'm not overstepping here...I felt that submitting code was clearer than my comment was on its own, and I didn't want to stall the bug at FQA. Setting back to SO to give Martin and Julian a chance to review.
Looks great to me, thanks.
Thanks for all the hard work! Pushed to main for the next 24.11.00 release as RM Assistant
Backported to 23.05.x for upcoming 24.05.02