Bug 34520 - Database update 22.06.00.078 breaks update process
Summary: Database update 22.06.00.078 breaks update process
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (web-based installer) (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
Depends on: 24857 24860
Blocks: 36424
  Show dependency treegraph
 
Reported: 2023-08-11 17:18 UTC by Pablo AB
Modified: 2024-04-01 16:23 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.05,22.11.12
Circulation function:


Attachments
Bug 34520: Fix FK for item_groups in reserves for new installations (3.47 KB, patch)
2023-10-29 14:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34520: Fix FK for item_groups in reserves for new installations (3.52 KB, patch)
2023-10-29 22:13 UTC, David Nind
Details | Diff | Splinter Review
Bug 34520: Fix FK for item_groups in reserves for new installations (3.58 KB, patch)
2023-10-30 16:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Pablo AB 2023-08-11 17:18:34 UTC
Trying to upgrade from 22.05.15-1, 10.6.14-MariaDB, Debian 11:

Upgrade to 22.06.00.078  [13:21:46]: Bug 24860 - Add ability to place item group level holds
ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Can't create table `koha_myinstance`.`reserves` (errno: 150 "Foreign key constraint is incorrectly formed") at /usr/share/koha/lib/C4/Installer.pm line 741

I had some issues (bug 33671 comments 14-19) that it seems are now fixed.
Comment 1 Jonathan Druart 2023-08-30 08:29:55 UTC
kohastructure.sql has
5033   CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE CASCADE ON UPDATE CASCADE


but db rev (220600078.pl) has:
14                 ADD CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE SET NULL ON UPDATE CASCADE;

Not sure the error is coming from there however.
Comment 2 Katrin Fischer 2023-10-29 14:01:20 UTC
OK, now I totally confused myself here:

kohastructure.sql:

reserves

  CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE CASCADE ON UPDATE CASCADE

This means, that when the item_group is deleted, we delete the reserve. I think this could create possible issues as we usually should not delete but cancel (move to old_reserves). If we set to null, it turns itself into a title level hold, which seems safer at least. I think SET NULL would make more sense here too.


old_reserves

  CONSTRAINT `old_reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE SET NULL ON UPDATE SET NULL

So, when an item_group is deleted, we set it to null in old_reserves. This makes sense, as we would want to keep the entry for hold statistics.

Database update:

        unless ( column_exists( 'reserves', 'item_group_id' ) ) {
            $dbh->do(q{
                ALTER TABLE reserves
                ADD COLUMN `item_group_id` int(11) NULL default NULL AFTER biblionumber,
                ADD CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE SET NULL ON UPDATE CASCADE;
            });
        }

        unless ( column_exists( 'old_reserves', 'item_group_id' ) ) {
            $dbh->do(q{
                ALTER TABLE old_reserves
                ADD COLUMN `item_group_id` int(11) NULL default NULL AFTER biblionumber,
                ADD CONSTRAINT `old_reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE SET NULL ON UPDATE SET NULL;
            });
        }

I suggest to alter the database definition to fit the original updates.
Comment 3 Katrin Fischer 2023-10-29 14:19:36 UTC
Created attachment 158041 [details] [review]
Bug 34520: Fix FK for item_groups in reserves for new installations

There was a discrepancy between the database update for reserves
and the kohastructure.sql definition. This makes sure that the
FK is always "ON DELETE SET NULL".

To test:
* Before applying this path
* sudo koha-mysql kohadev
* show create table reserves;

CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE CASCADE ON UPDATE CASCADE

* Apply patch
* Run database update
* Check database again:

CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE SET NULL ON UPDATE CASCADE

* reset_all (create a new database)
* Check database again:

CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE SET NULL ON UPDATE CASCADE
Comment 4 Katrin Fischer 2023-10-29 14:21:01 UTC
In the end, it wasn't that confusing, but check my patch carefully please :)
Comment 5 David Nind 2023-10-29 22:13:47 UTC
Created attachment 158052 [details] [review]
Bug 34520: Fix FK for item_groups in reserves for new installations

There was a discrepancy between the database update for reserves
and the kohastructure.sql definition. This makes sure that the
FK is always "ON DELETE SET NULL".

To test:
* Before applying this path
* sudo koha-mysql kohadev
* show create table reserves;

CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE CASCADE ON UPDATE CASCADE

* Apply patch
* Run database update
* Check database again:

CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE SET NULL ON UPDATE CASCADE

* reset_all (create a new database)
* Check database again:

CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE SET NULL ON UPDATE CASCADE

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Nick Clemens (kidclamp) 2023-10-30 16:12:28 UTC
Created attachment 158080 [details] [review]
Bug 34520: Fix FK for item_groups in reserves for new installations

There was a discrepancy between the database update for reserves
and the kohastructure.sql definition. This makes sure that the
FK is always "ON DELETE SET NULL".

To test:
* Before applying this path
* sudo koha-mysql kohadev
* show create table reserves;

CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE CASCADE ON UPDATE CASCADE

* Apply patch
* Run database update
* Check database again:

CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE SET NULL ON UPDATE CASCADE

* reset_all (create a new database)
* Check database again:

CONSTRAINT `reserves_ibfk_ig` FOREIGN KEY (`item_group_id`) REFERENCES `item_groups` (`item_group_id`) ON DELETE SET NULL ON UPDATE CASCADE

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Tomás Cohen Arazi (tcohen) 2023-10-31 14:06:38 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 8 Fridolin Somers 2023-11-02 20:23:12 UTC
Pushed to 23.05.x for 23.05.05
Comment 9 Matt Blenkinsop 2023-11-13 15:11:19 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x
Comment 10 Pablo AB 2023-11-16 20:44:35 UTC
After this bug being fixed, I tried again:

Unpacking koha-common (23.05.04-2) over (22.05.16-2) ...

[...]

Upgrade to 22.06.00.077  [17:29:18]: Bug 31713 - Add ACCOUNTS_SUMMARY slip notice
        Added new letter 'ACCOUNTS_SUMMARY' (print)
Upgrade to 22.06.00.078  [17:29:18]: Bug 24860 - Add ability to place item group level holds
ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Can't create table `koha_myinstance`.`#sql-e9_19a4` (errno: 150 "Foreign key constraint is incorrectly formed") at /usr/share/koha/lib/C4/Installer.pm line 741

dpkg: error processing package koha-common (--configure):
 installed koha-common package post-installation script subprocess returned error exit status 1

I should file a new issue, or this one should be reopened?
Comment 11 Jonathan Druart 2023-11-23 12:48:29 UTC
(In reply to Matt Blenkinsop from comment #9)
> Nice work everyone!
> 
> Pushed to oldstable for 22.11.x

Wrong commit

commit 2ce9f0ce5b6a0c502f3e329ad80ec85322047257

+$VERSION = "22.11.11.001";

 $VERSION = "22.11.11.000";
Comment 12 Jonathan Druart 2023-11-23 12:48:41 UTC
(In reply to Pablo AB from comment #10)
> I should file a new issue, or this one should be reopened?

Yes, please.
Comment 13 Jonathan Druart 2023-11-23 12:49:53 UTC
Why didn't we fix the problematic DBRev here?
Comment 14 Pedro Amorim 2023-11-28 14:40:21 UTC
Thanks Joubu, we fixed it.
Comment 15 Katrin Fischer 2024-04-01 16:23:56 UTC
(In reply to Pedro Amorim from comment #14)
> Thanks Joubu, we fixed it.

I think what Joubu meant was why we didn't fix the original database update too - have to admit I don't know. It looks like another problem popped up on bug 36424 now.