Bug 34276 - upgrading 23.05 to 23.05.002
Summary: upgrading 23.05 to 23.05.002
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (command-line installer) (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low blocker (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
: 34268 34416 (view as bug list)
Depends on: 21983
Blocks:
  Show dependency treegraph
 
Reported: 2023-07-14 03:13 UTC by SOS
Modified: 2023-12-28 20:46 UTC (History)
11 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.04,22.11.10


Attachments
Bug 34276: (bug 21983 follow-up) Fix db rev 23.06.00.002 (2.95 KB, patch)
2023-08-30 15:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34276: (bug 21983 follow-up) Fix db rev 23.06.00.002 (3.01 KB, patch)
2023-08-31 14:02 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 34276: (bug 21983 follow-up) Fix db rev 23.06.00.002 (3.10 KB, patch)
2023-09-01 07:43 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 SOS 2023-07-14 03:13:06 UTC
Upgrade to 23.05.00.002 [16:25:11]: Bug 21983 - Deleted biblio handling on ILL
ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate key name 'illrequests_bibfk' at /usr/share/koha/lib/C4/Installer.pm line 741
Comment 1 Pedro Amorim 2023-07-14 08:44:28 UTC
Interesting, just to add some info, we have successfully upgraded some of our customers to 22.11.07 (that contains bug 21983) and the database upgrades have all been successful thus far.
Comment 2 Katrin Fischer 2023-07-14 19:34:51 UTC
Which version did each of you update from?
Comment 3 SOS 2023-07-15 00:03:40 UTC
23.05.00
Comment 4 Katrin Fischer 2023-07-16 10:25:42 UTC
As it says the key is already there, you could try and remove it with SQL then running the update again... that might be  a quick fix to get you out of that situation. We need to take a closer look what happened there.
Comment 5 SOS 2023-07-16 12:15:13 UTC
I'm layman could you please help me with the SQL queries and how to run it.
Comment 6 Pedro Amorim 2023-07-17 09:16:03 UTC
(In reply to Katrin Fischer from comment #2)
> Which version did each of you update from?

The updates I can confirm were successful were from 22.11.06. And I'm not aware of us having any issues from previous versions either. We're not using 23.05 yet.
Taking a quick look at this, I can't find any obvious issue.(In reply to SOS from comment #5)

> I'm layman could you please help me with the SQL queries and how to run it.

You could run 'SHOW CREATE TABLE illrequests' and verify it matches what's on kohastructure.sql

You could also run 
'SELECT * FROM illrequests WHERE biblio.biblionumber IS NULL AND illrequests.biblio_id IS NOT NULL'
Just to rule out that there aren't any dangling non-existent biblio_ids in your illrequests table.

If both the above checks out, maybe just removing the dbrev and doing the update again is an option? The update seems to be idempotent so I'm not sure what happened here.
Comment 7 SOS 2023-07-17 09:40:05 UTC
I have used below query to fix the issue and now its running fine:

ALTER TABLE illrequests DROP KEY illrequests_bibfk;
Comment 8 Katrin Fischer 2023-07-17 09:55:24 UTC
We went back and forth on the FK a bit, could it be the problem only arises of you are coming from 22.11.05?
Comment 9 Pedro Amorim 2023-07-17 10:06:51 UTC
Here's what I did:
1) checkout v22.11.05
2) reset_all
3) checkout master
4) updatedatabase

All fine, no issues.

Did the same test again but this time with a non-existent biblionumber in illrequests.biblio_id, in step 1). Still all good and the bilbio_id ended up in deleted_biblio_id as expected.

Nothing obvious stands out as for what would cause this.
Comment 10 Katrin Fischer 2023-07-17 11:59:18 UTC
(In reply to Pedro Amorim from comment #9)
> Here's what I did:
> 1) checkout v22.11.05
> 2) reset_all
> 3) checkout master
> 4) updatedatabase
> 
> All fine, no issues.
> 
> Did the same test again but this time with a non-existent biblionumber in
> illrequests.biblio_id, in step 1). Still all good and the bilbio_id ended up
> in deleted_biblio_id as expected.
> 
> Nothing obvious stands out as for what would cause this.

Thanks a lot Pedro! So maybe we can downgrade this one. 

SOS - do you remember any issues or anything unusual about your previous updates?

Which version was initially installed?
Comment 11 Jeremiah 2023-07-20 18:11:21 UTC
Hi,

I am having the same problem as SOS.

Upgrade to 23.05.00.002 [11:09:02]: Bug 21983 - Deleted biblio handling on ILL
ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate key name 'illrequests_bibfk' at /usr/share/koha/lib/C4/Installer.pm line 741

I am coming from 22.05 I believe.

I tried their suggestion: 
"ALTER TABLE illrequests DROP KEY illrequests_bibfk;"

I got: Cannot drop index 'illrequests_bibfk': needed in a foreign key constraint
Comment 12 SOS 2023-07-21 04:34:24 UTC
My query got resolved through ChatGPT instructions, kindly copy-paste the error and ask for solution, it will guide tofurther steps. Maybe it will ask you to drop Foreign key.
Comment 13 Jonathan Druart 2023-07-21 05:56:14 UTC
*** Bug 34268 has been marked as a duplicate of this bug. ***
Comment 14 Jonathan Druart 2023-07-21 05:57:31 UTC
Tomas, Pedro, can you take care of this one please?
Comment 15 Pedro Amorim 2023-07-24 08:36:47 UTC
(In reply to Jonathan Druart from comment #14)
> Tomas, Pedro, can you take care of this one please?

I'd love to, but I'm not sure where to go from here! I'm unable to reproduce this no matter what.
The atomicupdate looks idempotent enough to me, even if we do follow-up your suggestion JD, "we should add a separate check for the key (one for the FK, one for the key)." how do we make sure this has indeed fixed the issue?
Comment 16 Katrin Fischer 2023-07-26 18:12:42 UTC
*** Bug 34416 has been marked as a duplicate of this bug. ***
Comment 17 ivan 2023-07-26 18:52:48 UTC
(In reply to Jeremiah from comment #11)
> Hi,
> 
> I am having the same problem as SOS.
> 
> Upgrade to 23.05.00.002 [11:09:02]: Bug 21983 - Deleted biblio handling on
> ILL
> ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate key
> name 'illrequests_bibfk' at /usr/share/koha/lib/C4/Installer.pm line 741
> 
> I am coming from 22.05 I believe.
> 
> I tried their suggestion: 
> "ALTER TABLE illrequests DROP KEY illrequests_bibfk;"
> 
> I got: Cannot drop index 'illrequests_bibfk': needed in a foreign key
> constraint

Hi,

I tried this suggestion and i got the same:

Cannot drop index 'illrequests_bibfk': needed in a foreign key constraint

message... So i searched and droped the foreign key constraint first:

"ALTER TABLE illrequests DROP FOREIGN KEY fk_illrequests_books;"

( I got some help from ChatGPT :) )

And then droped the index as proposed:

"ALTER TABLE illrequests DROP KEY illrequests_bibfk;"

And then  it worked! but now i have the question of the consequences of dropping
the foreign key constraint. Will it impact on some transaction? I guess this is
a provisional fix. I appreciate any help you can provide on this matter.

Thanks!
Comment 18 Jonathan Druart 2023-07-27 08:09:39 UTC
(In reply to Pedro Amorim from comment #15)
> (In reply to Jonathan Druart from comment #14)
> > Tomas, Pedro, can you take care of this one please?
> 
> I'd love to, but I'm not sure where to go from here! I'm unable to reproduce
> this no matter what.
> The atomicupdate looks idempotent enough to me, even if we do follow-up your
> suggestion JD, "we should add a separate check for the key (one for the FK,
> one for the key)." how do we make sure this has indeed fixed the issue?

We need to test the different situations.
I would do:

if FK or key does not exist:
  if key exists:
    drop key
  if fk exists:
    drop fk
create fk
Comment 19 beardst 2023-08-03 19:48:18 UTC
I upgraded from 22.05 and came across this issue as well. Actually had 3 issues in the DB that I had to work through independently. For this one here is what worked, had to track down the FK that was blocking the drop of the illrequests_bibfk key

ALTER TABLE illrequests DROP FOREIGN KEY illrequests_ibfk_1;
ALTER TABLE illrequests DROP KEY illrequests_bibfk;

After that I was able to retry the webinstall and it went through. Ran this command to see if the FK constraint was recreated. This is also the command I used to find the FK in the first place

SELECT RefCons.constraint_schema, RefCons.table_name, RefCons.referenced_table_name, RefCons.constraint_name, KeyCol.column_name
FROM information_schema.referential_constraints RefCons
JOIN information_schema.key_column_usage KeyCol ON RefCons.constraint_schema = KeyCol.table_schema
     AND RefCons.table_name = KeyCol.table_name
     AND RefCons.constraint_name = KeyCol.constraint_name
WHERE RefCons.constraint_schema = 'DATABASE_NAME';
Comment 20 Mohd Hafiz Yusoff 2023-08-22 16:56:15 UTC
Same error upgrade from Koha version: 23.05.00.000

Upgrade to 23.05.00.002 [23:53:09]: Bug 21983 - Deleted biblio handling on ILL
ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate key name 'illrequests_bibfk' at /usr/share/koha/lib/C4/Installer.pm line 741
Comment 21 dcowens76 2023-08-24 14:39:12 UTC
Thanks, so reading that thread, I ran these two commands in MySQL:

mysql> ALTER TABLE illrequests DROP FOREIGN KEY illrequests_ibfk_1;
mysql> ALTER TABLE illrequests DROP KEY illrequests_bibfk;

I reran the upgrade script without error. However, the web installer script still shows up rather than the staff interface.
Comment 22 Fridolin Somers 2023-08-24 18:38:41 UTC
Mmm I do not reproduce with a fresh 23.05.00 install upgraded to 23.05.02

Ah I see Bug 22440 added db_revs/221200026.pl
which is exactly like db_revs/230500002.pl from Bug 21983

They both contain the test :
  foreign_key_exists( 'illrequests', 'illrequests_bibfk' )
Comment 23 Fridolin Somers 2023-08-24 18:47:28 UTC
Ah, there has been a change in kohastructure.sql and db_revs/221200026.pl
during 23.05.00 release :
https://git.koha-community.org/Koha-community/Koha/commit/919197f5171f64dbb76bc365399e0255e0bb8ae1
https://git.koha-community.org/Koha-community/Koha/commit/78cce24e579bbcae67e1bb49845d3022f794a276
Comment 24 Jonathan Druart 2023-08-30 15:01:41 UTC
What I am seeing is that the name of the FK constraint is not kept, and illrequests_ibfk_1 is created (instead of illrequests_bibfk).

The syntax is wrong. Patch coming.
Comment 25 Jonathan Druart 2023-08-30 15:02:15 UTC
Created attachment 154970 [details] [review]
Bug 34276: (bug 21983 follow-up) Fix db rev 23.06.00.002
Comment 26 Jonathan Druart 2023-08-30 15:08:51 UTC
(one) Test plan:
git checkout v22.11.05
reset_all
git checkout master
yes|git bz apply 34276
updatedatabase

# idempotence
koha-mysql kohadev -e 'update systempreferences set value="22.1100000" where variable="version"'
updatedatabase

=> No error!

koha-mysql kohadev -e 'show create table illrequests'

You must see that the FK is named 'illrequests_bibfk'

You should also try to break the upgrade at 23.06.00.002 with the wrong FK name illrequests_ibfk_1 and updatedatabase again.
Comment 27 Jonathan Druart 2023-08-30 15:10:24 UTC
Would be great to test for MariaDB as well.
Comment 28 Jonathan Druart 2023-08-30 15:10:37 UTC
And... remember me to write a test for the upgrade process...
Comment 29 Pedro Amorim 2023-08-31 14:01:23 UTC
(In reply to Jonathan Druart from comment #26)
> You should also try to break the upgrade at 23.06.00.002 with the wrong FK
> name illrequests_ibfk_1 and updatedatabase again.

How exactly, sorry, I don't follow.

Steps to reproduce original problem:

1) Run ktd with mysql8.0:
DB_IMAGE=mysql:8.0 ktd up
2) Do the following steps, on ktd:
git checkout v22.11.05
reset_all
git checkout master
updatedatabase
Verify it blows up with the error:
ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate key name 'illrequests_bibfk' at /kohadevbox/koha/C4/Installer.pm line 741

Ran the test plan for both mysql:8.0 (where the issue could be reproduced) and mariadb10.5 (the issue did not happen before).
Both successful.
Comment 30 Pedro Amorim 2023-08-31 14:02:11 UTC
Created attachment 155009 [details] [review]
Bug 34276: (bug 21983 follow-up) Fix db rev 23.06.00.002

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 31 Jonathan Druart 2023-08-31 14:13:52 UTC
(In reply to Pedro Amorim from comment #29)
> (In reply to Jonathan Druart from comment #26)
> > You should also try to break the upgrade at 23.06.00.002 with the wrong FK
> > name illrequests_ibfk_1 and updatedatabase again.
> 
> How exactly, sorry, I don't follow.

Hum, maybe it was wrong. But now wondering: if you run the original query in 230600002 you will end up with 'illrequests_ibfk_1'. Shouldn't we have another atomic update to rename it then?
Comment 32 tony-the-hawk 2023-08-31 14:33:28 UTC
(In reply to beardst from comment #19)
> I upgraded from 22.05 and came across this issue as well. Actually had 3
> issues in the DB that I had to work through independently. For this one here
> is what worked, had to track down the FK that was blocking the drop of the
> illrequests_bibfk key
> 
> ALTER TABLE illrequests DROP FOREIGN KEY illrequests_ibfk_1;
> ALTER TABLE illrequests DROP KEY illrequests_bibfk;
> 
> After that I was able to retry the webinstall and it went through. Ran this
> command to see if the FK constraint was recreated. This is also the command
> I used to find the FK in the first place
> 
> SELECT RefCons.constraint_schema, RefCons.table_name,
> RefCons.referenced_table_name, RefCons.constraint_name, KeyCol.column_name
> FROM information_schema.referential_constraints RefCons
> JOIN information_schema.key_column_usage KeyCol ON RefCons.constraint_schema
> = KeyCol.table_schema
>      AND RefCons.table_name = KeyCol.table_name
>      AND RefCons.constraint_name = KeyCol.constraint_name
> WHERE RefCons.constraint_schema = 'DATABASE_NAME';

After the two commands ALTER TABLE... I retried the web install and I had the following Update report :

Upgrade to 23.05.00.002 [16:30:40]: Bug 21983 - Deleted biblio handling on ILL
Added foreign key constraint 'illrequests.illrequests_bibfk'
Upgrade to 23.05.00.003 [16:30:40]: Bug 32478 - Remove usage of Koha::Config::SysPref->find since bypasses cache
Replace 'NULL' with 'null' in ItemsDeniedRenewal system preference
Clear system preference cache
Upgrade to 23.05.01.000 [16:30:40]: Koha 23.05.01 release
Upgrade to 23.05.01.001 [16:30:40]: Bug 33961 - Remove Offline circulation tool
Removed system preference 'AllowOfflineCirculation'
Upgrade to 23.05.01.002 [16:30:40]: Bug 33117 - Patron checkout is not able to find patrons if using a second surname or other name during the search
Added new system preference 'DefaultPatronSearchMethod'
Update errors :

Upgrade to 23.05.01.003 [16:30:41]: Bug 34029 - Extend datatypes in biblioitems and deletedbiblioitems tables to avoid import errors 
ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: BLOB/TEXT column 'publishercode' used in key specification without a key length at /usr/share/koha/lib/C4/Installer.pm line 741
Updated biblioitems.place to text 
ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: BLOB/TEXT column 'publishercode' used in key specification without a key length at /usr/share/koha/lib/C4/Installer.pm line 741
Updated deletedbiblioitems.place to text 
ERROR: {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: BLOB/TEXT column 'publishercode' used in key specification without a key length at /usr/share/koha/lib/C4/Installer.pm line 741
Comment 33 Pedro Amorim 2023-08-31 14:34:15 UTC
Yes that makes sense, currently, if we:
git checkout v22.11.05
reset_all
git checkout master
updatedatabase
koha-mysql kohadev -e 'show create table illrequests'

we get:

  KEY `illrequests_bnfk` (`borrowernumber`),
  KEY `illrequests_bcfk_2` (`branchcode`),
  KEY `illrequests_safk` (`status_alias`),
  KEY `illrequests_bibfk` (`biblio_id`),
  CONSTRAINT `illrequests_bcfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE,
  CONSTRAINT `illrequests_bnfk` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE,
  CONSTRAINT `illrequests_ibfk_1` FOREIGN KEY (`biblio_id`) REFERENCES `biblio` (`biblionumber`) ON DELETE SET NULL ON UPDATE CASCADE,
  CONSTRAINT `illrequests_safk` FOREIGN KEY (`status_alias`) REFERENCES `authorised_values` (`authorised_value`) ON DELETE SET NULL ON UPDATE CASCADE

So those who have already upgraded may or may not still have a illrequests_ibfk_1 lingering. Is this what you mean?
Comment 34 Jonathan Druart 2023-08-31 14:42:14 UTC
(In reply to Pedro Amorim from comment #33)
> So those who have already upgraded may or may not still have a
> illrequests_ibfk_1 lingering. Is this what you mean?

Yes!
Comment 35 Marcel de Rooy 2023-09-01 06:36:13 UTC
Note this change on bug 29390 (waiting in NSO since June):

 sub foreign_key_exists {
-    my ( $table_name, $constraint_name ) = @_;
+    my ( $table_name, $constraint_name, $field_name ) = @_;
     my $dbh = C4::Context->dbh;
     my (undef, $infos) = $dbh->selectrow_array(qq|SHOW CREATE TABLE $table_name|);
-    return $infos =~ m|CONSTRAINT `$constraint_name` FOREIGN KEY|;
+    return $infos =~ m|CONSTRAINT `$constraint_name` FOREIGN KEY| if $constraint_name;
+    return $infos =~ m|FOREIGN KEY \(`$field_name`\)| if $field_name;
 }

This allows me to check for a FK without the specific constraint name..
Comment 36 Marcel de Rooy 2023-09-01 06:44:51 UTC
 WARN   installer/data/mysql/db_revs/230600002.pl
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   SKIP   spelling
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 8, now: 16)
   OK     valid
Comment 37 Marcel de Rooy 2023-09-01 06:50:40 UTC
Looking here
Comment 38 Jonathan Druart 2023-09-01 07:37:41 UTC
(In reply to Jonathan Druart from comment #28)
> And... remember me to write a test for the upgrade process...

s/remember/remind

See bug 34683.
Comment 39 Marcel de Rooy 2023-09-01 07:40:27 UTC
Just mentioning: TYPO Add new /ill_requests endopoint
Comment 40 Marcel de Rooy 2023-09-01 07:43:01 UTC
Created attachment 155053 [details] [review]
Bug 34276: (bug 21983 follow-up) Fix db rev 23.06.00.002

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 41 Tomás Cohen Arazi 2023-09-01 14:10:31 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 42 dcowens76 2023-09-05 16:39:37 UTC
Thanks so much for working on this!
Comment 43 Fridolin Somers 2023-09-08 00:28:37 UTC
Pushed to 23.05.x for 23.05.04
Comment 44 Matt Blenkinsop 2023-09-08 09:50:18 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x