Bug 40057 - Database update 24.12.00.017 fails if old ILL data points to non-existent borrowernumber
Summary: Database update 24.12.00.017 fails if old ILL data points to non-existent bor...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Pedro Amorim
QA Contact: Marcel de Rooy
URL:
Keywords: rel_25_05_candidate
Depends on: 32630
Blocks:
  Show dependency treegraph
 
Reported: 2025-06-03 11:16 UTC by Pedro Amorim
Modified: 2025-07-09 14:02 UTC (History)
7 users (show)

See Also:
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,25.05.01,24.11.07
Circulation function:


Attachments
Bug 40057: Fix db revision (2.26 KB, patch)
2025-06-03 11:20 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 40057: Fix db revision (2.30 KB, patch)
2025-06-05 10:34 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 40057: Fix db revision (2.39 KB, patch)
2025-06-05 21:49 UTC, David Nind
Details | Diff | Splinter Review
Bug 40057: Fix db revision (2.48 KB, patch)
2025-06-06 06:40 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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
Comment 11 Jonathan Druart 2025-06-19 10:32:56 UTC
(In reply to Marcel de Rooy from comment #9)
> 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.

I came here to say that.

This is really not the best we can do.

The following query using LEFT JOIN should work:

UPDATE illrequests AS ill
LEFT JOIN borrowers AS b ON ill.borrowernumber = b.borrowernumber
SET ill.borrowernumber = NULL
WHERE b.borrowernumber IS NULL;
Comment 12 Paul Derscheid 2025-06-22 21:47:25 UTC
Nice work everyone!

Pushed to 25.05.x for 25.05.03
Comment 13 Baptiste Wojtkowski (bwoj) 2025-07-09 14:02:40 UTC
Pushed to 24.11.x for 24.11.07