[MDEV-31086] MODIFY COLUMN can break FK constraints, and lead to unrestorable dumps Created: 2023-04-19  Updated: 2024-01-05  Resolved: 2023-06-27

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Affects Version/s: 10.11.1, 10.6.12, 10.8.7, 10.9.5, 10.10.3, 10.7
Fix Version/s: 10.4.31, 10.5.22, 10.6.15, 10.9.8, 10.10.6, 10.11.5, 11.0.3

Type: Bug Priority: Critical
Reporter: Hartmut Holzgraefe Assignee: Thirunarayanan Balathandayuthapani
Resolution: Fixed Votes: 2
Labels: foreign-keys

Issue Links:
Duplicate
duplicates MDEV-25619 Bogus ER_TABLE_EXISTS_ERROR upon an a... Closed
Problem/Incident
causes MDEV-31869 Server aborts when table does drop co... Closed
causes MDEV-31987 Cannot disable FOREIGN_KEY_CHECKS any... Closed
causes MDEV-32003 MODIFY COLUMN no longer possible with... Closed
causes MDEV-32018 Allow the setting of Auto_increment o... Closed
causes MDEV-32060 Server crashes in check_col_is_in_fk_... Closed
causes MDEV-32337 Assertion `pos < table->n_def' failed... Closed
causes MDEV-32527 Server aborts during alter operation ... Closed
causes MDEV-32638 MariaDB crashes with foreign_key_chec... Closed
Relates
relates to MDEV-32171 When updating cascaded foreign keys, ... Closed
relates to MDEV-25620 InnoDB: Failing assertion: id != 0 or... Closed
relates to MDEV-32729 look at SET FOREIGN_KEY_CHECK=0 Closed

 Description   

When creating a foreign key constraint the referenced columns on both sides need to have the same data types, including same character set. E.g.

CREATE TABLE t1(
 
  id SERIAL,
 
   msg VARCHAR(100) CHARACTER SET utf8mb3, 
  KEY(msg)
 
);
 
 
 
CREATE TABLE t2(
 
  id SERIAL,
 
  msg varchar(100) CHARACTER SET utf8mb4,
 
  CONSTRAINT fk_t1 FOREIGN KEY (msg) REFERENCES t1 (msg)
 
);

will fail with "Foreign key constraint is incorrectly formed" due to the mix of utf8mb3 and utf8mb4. Even with foreign_key_checks=OFF this will still not work.

Changing the character set to "utf8mb3" I can create the 2nd table now, as expected:

CREATE TABLE t2(
 
  id SERIAL,
 
  msg varchar(100) CHARACTER SET utf8mb3,
 
  CONSTRAINT fk_t1 FOREIGN KEY (msg) REFERENCES t1 (msg)
 
);

Now the problem is that starting with 10.6 I can change the character set "under the hood" without getting an error at all, regardless of foreign_key_checks being ON or OFF:

ALTER TABLE t2
 
  MODIFY COLUMN msg VARCHAR(100) CHARACTER SET utf8mb4;

With 10.5 and earlier the situation is a bit better, the MODIFY COLUMN will lead to

Cannot change column 'msg': used in a foreign key constraint 'fk_t1'

when foreign_key_checks are ON, but will succeed when checks are OFF; unlike the CREATE TABLE above that will fail regardless of checks being ON or OFF.

And with that we now have a table with a broken foreign key constraint, and a schema that we can dump with mysqldump, but not restore as the dump now contains a CREATE TABLE statement with a FK CONSTRAINT that will lead to an

Foreign key constraint is incorrectly formed

error even though mysqldump takes care to turn off foreign_key_checks for the duration of a restore.

So two things need to be fixed here IMHO:

  • on 10.6 or later: MODIFY TABLE definitely needs to fail if foreign_key_checks are ON
  • on all versions: with CREATE TABLE not allowing to create a FK constraint with column type mismatches regardless of foreign_key_checks, MODIFY COLUMN should not allow for this even with checks turned OFF either; or both should allow this when checks are OFF, for symmetry reasons

 Comments   
Comment by Hartmut Holzgraefe [ 2023-04-19 ]

MariaDB 10.5 and earlier correctly complain about "Foreign key constraint is incorrectly formed" on the MODIFY COLUMN attempt, it is only 10.6 and later that allow for this change to happen, and so to end up with an unrestorable dump

Comment by Hartmut Holzgraefe [ 2023-04-20 ]

I updated the description above a bit as there are actually two problems:

  • MODIFY COLUMN always possible in 10.6 or later
  • MODIFY COLUMN possible with checks OFF, even though CREATE TABLE not being possible even with checks disabled
Comment by Sergei Golubchik [ 2023-04-24 ]

That MODIFY COLUMN breaks an FK constraint under foreign_key_checks=ON in 10.6 is certainly a bug.

Behavior under foreign_key_checks=OFF — not necessarily

Comment by Marko Mäkelä [ 2023-04-24 ]

I think that this can be fixed by adding FOREIGN KEY checks to ha_innobase::can_convert_nocopy() or its callees.

Comment by Thirunarayanan Balathandayuthapani [ 2023-05-02 ]

I think we need to allow to change the column data type, column length and charset when foreign_key_checks is disabled. User
can modify the column in both parent and child table at the same time. Given patch does throw the error when foreign_key_checks
is enabled. I did write the procedure to show mismatch column in fk relationship.

--source include/have_innodb.inc
 
delimiter //
 
CREATE or REPLACE PROCEDURE fk_mismatch_col_new1()
 
BEGIN
 
CREATE TEMPORARY TABLE mismatch_fk_constraint(constraint_name varchar(200),foreign_schema varchar(200),foreign_table varchar(200), foreign_column varchar(200), referenced_schema varchar(200), referenced_table varchar(200),r_referenced_column varchar(200),r_COLUMN_TYPE longtext,r_CHARACTER_SET_NAME  varchar(100),remarks text)engine=innodb;
 
BEGIN
 
DECLARE constraint_name VARCHAR(200);
 
DECLARE foreign_schema VARCHAR(200);
 
DECLARE foreign_tbl_name VARCHAR(200);
 
DECLARE foreign_col VARCHAR(200);
 
DECLARE referenced_schema VARCHAR(200);
 
DECLARE referenced_tbl_name VARCHAR(200);
 
DECLARE referenced_col VARCHAR(200);
 
DECLARE foreign_tbl_id INTEGER;
 
DECLARE referenced_tbl_id INTEGER;
 
DECLARE f_col_len INTEGER;
 
DECLARE r_col_len INTEGER;
 
DECLARE f_prtype INTEGER;
 
DECLARE r_prtype INTEGER;
 
DECLARE done BOOLEAN DEFAULT false;
 
 
 
DECLARE curs CURSOR FOR
 
SELECT i.CONSTRAINT_NAME, i.table_schema, i.table_name, k.column_name, k.REFERENCED_TABLE_SCHEMA, k.REFERENCED_TABLE_NAME, k.REFERENCED_COLUMN_NAME FROM information_schema.TABLE_CONSTRAINTS i LEFT JOIN information_schema.KEY_COLUMN_USAGE k ON i.CONSTRAINT_NAME = k.CONSTRAINT_NAME WHERE i.CONSTRAINT_TYPE = 'FOREIGN KEY';
 
 
 
DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = true;
 
open curs;
 
label: LOOP
 
FETCH curs INTO constraint_name, foreign_schema, foreign_tbl_name,
 
foreign_col, referenced_schema, referenced_tbl_name, referenced_col;
 
 
 
IF done THEN LEAVE label; END IF;
 
 
 
SELECT table_id into @foreign_tbl_id FROM INFORMATION_SCHEMA.INNODB_SYS_TABLES WHERE name=concat(foreign_schema, "/", foreign_tbl_name);
 
 
 
SELECT table_id into @referenced_tbl_id FROM INFORMATION_SCHEMA.INNODB_SYS_TABLES WHERE name=concat(referenced_schema, "/", referenced_tbl_name);
 
 
 
SELECT LEN, PRTYPE into @f_col_len, @f_prtype FROM INFORMATION_SCHEMA.INNODB_SYS_COLUMNS WHERE table_id=@foreign_tbl_id and name=foreign_col;
 
 
 
SELECT LEN, PRTYPE into @r_col_len, @r_prtype FROM INFORMATION_SCHEMA.INNODB_SYS_COLUMNS WHERE table_id=@referenced_tbl_id and name=referenced_col;
 
 
 
if (@f_prtype != @r_prtype) then
 
 
 
INSERT INTO mismatch_fk_constraint values(
 
constraint_name, foreign_schema, foreign_tbl_name, foreign_col, referenced_schema, referenced_tbl_name, referenced_col,r_data_type, r_column_type, r_charset, "Column character set varies");
 
elseif (@f_col_len != @r_col_len) then
 
INSERT INTO mismatch_fk_constraint values(
 
constraint_name, foreign_schema, foreign_tbl_name, foreign_col, referenced_schema, referenced_tbl_name, referenced_col,r_data_type, r_column_type, r_charset, "Column length varies");
 
END IF;
 
 
 
END LOOP;
 
CLOSE curs;
 
END;
 
 
 
SELECT * FROM mismatch_fk_constraint;
 
DROP TABLE mismatch_fk_constraint;
 
end//
 
delimiter ;

I had discussion with pandi.gurusamy about stored procedure

Comment by Marko Mäkelä [ 2023-05-09 ]

I think that we must extend the test a little more and improve the check a little. I do not immediately see any problem if a FOREIGN KEY connected VARCHAR column is being extended, but the current fix would be prohibiting that.

When writing the test, please note that extending a VARCHAR from up to 127 bytes to at least 127 bytes can only be done instantly if ROW_FORMAT=REDUNDANT. See MDEV-16849 for some details.

Comment by Marko Mäkelä [ 2023-05-30 ]

As far as I understand, the first CREATE TABLE example in the Description would fail even if foreign_key_checks=OFF. To be consistent with that, a corresponding ALTER TABLE must fail too, regardless of whether ALGORITHM=COPY is being used.

Comment by Marko Mäkelä [ 2023-05-30 ]

It turns out in the SQL layer, the function fk_check_column_changes() would intentionally cause ALTER TABLE…ALGORITHM=COPY to ignore invalid foreign key constraints if foreign_key_checks=OFF. This would seem to be misguided, because a subsequent TRUNCATE TABLE or OPTIMIZE TABLE (if alter_algorithm=copy) and possibly ALTER TABLE…FORCE, ALGORITHM=COPY would be refused on the table. thiru, please fill in the details.

When foreign_key_checks=OFF, I think that it is reasonable to allow CREATE TABLE or ALTER TABLE to create FOREIGN KEY constraints that refer to non-existing tables. But I do not see the usefulness of allowing the creation of totally invalid foreign key constraints, that is, when columns are of incompatible type or use incompatible character encodings or collations.

I think a good guideline is that the output of SHOW CREATE TABLE after any number of DDL operations will be a valid CREATE TABLE statement if foreign_key_checks=OFF.

If someone wants to change the collation or encoding of a column that is connected to a FOREIGN KEY constraint, that can be done with 3 ALTER TABLE statements, something like this (untested SQL):

SET foreign_key_checks=OFF;
 
ALTER TABLE child DROP CONSTRAINT fk, MODIFY m VARCHAR(200) CHARSET utf8mb4;
 
ALTER TABLE parent MODIFY m VARCHAR(200) CHARSET utf8mb4;
 
ALTER TABLE child ADD CONSTRAINT FOREIGN KEY (m) REFERENCES PARENT(m);
 
SET foreign_key_checks=ON;

Disabling the checks will allow the ADD CONSTRAINT to be instantaneous; see also MDEV-16356.

Comment by Thirunarayanan Balathandayuthapani [ 2023-05-30 ] 

 
 
CREATE TABLE t1(
 
id SERIAL,
 
msg VARCHAR(100) CHARACTER SET utf8mb3,
 
KEY(msg))ENGINE=InnoDB;
 
 
 
CREATE TABLE t2(
 
id SERIAL,
 
msg varchar(100) CHARACTER SET utf8mb3,
 
INDEX (msg),
 
CONSTRAINT fk_t1 FOREIGN KEY (msg) REFERENCES t1 (msg)
 
ON DELETE CASCADE)ENGINE=InnoDB;
 
 
 
SET FOREIGN_KEY_CHECKS=0;
 
ALTER TABLE t2 MODIFY COLUMN msg VARCHAR(100) CHARACTER SET utf8mb4, algorithm=COPY;
 
 
 
SHOW CREATE TABLE t1;
 
Table	Create Table
 
t1	CREATE TABLE `t1` (
 
  `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
 
  `msg` varchar(100) CHARACTER SET utf8mb3 COLLATE utf8mb3_general_ci DEFAULT NULL,
 
  UNIQUE KEY `id` (`id`),
 
  KEY `msg` (`msg`)
 
) ENGINE=InnoDB DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
 
 
 
SHOW CREATE TABLE t2;
 
Table	Create Table
 
t2	CREATE TABLE `t2` (
 
  `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
 
  `msg` varchar(100) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci DEFAULT NULL,
 
  UNIQUE KEY `id` (`id`),
 
  KEY `msg` (`msg`),
 
  CONSTRAINT `fk_t1` FOREIGN KEY (`msg`) REFERENCES `t1` (`msg`) ON DELETE CASCADE
 
) ENGINE=InnoDB DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
 
 
 
SET FOREIGN_KEY_CHECKS=1;
 
 
 
TRUNCATE TABLE t2;
 
failed: ER_CANNOT_ADD_FOREIGN (1215): Cannot add foreign key constraint for `t2`
 
 
 
ALTER TABLE t2 FORCE, ALGORITHM=COPY;
 
failed: ER_ERROR_ON_RENAME (1025): Error on rename of './test/#sql-alter-1e9ed-4' to './test/t2' (errno: 150 "Foreign key constraint is incorrectly formed")

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

Thank you, it looks good. Once you have addressed my minor comments and this has passed testing, this will require another review of the change to fk_check_column_changes() (to make ALTER TABLE…ALGORITHM=COPY stricter).

Comment by Sergei Golubchik [ 2023-06-20 ]

thiru, server changes in f1746ad6dd are ok

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