Bug #46599 ALTER TABLE causes inconsistent values for foreign keys
Submitted: 7 Aug 2009 10:37 Modified: 3 Aug 2012 15:28
Reporter: Bogdan Degtyariov Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Storage Engine API Severity:S2 (Serious)
Version:5.0, 5.1 OS:Any
Assigned to: Assigned Account CPU Architecture:Any
Tags: ALTER TABLE, DEFAULT, foreign key, NOT NULL

[7 Aug 2009 10:37] Bogdan Degtyariov
Description:
ALTER TABLE causes inconsistent values for foreign keys when converting columns with NULL values to NOT NULL DEFAULT "" (empty string). After such conversion the FK column contains illegal empty string "" that is not present in the referenced table.

How to repeat:
--
-- CREATE THE REFERENCE TABLE AND INSERT DATA
--

create table ref_tab (id int auto_increment primary key, vc30 varchar(30), key vc30 (vc30)) engine=innodb;

insert into ref_tab (vc30) values ("test reference");

--
-- SELECT FROM REFERENCE TABLE SHOWS ONE ROW
--

select * from ref_tab;

-- +----+----------------+
-- | id | vc30           |
-- +----+----------------+
-- |  1 | test reference |
-- +----+----------------+

--
-- CREATE THE DATA TABLE WITH FK
--
create table data_tab (id int auto_increment primary key, vc30 varchar(30), foreign key (vc30) references ref_tab (vc30), txt text) engine=innodb;

-- INSERT A RECORD THAT REFERENCES ANOTHER RECORD IN ref_tab
insert into data_tab (vc30, txt) values ("test reference", "data text");

-- INSERT A RECORD WITH NULL
insert into data_tab (vc30, txt) values (NULL, "data text 2");

--
-- SELECT FROM REFERENCE TABLE SHOWS TWO ROWS
--

select * from data_tab;

-- +----+----------------+-------------+
-- | id | vc30           | txt         |
-- +----+----------------+-------------+
-- |  1 | test reference | data text   |
-- |  2 | NULL           | data text 2 |
-- +----+----------------+-------------+

--
-- ALTER THE FK COLUMN AS NOT NULL
--

ALTER TABLE data_tab MODIFY COLUMN `vc30` VARCHAR(30) NOT NULL DEFAULT "";

show warnings;

-- +---------+------+-------------------------------------------+
-- | Level   | Code | Message                                   |
-- +---------+------+-------------------------------------------+
-- | Warning | 1265 | Data truncated for column 'vc30' at row 2 |
-- +---------+------+-------------------------------------------+

--
-- THE ROW WITH id=2 SHOWS INCONSISTENT "" EMPTY STRING
--

select * from data_tab;

-- +----+----------------+-------------+
-- | id | vc30           | txt         |
-- +----+----------------+-------------+
-- |  1 | test reference | data text   |
-- |  2 |                | data text 2 |
-- +----+----------------+-------------+

Suggested fix:
Server should report an error similar to the following one:

mysql> insert into data_tab (vc30, txt) values ("", "data text 3");
ERROR 1452 (23000): Cannot add or update a child row: a foreign key constraint fails (`test/data_tab`, CONSTRAINT `data_tab_ibfk_1` FOREIGN KEY (`vc30`) REFERENCES `ref_tab` (`vc30`))
[7 Aug 2009 15:47] Bogdan Degtyariov
The same happens if set any non-NULL string as the DEFAULT value: all NULL records are converted to "".
[10 Aug 2009 14:05] Mikhail Izioumtchenko
Marko, could you have a look?
[11 Aug 2009 13:06] Marko Mäkelä
I am removing the assignment from me, because I believe that the fix requires changes to the storage engine API.

The foreign key constraint checks are being suppressed during the ALTER TABLE, because no foreign key constraints are copied for the temporary table that is to become the altered table. As far as I understand, the constraints will be reloaded from the InnoDB data dictionary, once the pre-ALTER table has been dropped and the temporary table has been renamed to the post-ALTER table.

Possibly the simplest fix is that we copy the foreign key information for the temporary table. That could severely reduce the performance of ALTER TABLE and OPTIMIZE TABLE. But then again, the checks could be skipped by SET SESSION foreign_key_checks=0 if needed.

Because the information on foreign key constraints is stored in the InnoDB data dictionary rather than the .frm file, mysql_alter_table() or copy_data_between_tables() in sql_table.cc cannot do copy the information  without asking InnoDB.

InnoDB creates foreign key information by parsing the SQL statement string–an ugly hack. It would be awkward and somewhat error-prone to pretty-print a CREATE TABLE statement for the post-ALTER table and ask InnoDB to process it.

Probably the cleanest solution would be to implement a storage engine interface for copying engine-specific metadata between tables. InnoDB would implement it by copying the foreign key constraints pointed to by the dict_table_t structures. It could even perform some consistency checks and return an error if the definition would be inconsistent. For example, when trying to drop an index that is needed by a foreign key constraint.
[12 Aug 2009 13:09] Marko Mäkelä
For the record, I repeated this bug with the following test case. The ALTER TABLE should not succeed, as it currently does.

-- source include/have_innodb.inc

CREATE TABLE bug46599a (
	b VARCHAR(2), KEY(b)) ENGINE=InnoDB;

CREATE TABLE bug46599b (b VARCHAR(2),
       FOREIGN KEY (b) REFERENCES bug46599a (b)) ENGINE=InnoDB;

INSERT INTO bug46599a VALUES ('b');
INSERT INTO bug46599b VALUES ('b'), (NULL);
--error ER_NO_REFERENCED_ROW_2
INSERT INTO bug46599b VALUES ('');

SELECT * FROM bug46599a;
SELECT * FROM bug46599b;

--error ER_NO_REFERENCED_ROW_2
ALTER TABLE bug46599b MODIFY COLUMN b VARCHAR(2) NOT NULL DEFAULT '';

SELECT * FROM bug46599b;

DROP TABLE bug46599b,bug46599a;
[15 Oct 2009 9:29] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/86912

2820 Anurag Shekhar	2009-10-15
      Bug #46599  ALTER TABLE causes inconsistent values for foreign keys
      
      While altering table foreign key constraint checks are suppressed which
      causes this behaviour. 
      To fix this problem modification in a field which is part of foreign 
      key is blocked. Work around to alter such field is to drop the key
      and recreate it after altering the column.
      A new error message is introduced which suggests uses to refer manual 
      for the work around.
     @ mysql-test/r/innodb_mysql.result
        updated result file.
     @ mysql-test/t/innodb_mysql.test
        test case for bug#46599.
     @ sql/share/errmsg.txt
        Added a new error message informing caller that column participating 
        in foreign key can't be modified.
     @ sql/sql_table.cc
        Added a new check in mysql_alter_table to check if the field being
        modified is part of a foreign key. An error ER_CANNOT_MODIFY_FOREIGN_KEY_FIELD
        is returned if it is.
[15 Oct 2009 9:58] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/86917

2820 Anurag Shekhar	2009-10-15
      Bug #46599  ALTER TABLE causes inconsistent values for foreign keys
      
      While altering table foreign key constraint checks are suppressed which
      causes this behaviour. 
      To fix this problem modification in a field which is part of foreign 
      key is blocked. Work around to alter such field is to drop the key
      and recreate it after altering the column.
      A new error message is introduced which suggests uses to refer manual 
      for the work around.
     @ mysql-test/r/innodb.result
        updated result file (new error message for one test case).
     @ mysql-test/r/innodb_mysql.result
        updated result file.
     @ mysql-test/t/innodb.test
        This patch blocks all modification in fields participating foreign 
        keys so error message for one case is different now.
     @ mysql-test/t/innodb_mysql.test
        Added test case for bug#46599.
     @ sql/share/errmsg.txt
        Added a new error message informing caller that column participating 
        in foreign key can't be modified.
     @ sql/sql_table.cc
        Added a new check in mysql_alter_table to check if the field being
        modified is part of a foreign key. An error ER_CANNOT_MODIFY_FOREIGN_KEY_FIELD
        is returned if it is.
[15 Oct 2009 10:00] Anurag Shekhar
Documentation note:

This fix blocks any modification in any field which is part of foreign key. The work around for the same is to drop the constraint first and recreate it after altering the table.
New Error message added advices user to refer documentation for the work around.
[15 Oct 2009 10:07] Marko Mäkelä
I would like to see a fix that communicates column changes (especially renames) to the storage engine. The patches do not address the case where you rename columns and then attempt to create a foreign key constraint on them. See Bug #44030, Bug #44571, Bug #45124.

Minor spelling nitpicking: "cann't" should be "cannot" or "can't", and " :" in the error message should be ": ".
[15 Oct 2009 11:26] Anurag Shekhar
Thanks Marko for looking at the patch and quick feedback.

Ideally the clean solution indeed will be to change SE API so that decision
about to raise error or allow changes in column is taken with consultation
with storage engines.
But that will make the fix quite complex with significant amount of changes
in SE API and also in innodb. The implementation of foreign key is anyway
going to handle this issue in a cleaner way.

Unfortunately this issue causes violation of constraint and needs to be addressed
right away.
[15 Oct 2009 11:36] Marko Mäkelä
Anurag, I agree that fixing the root cause will take its time and this kind of patches will be needed in the mean time. If there is a Worklog ticket for this work, it would be a good idea to link all related bugs there, so that those stop-gap measures that are rendered unnecessary by fixing the root cause can be reverted. (Some or all of them might have to remain, for compatibility reasons.)
[27 Oct 2009 20:59] Konstantin Osipov
I disagree that there is a need to communicate to the storage engine all the changes to fix this problem. 
The workaround for a user after Anurag's fix is to add DROP FOREIGN KEY, ADD FOREIGN KEY clauses to the ALTER specification. That will make sure that a) the ALTER can go through b) Foreign key constraint checks are performed.
In future we can do it implicitly and automatically, and thus allow such ALTER.
No need to consult with the engine.
[28 Oct 2009 11:17] Marko Mäkelä
Kostja, that would be a great idea. However, as far as I know, the FOREIGN KEY information is not available in the .frm file. MySQL would have to ask the storage engine to dump and drop the foreign key constraints before the ALTER, edit the info, and ask the storage engine to create the foreign key constraints. It should probably also lock the constrained tables, so that no constraint will be violated during the ALTER. (Not to mention that there currently is no interface for dropping or creating foreign key constraints. That would be a very welcome addition.)
[15 Oct 2010 12:07] Konstantin Osipov
Thoughs on this bug:

Since it's, although minor, incompatible change, it should be done
in 5.5. Another reason to look at the bug in 5.5 is that in 5.1
the storage engine API doesn't allow the sql/ layer to get a list
of foreign key relationships in which a table participates as a parent.

The patch itself is correct, but incomplete, namely:

1) ALTER should be allowed if @@foreign_key_checks=0
2) The code that checks whether ALTER is applicable should be
encapsulated into a function. It is recommended that the name of the function and error codes and messages it produces are taken from WL#148, since this worklog task has approved architecture and considers the issue in significant detail.
3) Foreign key relationships in which a table participates as a parent
should be taken into account.
4) Optionally (it would be nice), the check should allow simple ALTER statements, that can not lead to a foreign key constraint violation, namely: 
- change of DEFAULT
- altering NULLable columns to NOT NULL
5) The test case should demonstrate that the workaround with ADD FOREIGN KEY, DROP FOREIGN KEY is operational. This workaround should be part of the changeset comment and added to the manual after this bug is fixed.
[15 Oct 2010 12:08] Konstantin Osipov
Anurag,
feel free to assign it over to Davi if you deem this necessary.
[3 Aug 2012 15:28] Paul DuBois
Noted in 5.6.7, 5.7.0 changelogs.

ALTER TABLE operations that changed a column definition could cause a
loss of referential integrity for columns in a foreign key.
[9 Apr 2013 16:50] Paul DuBois
Revised changelog entry:

Using ALTER TABLE to change the definition of a foreign key column
could cause a loss of referential integrity. For example, changing a
foreign key column that contained NULL values to be NOT NULL caused
the NULL values to be the empty string. Similarly, an ALTER TABLE
IGNORE that removed rows in a parent table could break referential
integrity. 

The server now prohibits changes to foreign key columns with the
potential to cause loss of referential integrity. A workaround is to 
use ALTER TABLE ... DROP FOREIGN KEY before changing the column
definition and ALTER TABLE ... ADD FOREIGN KEY afterward.
[9 Apr 2013 16:52] Paul DuBois
Also updated text regarding this issue in the ALTER TABLE section.
[14 Jul 2015 13:59] Laurynas Biveinis
Is bug 71508 duplicate?
[14 Jul 2015 16:25] Dmitry Lenev
Indeed, it is. Bug #71508 was marked as duplicate of this one.