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: | |
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
[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.