Bug 40057

Summary: Database update 24.12.00.017 fails if old ILL data points to non-existent borrowernumber
Product: Koha Reporter: Pedro Amorim <pedro.amorim>
Component: ILLAssignee: Pedro Amorim <pedro.amorim>
Status: Pushed to main --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: david, m.de.rooy, martin.renvoize, pedro.amorim, tomascohen
Version: MainKeywords: rel_25_05_candidate
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
This fixes a database update related to ILL requests, for bug 32630 - Don't delete ILL requests when patron is deleted, added in Koha 25.05. Background: Some databases have very old ILL requests where 'borrowernumber' has a value of a borrowernumber that doesn't exist. We're not exactly how the data ended up this way, but it's happened at least twice now for one provider.
Version(s) released in:
25.11.00
Circulation function:
Bug Depends on: 32630    
Bug Blocks:    
Attachments: Bug 40057: Fix db revision
Bug 40057: Fix db revision
Bug 40057: Fix db revision
Bug 40057: Fix db revision

Description Pedro Amorim 2025-06-03 11:16:51 UTC

    
Comment 1 Pedro Amorim 2025-06-03 11:20:53 UTC
Created attachment 182919 [details] [review]
Bug 40057: Fix db revision

Test plan (ktd):
1) reset_all
2) Run first part of the 241200017.pl atomicupdate manually
	koha-mysql kohadev
	ALTER TABLE illrequests DROP FOREIGN KEY illrequests_bnfk;
3) Create an ILL request linked to a borrower that doesn't exist (simulate old data):
	INSERT INTO illrequests (, , ) VALUES ('Standard', '99999', 'CPL');
4) Run the 2nd part of the atomicupdate manually:
	ALTER TABLE illrequests ADD CONSTRAINT illrequests_bnfk
	FOREIGN KEY()
	REFERENCES  ()
	ON DELETE SET NULL ON UPDATE CASCADE;
5) Notice you get the error:
	ERROR 1452 (23000): Cannot add or update a child row: a foreign key constraint fails (., CONSTRAINT  FOREIGN KEY () REFERENCES  () ON DELETE SET NULL ON UPDATE CASCADE)
6) Run the added query in the patch:
	UPDATE illrequests SET borrowernumber = NULL WHERE borrowernumber NOT IN (SELECT borrowernumber FROM borrowers);
7) Repeat step 4)
8) Notice there's no errors
Comment 2 Pedro Amorim 2025-06-03 11:22:05 UTC
We've had this come up twice now on some of our upgrades.
Some databases have very old ILL requests where 'borrowernumber' has a value of a borrowernumber that doesn't exist. I don't know exactly how the data ended up this way, but it's happened twice now so this db revision fix is needed.
Comment 3 David Nind 2025-06-05 00:34:46 UTC
I get this error with step 2:

INSERT INTO illrequests (, , ) VALUES ('Standard', '99999', 'CPL');
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ' , ) VALUES ('Standard', '99999', 'CPL')' at line 1
Comment 4 Pedro Amorim 2025-06-05 10:34:41 UTC
Created attachment 182988 [details] [review]
Bug 40057: Fix db revision

Test plan (ktd):
1) reset_all
2) Run first part of the 241200017.pl atomicupdate manually
	koha-mysql kohadev
	ALTER TABLE illrequests DROP FOREIGN KEY illrequests_bnfk;
3) Create an ILL request linked to a borrower that doesn't exist (simulate old data):
	INSERT INTO illrequests (`backend`, `borrowernumber`,`branchcode` ) VALUES ('Standard', '99999', 'CPL');
4) Run the 2nd part of the atomicupdate manually:
	ALTER TABLE illrequests ADD CONSTRAINT illrequests_bnfk
	FOREIGN KEY()
	REFERENCES  ()
	ON DELETE SET NULL ON UPDATE CASCADE;
5) Notice you get the error:
	ERROR 1452 (23000): Cannot add or update a child row: a foreign key constraint fails (., CONSTRAINT  FOREIGN KEY () REFERENCES  () ON DELETE SET NULL ON UPDATE CASCADE)
6) Run the added query in the patch:
	UPDATE illrequests SET borrowernumber = NULL WHERE borrowernumber NOT IN (SELECT borrowernumber FROM borrowers);
7) Repeat step 4)
8) Notice there's no errors
Comment 5 Pedro Amorim 2025-06-05 10:35:16 UTC
(In reply to David Nind from comment #3)
> I get this error with step 2:
> 
> INSERT INTO illrequests (, , ) VALUES ('Standard', '99999', 'CPL');
> ERROR 1064 (42000): You have an error in your SQL syntax; check the manual
> that corresponds to your MariaDB server version for the right syntax to use
> near ' , ) VALUES ('Standard', '99999', 'CPL')' at line 1

Thank you David. It appears the column names got filtered out of the commit message! I've corrected and resubmitted.
Comment 6 David Nind 2025-06-05 21:49:28 UTC
Created attachment 183013 [details] [review]
Bug 40057: Fix db revision

Test plan (ktd):
1) reset_all
2) Run first part of the 241200017.pl atomicupdate manually
	koha-mysql kohadev
	ALTER TABLE illrequests DROP FOREIGN KEY illrequests_bnfk;
3) Create an ILL request linked to a borrower that doesn't exist (simulate old data):
	INSERT INTO illrequests (`backend`, `borrowernumber`,`branchcode` ) VALUES ('Standard', '99999', 'CPL');
4) Run the 2nd part of the atomicupdate manually:
   ALTER TABLE illrequests ADD CONSTRAINT illrequests_bnfk FOREIGN KEY(`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE;
5) Notice you get the error:
	ERROR 1452 (23000): Cannot add or update a child row: a foreign key constraint fails (., CONSTRAINT  FOREIGN KEY () REFERENCES  () ON DELETE SET NULL ON UPDATE CASCADE)
6) Run the added query in the patch:
	UPDATE illrequests SET borrowernumber = NULL WHERE borrowernumber NOT IN (SELECT borrowernumber FROM borrowers);
7) Repeat step 4)
8) Notice there's no errors

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2025-06-05 21:53:28 UTC
(In reply to Pedro Amorim from comment #5)
> (In reply to David Nind from comment #3)
> > I get this error with step 2:
> > 
> > INSERT INTO illrequests (, , ) VALUES ('Standard', '99999', 'CPL');
> > ERROR 1064 (42000): You have an error in your SQL syntax; check the manual
> > that corresponds to your MariaDB server version for the right syntax to use
> > near ' , ) VALUES ('Standard', '99999', 'CPL')' at line 1
> 
> Thank you David. It appears the column names got filtered out of the commit
> message! I've corrected and resubmitted.

Thanks Pedro!

I amended step 4 of the patch commit message, as the columns were missing as well.

Testing notes (using KTD):
1. After running through the test plan, I applied the patch and added my sign-off.
Comment 8 Marcel de Rooy 2025-06-06 06:40:52 UTC
Created attachment 183038 [details] [review]
Bug 40057: Fix db revision

Test plan (ktd):
1) reset_all
2) Run first part of the 241200017.pl atomicupdate manually
	koha-mysql kohadev
	ALTER TABLE illrequests DROP FOREIGN KEY illrequests_bnfk;
3) Create an ILL request linked to a borrower that doesn't exist (simulate old data):
	INSERT INTO illrequests (`backend`, `borrowernumber`,`branchcode` ) VALUES ('Standard', '99999', 'CPL');
4) Run the 2nd part of the atomicupdate manually:
   ALTER TABLE illrequests ADD CONSTRAINT illrequests_bnfk FOREIGN KEY(`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE;
5) Notice you get the error:
	ERROR 1452 (23000): Cannot add or update a child row: a foreign key constraint fails (., CONSTRAINT  FOREIGN KEY () REFERENCES  () ON DELETE SET NULL ON UPDATE CASCADE)
6) Run the added query in the patch:
	UPDATE illrequests SET borrowernumber = NULL WHERE borrowernumber NOT IN (SELECT borrowernumber FROM borrowers);
7) Repeat step 4)
8) Notice there's no errors

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2025-06-06 06:41:11 UTC
Just curious
SELECT COUNT(*) FROM illrequests WHERE borrowernumber NOT IN (SELECT borrowernumber FROM borrowers)
UPDATE illrequests SET borrowernumber = NULL WHERE borrowernumber NOT IN (SELECT borrowernumber FROM borrowers)
Wouldnt a join on borrowers with a NULL test be better? No big deal.
Comment 10 Lucas Gass (lukeg) 2025-06-06 15:46:18 UTC
Nice work everyone!

Pushed to main for 25.11