Bug 30899 - Upgrade sometimes fails at "Upgrade to 21.11.05.004"
Summary: Upgrade sometimes fails at "Upgrade to 21.11.05.004"
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Marcel de Rooy
QA Contact: Fridolin Somers
URL:
Keywords:
Depends on: 30449
Blocks:
  Show dependency treegraph
 
Reported: 2022-06-03 09:20 UTC by Magnus Enger
Modified: 2022-07-27 09:16 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This database revision fixes the one from bug 30449 for table borrower_attribute_types.
Version(s) released in:
22.11.00,22.05.01


Attachments
Bug 30899: [MASTER 22.12] Correct dbrev 211200041 (2.10 KB, patch)
2022-06-07 14:25 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30899: [22.05.x] Follow-up for dbrev 211200041 from bug 30449 (4.29 KB, patch)
2022-06-07 14:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30899: [21.11.x] Follow-up for dbrev 211200041 from bug 30449 (4.29 KB, patch)
2022-06-07 14:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30899: [MASTER 22.12] Correct dbrev 211200041 (2.12 KB, patch)
2022-06-08 07:17 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2022-06-03 09:20:35 UTC
I just ran a Debian package upgrade on a test server, from 21.11.04-2 to 21.11.06-1. Some of the upgrades ran just fine, while others stopped at 21.11.05.004:

...
Upgrade to 21.11.05.003  [10:59:51]: Bug 30565 - Update table stockrotationrotas
Upgrade to 21.11.05.004  [10:59:51]: Bug 30449 - Check borrower_attribute_types FK constraint
ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: 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 'CONSTRAINT category_code_fk' at line 1 at /usr/share/koha/lib/C4/Installer.pm line 739
Upgrading database schema for <next site>
...

The server has 79 instances on the server, and 19 of those were affected by this problem. So far I can not really see a pattern based on the age of the sites, or similar things.
Comment 1 Magnus Enger 2022-06-03 09:23:39 UTC
Looks like the problem is on line 11 of /usr/share/koha/intranet/cgi-bin/installer/data/mysql/db_revs/211105004.pl:

 10         if( foreign_key_exists('borrower_attribute_types', 'category_code_fk') ) {
 11             $dbh->do( q|ALTER TABLE borrower_attribute_types DROP CONSTRAINT category_code_fk| );
 12             if( index_exists('borrower_attribute_types', 'category_code_fk') ) {
 13                 $dbh->do( q|ALTER TABLE borrower_attribute_types DROP INDEX category_code_fk| );
 14             }
 15         }
Comment 2 Magnus Enger 2022-06-03 09:27:01 UTC
This is from a database where the update succeeded: 

MariaDB [koha_ok]> show create table borrower_attribute_types\G
*************************** 1. row ***************************
       Table: borrower_attribute_types
Create Table: CREATE TABLE `borrower_attribute_types` (
  `code` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL,
  `description` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL,
  `repeatable` tinyint(1) NOT NULL DEFAULT '0',
  `unique_id` tinyint(1) NOT NULL DEFAULT '0',
  `opac_display` tinyint(1) NOT NULL DEFAULT '0',
  `opac_editable` tinyint(1) NOT NULL DEFAULT '0',
  `staff_searchable` tinyint(1) NOT NULL DEFAULT '0',
  `authorised_value_category` varchar(32) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `display_checkout` tinyint(1) NOT NULL DEFAULT '0',
  `category_code` varchar(10) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `class` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',
  `keep_for_pseudonymization` tinyint(1) NOT NULL DEFAULT '0',
  `mandatory` tinyint(1) NOT NULL DEFAULT '0',
  PRIMARY KEY (`code`),
  KEY `auth_val_cat_idx` (`authorised_value_category`),
  KEY `category_code` (`category_code`),
  CONSTRAINT `borrower_attribute_types_ibfk_1` FOREIGN KEY (`category_code`) REFERENCES `categories` (`categorycode`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci
1 row in set (0.00 sec)

And this is from a database where it failed:

MariaDB [koha_fail]> show create table borrower_attribute_types\G
*************************** 1. row ***************************
       Table: borrower_attribute_types
Create Table: CREATE TABLE `borrower_attribute_types` (
  `code` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL,
  `description` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL,
  `repeatable` tinyint(1) NOT NULL DEFAULT '0',
  `unique_id` tinyint(1) NOT NULL DEFAULT '0',
  `opac_display` tinyint(1) NOT NULL DEFAULT '0',
  `opac_editable` tinyint(1) NOT NULL DEFAULT '0',
  `staff_searchable` tinyint(1) NOT NULL DEFAULT '0',
  `authorised_value_category` varchar(32) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `display_checkout` tinyint(1) NOT NULL DEFAULT '0',
  `category_code` varchar(10) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `class` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',
  `keep_for_pseudonymization` tinyint(1) NOT NULL DEFAULT '0',
  `mandatory` tinyint(1) NOT NULL DEFAULT '0',
  PRIMARY KEY (`code`),
  KEY `auth_val_cat_idx` (`authorised_value_category`),
  KEY `category_code_fk` (`category_code`),
  CONSTRAINT `category_code_fk` FOREIGN KEY (`category_code`) REFERENCES `categories` (`categorycode`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci
1 row in set (0.00 sec)
Comment 3 Magnus Enger 2022-06-03 09:31:07 UTC
This is from the site that succeeded, before the upgrade (taken from last night's backup):

CREATE TABLE `borrower_attribute_types` (
  `code` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL,
  `description` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL,
  `repeatable` tinyint(1) NOT NULL DEFAULT '0', 
  `unique_id` tinyint(1) NOT NULL DEFAULT '0', 
  `opac_display` tinyint(1) NOT NULL DEFAULT '0', 
  `opac_editable` tinyint(1) NOT NULL DEFAULT '0', 
  `staff_searchable` tinyint(1) NOT NULL DEFAULT '0', 
  `authorised_value_category` varchar(32) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `display_checkout` tinyint(1) NOT NULL DEFAULT '0', 
  `category_code` varchar(10) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `class` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',
  `keep_for_pseudonymization` tinyint(1) NOT NULL DEFAULT '0', 
  `mandatory` tinyint(1) NOT NULL DEFAULT '0', 
  PRIMARY KEY (`code`),
  KEY `auth_val_cat_idx` (`authorised_value_category`),
  KEY `category_code_fk` (`category_code`),
  CONSTRAINT `category_code_fk` FOREIGN KEY (`category_code`) REFERENCES `categories` (`categorycode`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

And this is the same for the site that failed:

CREATE TABLE `borrower_attribute_types` (
  `code` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL,
  `description` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL,
  `repeatable` tinyint(1) NOT NULL DEFAULT '0', 
  `unique_id` tinyint(1) NOT NULL DEFAULT '0', 
  `opac_display` tinyint(1) NOT NULL DEFAULT '0', 
  `opac_editable` tinyint(1) NOT NULL DEFAULT '0', 
  `staff_searchable` tinyint(1) NOT NULL DEFAULT '0', 
  `authorised_value_category` varchar(32) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `display_checkout` tinyint(1) NOT NULL DEFAULT '0', 
  `category_code` varchar(10) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `class` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',
  `keep_for_pseudonymization` tinyint(1) NOT NULL DEFAULT '0', 
  `mandatory` tinyint(1) NOT NULL DEFAULT '0', 
  PRIMARY KEY (`code`),
  KEY `auth_val_cat_idx` (`authorised_value_category`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
Comment 4 Jonathan Druart 2022-06-03 09:32:02 UTC
It's because you have a FK with the same name "category_code_fk"
Comment 5 Fridolin Somers 2022-06-03 09:39:58 UTC
I see upgrade 3.07.00.034 adds :
ALTER TABLE borrower_attribute_types ADD CONSTRAINT category_code_fk FOREIGN KEY (category_code) REFERENCES categories(categorycode)
Comment 6 Fridolin Somers 2022-06-03 09:44:56 UTC
This is from Bug 7154 that did not add the constraint in kohastructure.sql
Comment 7 Magnus Enger 2022-06-03 09:52:02 UTC
The relevant code in the database update is:

if( foreign_key_exists('borrower_attribute_types', 'category_code_fk') ) {
  $dbh->do( q|ALTER TABLE borrower_attribute_types DROP CONSTRAINT category_code_fk| );
  ...
}

The database that succeeds had this before the upgrade: 

  PRIMARY KEY (`code`),
  KEY `auth_val_cat_idx` (`authorised_value_category`),
  KEY `category_code_fk` (`category_code`),
  CONSTRAINT `category_code_fk` FOREIGN KEY (`category_code`) REFERENCES `categories` (`categorycode`)

And the one that fails had:

  PRIMARY KEY (`code`),
  KEY `auth_val_cat_idx` (`authorised_value_category`)

C4::Installer::foreign_key_exists looks like this

 648 sub foreign_key_exists {
 649     my ( $table_name, $constraint_name ) = @_;
 650     my $dbh = C4::Context->dbh;
 651     my (undef, $infos) = $dbh->selectrow_array(qq|SHOW CREATE TABLE $table_name|);
 652     return $infos =~ m|CONSTRAINT `$constraint_name` FOREIGN KEY|;
 653 }

Looks to me like the "if" above should prevent the "ALTER TABLE borrower_attribute_types DROP CONSTRAINT category_code_fk" from being run against the database at all?
Comment 8 Fridolin Somers 2022-06-03 09:56:59 UTC
(In reply to Magnus Enger from comment #1)
> Looks like the problem is on line 11 of
> /usr/share/koha/intranet/cgi-bin/installer/data/mysql/db_revs/211105004.pl:
> 
>  10         if( foreign_key_exists('borrower_attribute_types',
> 'category_code_fk') ) {
>  11             $dbh->do( q|ALTER TABLE borrower_attribute_types DROP
> CONSTRAINT category_code_fk| );
>  12             if( index_exists('borrower_attribute_types',
> 'category_code_fk') ) {
>  13                 $dbh->do( q|ALTER TABLE borrower_attribute_types DROP
> INDEX category_code_fk| );
>  14             }
>  15         }

Ahhh, the syntax is not :
  DROP CONSTRAINT category_code_fk
it is :
  DROP FOREIGN KEY category_code_fk
Comment 10 Magnus Enger 2022-06-03 10:24:18 UTC
If I change this line in the database update: 

  $dbh->do( q|ALTER TABLE borrower_attribute_types DROP CONSTRAINT category_code_fk| );

to this:

  $dbh->do( q|ALTER TABLE borrower_attribute_types DROP FOREIGN KEY category_code_fk| );

the update completes: 

  $ sudo koha-upgrade-schema koha
  Upgrading database schema for koha
  Upgrade to 21.11.05.004  [12:19:49]: Bug 30449 - Check borrower_attribute_types FK constraint
  Upgrade to 21.11.05.005  [12:19:49]: Bug 30572 - Adjust search_marc_to_field.sort
  Upgrade to 21.11.06.000  [12:19:49]: Koha 21.11.06 release

And after this the table looks like this: 

MariaDB [koha_esme]> SHOW CREATE TABLE borrower_attribute_types\G
*************************** 1. row ***************************
       Table: borrower_attribute_types
Create Table: CREATE TABLE `borrower_attribute_types` (
  `code` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL,
  `description` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL,
  `repeatable` tinyint(1) NOT NULL DEFAULT '0',
  `unique_id` tinyint(1) NOT NULL DEFAULT '0',
  `opac_display` tinyint(1) NOT NULL DEFAULT '0',
  `opac_editable` tinyint(1) NOT NULL DEFAULT '0',
  `staff_searchable` tinyint(1) NOT NULL DEFAULT '0',
  `authorised_value_category` varchar(32) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `display_checkout` tinyint(1) NOT NULL DEFAULT '0',
  `category_code` varchar(10) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `class` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',
  `keep_for_pseudonymization` tinyint(1) NOT NULL DEFAULT '0',
  `mandatory` tinyint(1) NOT NULL DEFAULT '0',
  PRIMARY KEY (`code`),
  KEY `auth_val_cat_idx` (`authorised_value_category`),
  KEY `category_code` (`category_code`),
  CONSTRAINT `borrower_attribute_types_ibfk_1` FOREIGN KEY (`category_code`) REFERENCES `categories` (`categorycode`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci
1 row in set (0.00 sec)

As far as I can see, this is the same as for the database where the upgrade succeeded in the first place.
Comment 11 Marcel de Rooy 2022-06-07 14:25:57 UTC
Created attachment 135779 [details] [review]
Bug 30899: [MASTER 22.12] Correct dbrev 211200041

Replace DROP CONSTRAINT by DROP FOREIGN KEY.

Note: It seems that we do not need a new dbrev for master. A developer
could follow test plan below to correct if really needed.

Test plan:

NOTE: When you check show create table, verify that you see an index for category_code and a foreign key called borrower_attribute_types_ibfk_1.

[1] cp installer/data/mysql/db_revs/211200041.pl installer/data/mysql/atomicupdate/
[2] run updatedatabase; check show create table
[3] alter table borrower_attribute_types drop foreign key borrower_attribute_types_ibfk_1; alter table borrower_attribute_types drop index category_code; RUN updatedatabase again; check show create table
[4] alter table borrower_attribute_types drop foreign key borrower_attribute_types_ibfk_1; ALTER TABLE borrower_attribute_types ADD CONSTRAINT category_code_fk FOREIGN KEY (category_code) REFERENCES categories(categorycode); RUN updatedatabase again; check show create table
[5] rm installer/data/atomicupdate/211200041.pl
Comment 12 Marcel de Rooy 2022-06-07 14:26:55 UTC
Still coming patches for 22.05 and 21.11
Comment 13 Marcel de Rooy 2022-06-07 14:38:13 UTC
Created attachment 135780 [details] [review]
Bug 30899: [22.05.x] Follow-up for dbrev 211200041 from bug 30449

Replace DROP CONSTRAINT by DROP FOREIGN KEY in old dbrev, and
submit a new dbrev to fix cases where dbrev went wrong.

Test plan:

NOTE: When you check show create table, verify that you see an index for category_code and a foreign key called borrower_attribute_types_ibfk_1.

[1] run updatedatabase; check show create table
[2] alter table borrower_attribute_types drop foreign key borrower_attribute_types_ibfk_1; alter table borrower_attribute_types drop index category_code; RUN updatedatabase again; check show create table
[3] alter table borrower_attribute_types drop foreign key borrower_attribute_types_ibfk_1; ALTER TABLE borrower_attribute_types ADD CONSTRAINT category_code_fk FOREIGN KEY (category_code) REFERENCES categories(categorycode); RUN updatedatabase again; check show create table
Comment 14 Marcel de Rooy 2022-06-07 14:50:34 UTC
Created attachment 135783 [details] [review]
Bug 30899: [21.11.x] Follow-up for dbrev 211200041 from bug 30449

Replace DROP CONSTRAINT by DROP FOREIGN KEY in old dbrev, and
submit a new dbrev to fix cases where dbrev went wrong.

Test plan:

NOTE: When you check show create table, verify that you see an index for category_code and a foreign key called borrower_attribute_types_ibfk_1.

[1] run updatedatabase; check show create table
[2] alter table borrower_attribute_types drop foreign key borrower_attribute_types_ibfk_1; alter table borrower_attribute_types drop index category_code; RUN updatedatabase again; check show create table
[3] alter table borrower_attribute_types drop foreign key borrower_attribute_types_ibfk_1; ALTER TABLE borrower_attribute_types ADD CONSTRAINT category_code_fk FOREIGN KEY (category_code) REFERENCES categories(categorycode); RUN updatedatabase again; check show create table
Comment 15 Marcel de Rooy 2022-06-07 14:54:24 UTC
Jonathan: Do you want to check this one, or straight to Tomas?
Comment 16 Jonathan Druart 2022-06-07 15:23:36 UTC
Magnus maybe, can you test this is working for you?
Comment 17 Marcel de Rooy 2022-06-08 06:10:00 UTC
(In reply to Jonathan Druart from comment #16)
> Magnus maybe, can you test this is working for you?

Comment10 confirms that more or less.
Comment 18 Fridolin Somers 2022-06-08 07:17:21 UTC
Created attachment 135790 [details] [review]
Bug 30899: [MASTER 22.12] Correct dbrev 211200041

Replace DROP CONSTRAINT by DROP FOREIGN KEY.

Note: It seems that we do not need a new dbrev for master. A developer
could follow test plan below to correct if really needed.

Test plan:

NOTE: When you check show create table, verify that you see an index for category_code and a foreign key called borrower_attribute_types_ibfk_1.

[1] cp installer/data/mysql/db_revs/211200041.pl installer/data/mysql/atomicupdate/
[2] run updatedatabase; check show create table
[3] alter table borrower_attribute_types drop foreign key borrower_attribute_types_ibfk_1; alter table borrower_attribute_types drop index category_code; RUN updatedatabase again; check show create table
[4] alter table borrower_attribute_types drop foreign key borrower_attribute_types_ibfk_1; ALTER TABLE borrower_attribute_types ADD CONSTRAINT category_code_fk FOREIGN KEY (category_code) REFERENCES categories(categorycode); RUN updatedatabase again; check show create table
[5] rm installer/data/atomicupdate/211200041.pl

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 19 Fridolin Somers 2022-06-08 07:18:13 UTC
@RM : remove [MASTER 22.12] when pushing
Comment 20 Tomás Cohen Arazi 2022-06-08 14:28:04 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 21 Lucas Gass 2022-06-24 16:01:18 UTC
Backported to 22.05.x for 22.05.01
Comment 22 victor 2022-07-05 01:27:04 UTC
Will the next updates fix this bug?
Or do you need to do the correction manually?
Comment 23 victor 2022-07-05 02:04:30 UTC
Complementing the information.
I saw this error when updating to version 22.05.02.000
Comment 24 victor 2022-07-06 01:11:13 UTC
In ubuntu 20.05 and Koha 22.05.02.000
$ koha-upgrade-schema faes
Upgrading database schema for faes
DEV atomic update /usr/share/koha/intranet/cgi-bin/installer/data/mysql/atomicupdate/bug_30899.pl  [22:09:37]: Bug 30899 - Check borrower_attribute_types FK constraint (30449 follow-up)
Comment 25 Jonathan Druart 2022-07-06 08:44:27 UTC
(In reply to victor from comment #24)
> In ubuntu 20.05 and Koha 22.05.02.000
> $ koha-upgrade-schema faes
> Upgrading database schema for faes
> DEV atomic update
> /usr/share/koha/intranet/cgi-bin/installer/data/mysql/atomicupdate/bug_30899.
> pl  [22:09:37]: Bug 30899 - Check borrower_attribute_types FK constraint
> (30449 follow-up)

Nothing wrong here. Your DB schema is up-to-date.

Lucas forgot to move a file from the atomicupdate directory to db_revs.
Comment 26 Fridolin Somers 2022-07-27 09:16:39 UTC
Fixed in 22.05.03 by :
e40fa1af65 Remove atomicupdate file