Bug 27534 - koha upgrade throws SQL error while applying Bug 25333 - Change message transport type for Talking Tech from "phone" to "itiva"
Summary: koha upgrade throws SQL error while applying Bug 25333 - Change message trans...
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 25333 9016
Blocks:
  Show dependency treegraph
 
Reported: 2021-01-23 10:15 UTC by Manos PETRIDIS
Modified: 2021-02-25 09:29 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.04


Attachments
Bug 27534: Add missing ON UPDATE/DELETE clause to message_transport_type_fk (2.07 KB, patch)
2021-01-27 08:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27534: Add missing ON UPDATE/DELETE clause to message_transport_type_fk (2.14 KB, patch)
2021-02-23 08:57 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Manos PETRIDIS 2021-01-23 10:15:36 UTC
Noticed the following error while upgrading to 20.11.01.000:
 
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_usmarc`.`letter`, CONSTRAINT `message_transport_type_fk` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`)) [for Statement "
        UPDATE message_transport_types SET message_transport_type = "itiva" WHERE message_transport_type = "phone"
    "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 23192.
Upgrade to 20.06.00.058 done [00:14:07]: Bug 25333 - Change message transport type for Talking Tech from "phone" to "itiva"

Apparently it pertains to Bug: 25333 

Log follows:

root@koha:~# apt-get update
Ign:1 http://debian.otenet.gr/debian stretch InRelease
Hit:2 http://debian.otenet.gr/debian stretch Release                           
Ign:3 http://debian.noc.ntua.gr/debian stretch InRelease                       
Ign:4 http://deb.debian.org/debian stretch InRelease                           
Get:5 http://debian.noc.ntua.gr/debian stretch-updates InRelease [93.6 kB]     
Get:6 http://security.debian.org/debian-security stretch/updates InRelease [53.0 kB]
Hit:7 http://deb.debian.org/debian stretch Release                             
Hit:8 http://debian.noc.ntua.gr/debian stretch Release                         
Hit:10 http://debian.koha-community.org/koha stable InRelease                  
Get:12 http://security.debian.org/debian-security stretch/updates/main Sources [331 kB]
Hit:13 http://espejito.fder.edu.uy/mariadb/repo/10.4/debian stretch InRelease  
Get:14 http://security.debian.org/debian-security stretch/updates/main amd64 Packages [649 kB]
Fetched 1,127 kB in 2s (448 kB/s)                                              
Reading package lists... Done


root@koha:~# apt-get upgrade
Reading package lists... Done
Building dependency tree       
Reading state information... Done
Calculating upgrade... Done
The following packages were automatically installed and are no longer required:
  libcache-fastmmap-perl libpdf-api2-simple-perl libppi-perl
  libwww-youtube-download-perl libxml-treepp-perl tinymce
Use 'apt autoremove' to remove them.
The following packages have been kept back:
  libjson-validator-perl libmojolicious-perl
  libmojolicious-plugin-openapi-perl
0 upgraded, 0 newly installed, 0 to remove and 3 not upgraded.
1 not fully installed or removed.
After this operation, 0 B of additional disk space will be used.
Do you want to continue? [Y/n] y
Setting up koha-common (20.11.01-1) ...
Upgrading database schema for usmarc
Upgrade to 20.06.00.023 done [00:13:16]: Bug 16371 - Quote of the Day (QOTD) for the staff interface 
Upgrade to 20.06.00.024 done [00:13:17]: Bug 25867 - Update subfield descriptions for 952$a and 952$b
Upgrade to 20.06.00.025 done [00:13:17]: Bug  6725 - Adds PatronDuplicateMatchingAddFields system preference
Upgrade to 20.06.00.026 done [00:13:17]: Bug 19036 - Add accountlines.credit_number, account_credit_types.credit_number_enabled and syspref AutoCreditNumber
Upgrade to 20.06.00.027 done [00:13:17]: Bug  8732 - Add new BiblioItemtypeInfo to system preferences
Upgrade to 20.06.00.028 done [00:13:17]: Bug 25958 - Allow LongOverdue cron to exclude specified lost values
Upgrade to 20.06.00.029 done [00:13:17]: Bug 25534 - Add ability to send an email specifying a reason when canceling a hold
Upgrade to 20.06.00.030 done [00:13:17]: Bug 20057 - Add new system preference 'AutoApprovePatronProfileSettings'
Upgrade to 20.06.00.031 done [00:13:17]: Bug 22789 - Add non_priority column on reserves and old_reserves tables
Upgrade to 20.06.00.032 done [00:13:17]: Bug 19889 - Add exclude_from_local_holds_priority column to items, deleteditems and categories tables
Upgrade to 20.06.00.033 done [00:13:17]: Bug 21066 - Rename column opac_news.timestamp with published_on
						   - Add new column opac_news.updated_on
						   - Replace timestamp references in letters table
Upgrade to 20.06.00.034 done [00:13:17]: Bug 24197 - Add new system preference 'AddressForFailedOverdueNotices'
Upgrade to 20.06.00.035 done [00:13:17]: Bug 23682 - Add new system preference 'EdifactInvoiceImport'
Upgrade to 20.06.00.036 done [00:13:17]: Bug 20168 - Update OPACSearchForTitleIn to work with Bootstrap 4
Upgrade to 20.06.00.037 done [00:13:17]: Bug 23816 - Add min_password_length and require_strong_password columns in categories table
Upgrade to 20.06.00.038 done [00:13:18]: Bug 24807 - Add 'year' type to improve sorting behaviour
Upgrade to 20.06.00.039 done [00:13:18]: Bug 18958 - Add reserve_id to hold_fill_targets
Upgrade to 20.06.00.040 done [00:13:18]: Bug 26454 - Add system preference to set meta description for the OPAC
Upgrade to 20.06.00.041 done [00:13:18]: Bug 26268 - Remove items.paidfor field
Upgrade to 20.06.00.042 done [00:13:18]: Bug 25776 - Add letter.updated_on
Upgrade to 20.06.00.043 done [00:13:18]: Bug 25261 - Add CircConfirmItemParts syspref
Upgrade to 20.06.00.044 done [00:13:18]: Bug 22343 - Add SMTP configuration options
Upgrade to 20.06.00.045 done [00:13:18]: Bug 22417 - Add new table background_jobs
Upgrade to 20.06.00.046 done [00:13:18]: Bug 13535 - Add FK constraint on borrowernumber to alert table
Upgrade to 20.06.00.047 done [00:13:18]: Bug 23420 - Allow configuration of hidden fields on the suggestion form in OPAC
Upgrade to 20.06.00.048 done [00:13:18]: Bug 26529 - Remove blank default branch rules
Upgrade to 20.06.00.049 done [00:14:04]: Bug 26145 - Add the biblioimages.itemnumber column
						   - Rename table biblioimages with cover_images
Upgrade to 20.06.00.050 done [00:14:04]: Bug 19382 - Add ability to block guarantees based on fees owed by guarantor and other guarantee - new system preference 'NoIssuesChargeGuarantorsWithGuarantees'
Upgrade to 20.06.00.051 done [00:14:04]: Bug 12556 - Add new syspref HoldsNeedProcessingSIP
Upgrade to 20.06.00.052 done [00:14:04]: Bug 25460 - Update OAI set when adding/editing/deleting item records
Upgrade to 20.06.00.053 done [00:14:04]: Bug 26569 - Use gender neutral pronouns in system preference explanations
Upgrade to 20.06.00.054 done [00:14:04]: Bug 25596 - Add OVERPAYMENT credit type
Upgrade to 20.06.00.055 done [00:14:05]: Bug 18050 - Add FK constraint on aqbudgets.budget_period_id
Upgrade to 20.06.00.056 done [00:14:07]: Bug 26853 - Update import_biblios columns and indexes
Upgrade to 20.06.00.057 done [00:14:07]: Bug 26638 - Add missing system preference ArticleRequestsMandatoryFieldsItemOnly
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_usmarc`.`letter`, CONSTRAINT `message_transport_type_fk` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`)) [for Statement "
        UPDATE message_transport_types SET message_transport_type = "itiva" WHERE message_transport_type = "phone"
    "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 23192.
Upgrade to 20.06.00.058 done [00:14:07]: Bug 25333 - Change message transport type for Talking Tech from "phone" to "itiva"
Upgrade to 20.06.00.059 done [00:14:07]: Bug 19482 - Add mandatory column to search_field for ES mapping
Upgrade to 20.06.00.060 done [00:14:08]: Bug 25334 - Add generic 'phone' message transport type
Upgrade to 20.06.00.061 done [00:14:08]: Bug 24412 - Attach waiting reserve to desk
Upgrade to 20.06.00.062 done [00:14:08]: Bug 23091 - Update refund rules
Upgrade to 20.06.00.063 done [00:14:08]: Bug 14866 - Add decreaseloanholds circulation rule
Upgrade to 20.06.00.064 done [00:14:08]: Bug 24603 - Add CANCELLATION credit_type_code
Upgrade to 20.06.00.065 done [00:14:08]: Bug 23916 - Add new [old_]issues.issuer DB fields
						   - Add new syspref RecordStaffUserOnCheckout
Upgrade to 20.06.00.066 done [00:14:09]: Bug 22818 - Add ILL notices
Upgrade to 20.06.00.067 done [00:14:09]: Bug 20936 - Add new system preference OPACHoldsHistory
Upgrade to 20.06.00.068 done [00:14:09]: Bug 23019 - Add import_batch_profiles table and profile_id column in import_batches
Upgrade to 20.06.00.069 done [00:14:09]: Bug 24083 - Add circulation_rules 'unseen_renewals_allowed'
						   - Add issues.unseen_renewals & old_issues.unseen_renewals)
						   - Add new system preference UnseenRenewals
Upgrade to 20.11.00.000 done [00:14:09]: Koha 20.11.00 release
Upgrade to 20.11.01.000 done [00:14:09]: Koha 20.11.01 release
The following plugins have been enabled:
  amqp_client
  rabbitmq_stomp

Applying plugin configuration to rabbit@koha... started 2 plugins.
W: APT had planned for dpkg to do more than it reported back (0 vs 4).
   Affected packages: koha-common:amd64
root@koha:~#
Comment 1 Kyle M Hall 2021-01-25 12:09:31 UTC
It sounds like your database structure isn't quite right, unless it's a bug in your database software version.

I did the following to test and it worked:
UPDATE message_transport_types SET message_transport_type = "asdf" WHERE message_transport_type = "email"

Maybe your FK on `letter` is missing it's "ON UPDATE CASCADE"?
Comment 2 Kyle M Hall 2021-01-25 12:10:14 UTC
This is, 'SHOW CREATE TABLE letter' should have this in it:
  CONSTRAINT `message_transport_type_fk` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`) ON DELETE CASCADE ON UPDATE CASCADE
Comment 3 Manos PETRIDIS 2021-01-27 07:01:17 UTC
I'm running mariadb 1:10.4.17+maria~stretch on Debian GNU/Linux 9.12 (stretch) with Linux 4.9.0-14-amd64 kernel. Pretty standard I understand.

'UPDATE message_transport_types SET message_transport_type = "asdf" WHERE message_transport_type = "email"' comes up with: 
08:45:26 Kernel error: Error( 1451 ) 23000: "Cannot delete or update a parent row: a foreign key constraint fails (`koha_usmarc`.`letter`, CONSTRAINT `message_transport_type_fk` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`))"


...while 'SHOW CREATE TABLE letter' produces:
INSERT INTO ``(
    `Table`,
    `Create Table`
) VALUES (
    'letter',
    'CREATE TABLE `letter` (
  `module` varchar(20) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '''',
  `code` varchar(20) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '''',
  `branchcode` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '''',
  `name` varchar(100) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '''',
  `is_html` tinyint(1) DEFAULT 0,
  `title` varchar(200) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '''',
  `content` mediumtext COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `message_transport_type` varchar(20) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT ''email'',
  `lang` varchar(25) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT ''default'',
  `updated_on` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
  PRIMARY KEY (`module`,`code`,`branchcode`,`message_transport_type`,`lang`),
  KEY `message_transport_type_fk` (`message_transport_type`),
  CONSTRAINT `message_transport_type_fk` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci'
 );

When were these FK and CONSTRAINT introduced in the database? In my case, upgrade steps begun with 'Upgrade to 20.06.00.023', therefore I gather that they were not [both] present at 20.06.00.022.
Comment 4 Jonathan Druart 2021-01-27 08:46:10 UTC
Created attachment 115848 [details] [review]
Bug 27534: Add missing ON UPDATE/DELETE clause to message_transport_type_fk

Bug 9016 (3.15.00.039) added message_transport_type_fk but the ON DELETE
CASCADE ON UPDATE CASCADE clause was missing from the updatedatabase
entry.
If we are upgrading from an old version, 20.06.00.058 will fail on
updating the message_transport_type, with
"Cannot delete or update a parent row: a foreign key constraint fails".
Comment 5 Jonathan Druart 2021-01-27 08:47:31 UTC
Manos, can you confirm you are upgrading from an old version (older than 3.15.00.039)?

This patch should fix the problem, there was missing the ON DELETE CASCADE ON UPDATE CASCADE part on the FK.
Comment 6 Manos PETRIDIS 2021-01-28 09:17:32 UTC
Indeed, my database begun its life in a 3.08 installation, if memory serves.
Comment 7 Jonathan Druart 2021-01-29 10:59:53 UTC
Manos, could you test this patch?
Comment 8 Manos PETRIDIS 2021-01-31 23:21:05 UTC
I don't think that I can test the patch Jonathan, as I'm not running a git installation, but one based on koha packages. 

I can report though that the mere addition of "ON DELETE CASCADE ON UPDATE CASCADE" won't do he trick in an existing db, as it comes up with Kernel error: Error( 1005 ) HY000: "Can't create table `koha_usmarc`.`letter` (errno: 121 "Duplicate key on write or update")"

What I have done is first
  ALTER TABLE letter drop CONSTRAINT message_transport_type_fk 

01:12:50 Query time: 91 millisecond(s)
01:12:50 Query has been executed


and then 
  ALTER TABLE letter ADD CONSTRAINT message_transport_type_fk FOREIGN KEY (message_transport_type) REFERENCES message_transport_types(message_transport_type) ON DELETE CASCADE ON UPDATE CASCADE

01:13:21 Query time: 518 millisecond(s), Number of affected records: 62
01:13:21 Number of affected rows: 62


Having done that, the failing update now works fine
  UPDATE message_transport_types SET message_transport_type = "asdf" WHERE message_transport_type = "email"

01:13:51 Query time: 223 millisecond(s), Number of affected records: 1
01:13:51 Number of affected rows: 1
01:13:51 UPDATE message_transport_types SET message_transport_type = "asdf" WHERE message_transport_type = "email"
Comment 9 Jonathan Druart 2021-02-02 10:15:14 UTC
Manos, it means you already have it in the table.

MariaDB [koha_kohadev]> select count(*) from letter where message_transport_type="phone";
+----------+
| count(*) |
+----------+
|        1 |
+----------+
1 row in set (0.000 sec)

MariaDB [koha_kohadev]> UPDATE message_transport_types SET message_transport_type = "xxx" WHERE message_transport_type = "phone";
Query OK, 1 row affected (0.019 sec)
Rows matched: 1  Changed: 1  Warnings: 0

MariaDB [koha_kohadev]> select count(*) from letter where message_transport_type="xxx";
+----------+
| count(*) |
+----------+
|        1 |
+----------+
1 row in set (0.002 sec)


That seems to work for me.
Comment 10 Manos PETRIDIS 2021-02-03 14:09:35 UTC
(In reply to Jonathan Druart from comment #9)

> Manos, it means you already have it in the table.
> 
> MariaDB [koha_kohadev]> select count(*) from letter where
> message_transport_type="phone";
> +----------+
> | count(*) |
> +----------+
> |        1 |
> +----------+
> 1 row in set (0.000 sec)
> 
> MariaDB [koha_kohadev]> UPDATE message_transport_types SET
> message_transport_type = "xxx" WHERE message_transport_type = "phone";
> Query OK, 1 row affected (0.019 sec)
> Rows matched: 1  Changed: 1  Warnings: 0
> 
> MariaDB [koha_kohadev]> select count(*) from letter where
> message_transport_type="xxx";
> +----------+
> | count(*) |
> +----------+
> |        1 |
> +----------+
> 1 row in set (0.002 sec)
> 
> 
> That seems to work for me.

The error originally reported was 

DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_usmarc`.`letter`, CONSTRAINT `message_transport_type_fk` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`)) [for Statement "
        UPDATE message_transport_types SET message_transport_type = "itiva" WHERE message_transport_type = "phone"
    "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 23192.
Upgrade to 20.06.00.058 done [00:14:07]: Bug 25333 - Change message transport type for Talking Tech from "phone" to "itiva"

... and Kyle HALL noted that it may be due to a missing "ON UPDATE CASCADE" on the `letter` table. I followed the test he suggested and verified that in my installation the update failed. 

If I read correctly the patch you supplied, you drop FOREIGN KEY message_transport_type_fk in letter table. As far as I know though, one doesn't need to drop the FOREIGN KEY altogether, just drop the CONSTRAINT and re-create it, including the missing CASCADE clause. 

I was then able to sucessufuly perform the test update 
UPDATE message_transport_types SET message_transport_type = "asdf" WHERE message_transport_type = "email" .

I understand that I have to run 
UPDATE message_transport_types SET message_transport_type = "itiva" WHERE message_transport_type = "phone"
so that the correction in Bug 25333 is performed, but apart from that, what is that I already have [some] table?
Comment 11 Jonathan Druart 2021-02-03 14:26:49 UTC
(In reply to Manos PETRIDIS from comment #10)
> If I read correctly the patch you supplied, you drop FOREIGN KEY
> message_transport_type_fk in letter table. As far as I know though, one
> doesn't need to drop the FOREIGN KEY altogether, just drop the CONSTRAINT
> and re-create it, including the missing CASCADE clause. 

It's exactly what the patch does :)
Comment 12 Jonathan Druart 2021-02-03 14:28:16 UTC
(In reply to Jonathan Druart from comment #11)
> (In reply to Manos PETRIDIS from comment #10)
> > If I read correctly the patch you supplied, you drop FOREIGN KEY
> > message_transport_type_fk in letter table. As far as I know though, one
> > doesn't need to drop the FOREIGN KEY altogether, just drop the CONSTRAINT
> > and re-create it, including the missing CASCADE clause. 
> 
> It's exactly what the patch does :)

It drops the FK and recreate it it with the correct CONSTRAINT. AFAIK it's not possible to do it without removing the FK.
Comment 13 Manos PETRIDIS 2021-02-03 14:56:14 UTC
I believe they may:

 ALTER TABLE letter DROP CONSTRAINT message_transport_type_fk 
 ALTER TABLE letter ADD CONSTRAINT message_transport_type_fk 
   FOREIGN KEY (message_transport_type) 
   REFERENCES message_transport_types(message_transport_type)    
   ON DELETE CASCADE ON UPDATE CASCADE

Generally speaking, a foreign key can be used for improved preformance (hint for implicit joins) even if no rules (constraints) are defined for data referential integrity. Foreign keys may well be defined without corresponding constraints, as it was apparently the case in older koha DBs. Constraints howerver need to be supported by indexes and foreign key definitions in order to enforce data integrity rules in time. 

In any case, all's well that ends well :-)
Comment 14 Martin Renvoize 2021-02-23 08:57:20 UTC
Created attachment 117176 [details] [review]
Bug 27534: Add missing ON UPDATE/DELETE clause to message_transport_type_fk

Bug 9016 (3.15.00.039) added message_transport_type_fk but the ON DELETE
CASCADE ON UPDATE CASCADE clause was missing from the updatedatabase
entry.
If we are upgrading from an old version, 20.06.00.058 will fail on
updating the message_transport_type, with
"Cannot delete or update a parent row: a foreign key constraint fails".

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2021-02-23 09:02:02 UTC
I've tested this with an old database that had the problem during upgrades.

The patch reads well and works as expected on MySQL 5.5, MariaDB and MySQL 8 in my tests.

Going for a straight QA here.

Passed
Comment 16 Jonathan Druart 2021-02-23 12:21:49 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 17 Fridolin Somers 2021-02-25 09:29:55 UTC
Pushed to 20.11.x for 20.11.04