[MDEV-32638] MariaDB crashes with foreign_key_checks=0 when changing a column and adding a foreign key at the same time Created: 2023-10-31  Updated: 2023-11-07  Resolved: 2023-11-02

Status: Closed
Project: MariaDB Server
Component/s: Data Definition - Alter Table, Storage Engine - InnoDB
Affects Version/s: 10.4, 10.5, 10.6.15, 10.9.8, 11.1.2
Fix Version/s: 10.4.33, 10.5.24, 10.6.17, 10.11.7, 11.0.5, 11.1.4, 11.2.3, 11.3.2

Type: Bug Priority: Critical
Reporter: Simon Williams Assignee: Thirunarayanan Balathandayuthapani
Resolution: Fixed Votes: 0
Labels: crash, foreign-keys, regression
Environment:

Docker on Linux Mint 20.3

Attachments: Text File marialog.txt     File migrate.sql    
Issue Links:
Problem/Incident
is caused by MDEV-31086 MODIFY COLUMN can break FK constraint... Closed

 Description    

docker run --rm -p 20433:3306 -e MARIADB_DATABASE=test -e MARIADB_ROOT_PASSWORD=root mariadb:11.1.2
 
mysql -h 127.0.0.1 --port=20433 -u root -proot test < migrate.sql

migrate.sql:

CREATE TABLE items (
 
    id VARCHAR(20) NOT NULL,
 
    PRIMARY KEY (id)
 
);
 
CREATE TABLE sgs_data (
 
    id INT(11) NOT NULL AUTO_INCREMENT,
 
    Number varchar(15) NOT NULL DEFAULT '',
 
    PRIMARY KEY (id)
 
);
 
SET foreign_key_checks=0;
 
ALTER TABLE sgs_data
 
    CHANGE COLUMN `Number` itemId VARCHAR(20) NOT NULL,
 
    ADD CONSTRAINT fk_sgs_data_itemId FOREIGN KEY (itemId) REFERENCES items (id);

This causes the MariaDB server to crash (see attached log output).

  • Moving the CHANGE COLUMN to a separate ALTER statement avoids the crash
  • Removing `SET foreign_key_checks=0` avoids the crash
  • Removing `CREATE TABLE items` still results in a crash even though the foreign key is invalid. If you additionally remove the `SET foreign_key_checks=0` it will instead report that the foreign key is malformed and not crash.

Verified on 10.6.15, 10.9.8 and 11.1.2 (all with docker).

 Comments   
Comment by Marko Mäkelä [ 2023-10-31 ]

This is something that was missed in MDEV-32527, MDEV-32337, MDEV-32060, MDEV-31869 and caused by the fix of MDEV-31086.

We fail to check for a null pointer:

diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
 
index 092aa54737d..6da95e42c11 100644
 
--- a/storage/innobase/handler/handler0alter.cc
 
+++ b/storage/innobase/handler/handler0alter.cc
 
@@ -7759,12 +7759,15 @@ bool check_col_is_in_fk_indexes(
 
 
   for (const auto &a : add_fk)
 
   {
 
-    for (ulint i= 0; i < a->n_fields; i++)
 
+    if (const dict_index_t* foreign_index= a->foreign_index)
 
     {
 
-      if (a->foreign_index->fields[i].col == col)
 
+      for (ulint i= 0; i < a->n_fields; i++)
 
       {
 
-        fk_id= a->id;
 
-        goto err_exit;
 
+        if (foreign_index->fields[i].col == col)
 
+        {
 
+          fk_id= a->id;
 
+          goto err_exit;
 
+        }
 
       }
 
     }
 
   }

Comment by Thirunarayanan Balathandayuthapani [ 2023-11-02 ]

https://github.com/MariaDB/server/pull/2814

Comment by Marko Mäkelä [ 2023-11-02 ]

OK to push.

Generated at Thu Feb 08 10:32:51 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.