1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

MDEV-32018 Allow the setting of Auto_increment on FK referenced columns

Browse Source 
In MDEV-31086, SET FOREIGN_KEY_CHECKS=0 cannot bypass checks that
make column types of foreign keys incompatible. An unfortunate
consequence is that adding an AUTO_INCREMENT is considered
incompatible in Field_{num,decimal}::is_equal and for the purpose
of FK checks this isn't relevant.

innodb.foreign_key - pragmaticly left wait_until_count_sessions.inc at
end of test to match the second line of test.

Reporter: horrockss@github - https://github.com/MariaDB/mariadb-docker/issues/528
Co-Author: Marko Mäkelä <marko.makela@mariadb.com>
Reviewer: Nikita Malyavin

For the future reader this was attempted:

Removing AUTO_INCREMENT checks from Field_{num,decimal}::is_equals
failed in the following locations (noted for future fixing):
* MyISAM and Aria (not InnoDB) don't adjust AUTO_INCREMENT next number
  correctly, hence added a test to main.auto_increment to catch
  the next person that attempts this fix.
* InnoDB must perform an ALGORITHM=COPY to populate NULL values of
  an original table (MDEV-19190 mtr test period.copy), this requires
  ALTER_STORED_COLUMN_TYPE to be set in fill_alter_inplace_info
  which doesn't get hit because field->is_equal is true.
* InnoDB must not perform the change inplace (below patch)
* innodb.innodb-alter-timestamp main.partition_innodb test would
  also need futher investigation.

InnoDB ha_innobase::check_if_supported_inplace_alter to support the
removal of Field_{num,decimal}::is_equal AUTO_INCREMENT checks would need the following change

diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
index a5ccb1957f3..9d778e2d39a 100644
--- a/storage/innobase/handler/handler0alter.cc
+++ b/storage/innobase/handler/handler0alter.cc
@@ -2455,10 +2455,15 @@ ha_innobase::check_if_supported_inplace_alter(
                        /* An AUTO_INCREMENT attribute can only
                        be added to an existing column by ALGORITHM=COPY,
                        but we can remove the attribute. */
-                       ut_ad((MTYP_TYPENR((*af)->unireg_check)
-                              != Field::NEXT_NUMBER)
-                             || (MTYP_TYPENR(f->unireg_check)
-                                 == Field::NEXT_NUMBER));
+                       if ((MTYP_TYPENR((*af)->unireg_check)
+                              == Field::NEXT_NUMBER)
+                             && (MTYP_TYPENR(f->unireg_check)
+                                 != Field::NEXT_NUMBER))
+                       {
+                               ha_alter_info->unsupported_reason = my_get_err_msg(
+                                       ER_ALTER_OPERATION_NOT_SUPPORTED_REASON_AUTOINC);
+                               DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
+                       }

With this change the main.auto_increment test for bug #14573, under
innodb, will pass without the 2 --error ER_DUP_ENTRY entries.

The function header comment was updated to reflect the MDEV-31086
changes.
This commit is contained in:
Daniel Black 2023-08-26 13:32:47 +10:00
parent 4f5346579a
commit 1182451af1
3 changed files with 49 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
mysql-test/suite/innodb/r/foreign_key.result
View File

@ -892,4 +892,19 @@ DROP INDEX fx ON t1;
INSERT INTO t1 VALUES (2,11,11);
DROP TABLE t1;
SET FOREIGN_KEY_CHECKS=DEFAULT;
#
# MDEV-32018 Allow the setting of Auto_increment on FK referenced columns
#
CREATE TABLE t1 (
id int unsigned NOT NULL PRIMARY KEY
);
CREATE TABLE t2 (
id int unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY,
t1_id int unsigned DEFAULT NULL,
CONSTRAINT FK_t1_id FOREIGN KEY (t1_id) REFERENCES t1 (id)
);
ALTER TABLE t1 MODIFY id INT unsigned AUTO_INCREMENT;
DROP TABLE t1,t2;
#
# End of 10.4 tests
#

22
mysql-test/suite/innodb/t/foreign_key.test
View File

@ -935,6 +935,26 @@ INSERT INTO t1 VALUES (2,11,11);
DROP TABLE t1;
SET FOREIGN_KEY_CHECKS=DEFAULT;
-- echo # End of 10.4 tests
--echo #
--echo # MDEV-32018 Allow the setting of Auto_increment on FK referenced columns
--echo #
CREATE TABLE t1 (
id int unsigned NOT NULL PRIMARY KEY
);
CREATE TABLE t2 (
id int unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY,
t1_id int unsigned DEFAULT NULL,
CONSTRAINT FK_t1_id FOREIGN KEY (t1_id) REFERENCES t1 (id)
);
ALTER TABLE t1 MODIFY id INT unsigned AUTO_INCREMENT;
DROP TABLE t1,t2;
--echo #
--echo # End of 10.4 tests
--echo #
--source include/wait_until_count_sessions.inc

16
sql/sql_table.cc
View File

@ -9135,8 +9135,7 @@ enum fk_column_change_type
@retval FK_COLUMN_NO_CHANGE No significant changes are to be done on
foreign key columns.
@retval FK_COLUMN_DATA_CHANGE ALTER TABLE might result in value
change in foreign key column (and
foreign_key_checks is on).
change in foreign key column.
@retval FK_COLUMN_RENAMED Foreign key column is renamed.
@retval FK_COLUMN_DROPPED Foreign key column is dropped.
*/
@ -9172,7 +9171,18 @@ fk_check_column_changes(THD *thd, Alter_info *alter_info,
return FK_COLUMN_RENAMED;
}
if ((old_field->is_equal(*new_field) == IS_EQUAL_NO) ||
/*
Field_{num|decimal}::is_equal evaluates to IS_EQUAL_NO where
the new_field adds an AUTO_INCREMENT flag on a column due to a
limitation in MyISAM/ARIA. For the purposes of FK determination
it doesn't matter if AUTO_INCREMENT is there or not.
*/
const uint flags= new_field->flags;
new_field->flags&= ~AUTO_INCREMENT_FLAG;
const bool equal_result= old_field->is_equal(*new_field);
new_field->flags= flags;
if ((equal_result == IS_EQUAL_NO) ||
((new_field->flags & NOT_NULL_FLAG) &&
!(old_field->flags & NOT_NULL_FLAG)))
{