Bug 23813

Summary: DB error on 18.12.00.020
Product: Koha Reporter: Magnus Enger <magnus>
Component: Installation and upgrade (command-line installer)Assignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: critical    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, katrin.fischer, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18925
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.05
Bug Depends on: 18925    
Bug Blocks:    
Attachments: Bug 23813: (bug 18925 follow-up) Remove invalid circ rules to avoid FK constraints to fail
Bug 23813: (bug 18925 follow-up) Remove invalid circ rules to avoid FK constraints to fail

Description Magnus Enger 2019-10-15 15:53:24 UTC
I just copied the DB from a gitified install running 18.1200014 to a standard Debian install of 19.05.04. When I ran "sudo koha-upgrade-schema <instance> I got this error:

DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`koha_alingsas`.`circulation_rules`, CONSTRAINT `circ_rules_ibfk_3` FOREIGN KEY (`itemtype`) REFERENCES `itemtypes` (`itemtype`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "
            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
            SELECT IF(categorycode='*', NULL, categorycode),
                   IF(branchcode='*', NULL, branchcode),
                   IF(itemtype='*', NULL, itemtype),
                   'maxissueqty',
                   COALESCE( maxissueqty, '' )
            FROM issuingrules
        "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 17545.
DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`koha_alingsas`.`circulation_rules`, CONSTRAINT `circ_rules_ibfk_3` FOREIGN KEY (`itemtype`) REFERENCES `itemtypes` (`itemtype`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "
            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
            SELECT IF(categorycode='*', NULL, categorycode),
                   IF(branchcode='*', NULL, branchcode),
                   IF(itemtype='*', NULL, itemtype),
                   'maxonsiteissueqty',
                   COALESCE( maxonsiteissueqty, '' )
            FROM issuingrules
        "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 17554.
Upgrade to 18.12.00.020 done (Bug 18925 - Move maxissueqty and maxonsiteissueqty to circulation_rules)

I have not looked further into this yet, just thought I'd report it sooner rather than later.
Comment 1 Magnus Enger 2019-10-15 16:04:43 UTC
After the upgrade there is 0 rows in circulation_rules and 225 in issuingrules.
Comment 2 Jonathan Druart 2019-10-16 07:29:12 UTC
Confirmed, it happens if the item type of a circ rule has been deleted.

Should we remove the rule silently? Create a temporary item type and display a warning?

Upping severity.
Comment 3 Katrin Fischer 2019-10-18 06:35:23 UTC
(In reply to Jonathan Druart from comment #2)
> Confirmed, it happens if the item type of a circ rule has been deleted.
> 
> Should we remove the rule silently? Create a temporary item type and display
> a warning?
> 
> Upping severity.

I was always under the impression that they would be deleted by FK constraint (delete the itemtype, issuingrules cleans up...) So I'd opt for removing silently. There can be no items with this itemtype, so cleaning up the issuing rule makes sense to me, but wondering if there is a deeper issue to have them in the first place?
Comment 4 Jonathan Druart 2019-10-22 10:49:22 UTC
Created attachment 94524 [details] [review]
Bug 23813: (bug 18925 follow-up) Remove invalid circ rules to avoid FK constraints to fail

There is not FK constraints on the itemtype, categorycode and branchcode
columns of the issuingrules table, because of the '*' value we use for
default.
We created the new table circulation_rules with those constraints, and
when we moved the rules for maxissueqty, the INSERT failed.

To avoid the FK constraints to fail we are going to delete the invalid
rows before.

It could lead to data lost, but this situation certainly comes from a
bad data manipulation at some point of the history of the installation.

Test plan:
git reset --hard 1e4f442442844cfaf17f5b06f122f6259d5fee92 # before 18.12.00.020
reset_all

insert into branches(branchcode, branchname) values('rmme', 'rmme');
insert into categories (categorycode, description) values ('rmme', 'rmme');
insert into itemtypes (itemtype, description) values('rmme', 'rmme');

Go to http://pro.kohadev.org/cgi-bin/koha/admin/smart-rules.pl
Create a new rule for the patron category
Create a new rule for the item type
Copy all the default rule to the new library
Create other rules for "Default checkout, hold policy by patron category" and "Default holds policy by item type" using this patron's category and itemtype.
Same for patrons' categories and itemtypes you will not delete.

delete from branches where branchcode="rmme";
delete from categories where categorycode="rmme";
delete from itemtypes where itemtype="rmme";

git checkout master
and execute the DB process.

=> With this patch applied you will not get the error and the rules will
be moved correctly.
Comment 5 Magnus Enger 2019-10-22 12:27:13 UTC
I tried to copy over the db again, cleaned up some deleted itemtypes and tried the upgrade. This time I got errors related to categorycodes:

$ sudo koha-upgrade-schema <instance>
Upgrading database schema for <instance>
...
Upgrade to 18.12.00.019 done (Bug 21728 - Add 'Reserve Fee' to the account_offset_types table if missing)
DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`koha_alingsas`.`circulation_rules`, CONSTRAINT `circ_rules_ibfk_2` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "
            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
            SELECT IF(categorycode='*', NULL, categorycode),
                   IF(branchcode='*', NULL, branchcode),
                   IF(itemtype='*', NULL, itemtype),
                   'maxissueqty',
                   COALESCE( maxissueqty, '' )
            FROM issuingrules
        "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 17545.
DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`koha_alingsas`.`circulation_rules`, CONSTRAINT `circ_rules_ibfk_2` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "
            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
            SELECT IF(categorycode='*', NULL, categorycode),
                   IF(branchcode='*', NULL, branchcode),
                   IF(itemtype='*', NULL, itemtype),
                   'maxonsiteissueqty',
                   COALESCE( maxonsiteissueqty, '' )
            FROM issuingrules
        "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 17554.
Upgrade to 18.12.00.020 done (Bug 18925 - Move maxissueqty and maxonsiteissueqty to circulation_rules)
...
Comment 6 Jonathan Druart 2019-10-22 12:43:30 UTC
My patch deals with itemtype, categorycode, and branchcode.
Comment 7 Martin Renvoize 2019-11-04 16:57:55 UTC
Created attachment 95020 [details] [review]
Bug 23813: (bug 18925 follow-up) Remove invalid circ rules to avoid FK constraints to fail

There is not FK constraints on the itemtype, categorycode and branchcode
columns of the issuingrules table, because of the '*' value we use for
default.
We created the new table circulation_rules with those constraints, and
when we moved the rules for maxissueqty, the INSERT failed.

To avoid the FK constraints to fail we are going to delete the invalid
rows before.

It could lead to data lost, but this situation certainly comes from a
bad data manipulation at some point of the history of the installation.

Test plan:
git reset --hard 1e4f442442844cfaf17f5b06f122f6259d5fee92 # before 18.12.00.020
reset_all

insert into branches(branchcode, branchname) values('rmme', 'rmme');
insert into categories (categorycode, description) values ('rmme', 'rmme');
insert into itemtypes (itemtype, description) values('rmme', 'rmme');

Go to http://pro.kohadev.org/cgi-bin/koha/admin/smart-rules.pl
Create a new rule for the patron category
Create a new rule for the item type
Copy all the default rule to the new library
Create other rules for "Default checkout, hold policy by patron category" and "Default holds policy by item type" using this patron's category and itemtype.
Same for patrons' categories and itemtypes you will not delete.

delete from branches where branchcode="rmme";
delete from categories where categorycode="rmme";
delete from itemtypes where itemtype="rmme";

git checkout master
and execute the DB process.

=> With this patch applied you will not get the error and the rules will
be moved correctly.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2019-11-04 16:58:31 UTC
Code reads well and the patch does resolve the issue for me..

Signing Off
Comment 9 Martin Renvoize 2019-11-07 09:23:25 UTC
Going for PQA here, it's a simple patch that should only improve the situation.
Comment 10 Martin Renvoize 2019-11-07 09:23:58 UTC
Nice work!

Pushed to master for 19.11.00
Comment 11 Fridolin Somers 2019-11-15 08:17:34 UTC
Pushed to 19.05.x for 19.05.05