Bug 34520

Summary:	Database update 22.06.00.078 breaks update process
Product:	Koha	Reporter:	Pablo AB <pablo.bianchi>
Component:	Installation and upgrade (web-based installer)	Assignee:	Katrin Fischer <katrin.fischer>
Status:	Pushed to oldstable ---	QA Contact:	Testopia <testopia>
Severity:	critical
Priority:	P5 - low	CC:	fridolin.somers, gmcharlt, jonathan.druart, kyle, pedro.amorim
Version:	Main
Hardware:	All
OS:	All
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:
Bug Depends on:	24857, 24860
Bug Blocks:	36424
Attachments:	Bug 34520: Fix FK for item_groups in reserves for new installations Bug 34520: Fix FK for item_groups in reserves for new installations Bug 34520: Fix FK for item_groups in reserves for new installations

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.