MDEV-16830 Crash in ALTER TABLE DROP FOREIGN KEY

ha_innobase::inplace_alter_table(): Do nothing if INNOBASE_ALTER_INSTANT flags (such as DROP FOREIGN KEY) was present. Also, use ALTER_OPTIONS instead of the alias ALTER_CHANGE_CREATE_OPTION. This bug was caused by MDEV-11369, MDEV-13134 or related work.
MariaDB · Aug 3, 2018 · aab5c55 · aab5c55
1 parent 0545970
commit aab5c55
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 17 deletions.
diff --git a/mysql-test/suite/innodb/r/instant_alter.result b/mysql-test/suite/innodb/r/instant_alter.result
@@ -446,6 +446,25 @@ ALTER TABLE t1 ADD COLUMN c INT;
 UPDATE t1 SET c = 1;
 UPDATE t1 SET c = 2;
 DROP TABLE t1;
+CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB ROW_FORMAT=REDUNDANT;
+CREATE TABLE t2 (b INT PRIMARY KEY, FOREIGN KEY(b) REFERENCES t1(a))
+ENGINE=InnoDB ROW_FORMAT=REDUNDANT;
+INSERT INTO t1 SET a=1;
+INSERT INTO t2 SET b=1;
+ALTER TABLE t2 ADD COLUMN a INT, DROP FOREIGN KEY t2_ibfk_1;
+ALTER TABLE t2 ADD INDEX(a);
+ALTER TABLE t1 ADD COLUMN b INT, ADD FOREIGN KEY(a) REFERENCES t2(a),
+ALGORITHM=INSTANT;
+ERROR 0A000: ALGORITHM=INSTANT is not supported. Reason: Adding foreign keys needs foreign_key_checks=OFF. Try ALGORITHM=COPY
+SET foreign_key_checks=0;
+ALTER TABLE t1 ADD COLUMN b INT, ADD FOREIGN KEY(a) REFERENCES t2(a),
+ALGORITHM=INSTANT;
+ALTER TABLE t2 ADD CONSTRAINT fk FOREIGN KEY(b) REFERENCES t1(a),
+ALGORITHM=INSTANT;
+SET foreign_key_checks=1;
+ALTER TABLE t2 COMMENT 'domestic keys only', DROP FOREIGN KEY fk;
+ALTER TABLE t1 DROP FOREIGN KEY t1_ibfk_1;
+DROP TABLE t2, t1;
 CREATE TABLE t1
 (id INT PRIMARY KEY, c2 INT UNIQUE,
 c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'),
@@ -838,6 +857,25 @@ ALTER TABLE t1 ADD COLUMN c INT;
 UPDATE t1 SET c = 1;
 UPDATE t1 SET c = 2;
 DROP TABLE t1;
+CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB ROW_FORMAT=COMPACT;
+CREATE TABLE t2 (b INT PRIMARY KEY, FOREIGN KEY(b) REFERENCES t1(a))
+ENGINE=InnoDB ROW_FORMAT=COMPACT;
+INSERT INTO t1 SET a=1;
+INSERT INTO t2 SET b=1;
+ALTER TABLE t2 ADD COLUMN a INT, DROP FOREIGN KEY t2_ibfk_1;
+ALTER TABLE t2 ADD INDEX(a);
+ALTER TABLE t1 ADD COLUMN b INT, ADD FOREIGN KEY(a) REFERENCES t2(a),
+ALGORITHM=INSTANT;
+ERROR 0A000: ALGORITHM=INSTANT is not supported. Reason: Adding foreign keys needs foreign_key_checks=OFF. Try ALGORITHM=COPY
+SET foreign_key_checks=0;
+ALTER TABLE t1 ADD COLUMN b INT, ADD FOREIGN KEY(a) REFERENCES t2(a),
+ALGORITHM=INSTANT;
+ALTER TABLE t2 ADD CONSTRAINT fk FOREIGN KEY(b) REFERENCES t1(a),
+ALGORITHM=INSTANT;
+SET foreign_key_checks=1;
+ALTER TABLE t2 COMMENT 'domestic keys only', DROP FOREIGN KEY fk;
+ALTER TABLE t1 DROP FOREIGN KEY t1_ibfk_1;
+DROP TABLE t2, t1;
 CREATE TABLE t1
 (id INT PRIMARY KEY, c2 INT UNIQUE,
 c3 POINT NOT NULL DEFAULT ST_GeomFromText('POINT(3 4)'),
@@ -1230,10 +1268,29 @@ ALTER TABLE t1 ADD COLUMN c INT;
 UPDATE t1 SET c = 1;
 UPDATE t1 SET c = 2;
 DROP TABLE t1;
+CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB ROW_FORMAT=DYNAMIC;
+CREATE TABLE t2 (b INT PRIMARY KEY, FOREIGN KEY(b) REFERENCES t1(a))
+ENGINE=InnoDB ROW_FORMAT=DYNAMIC;
+INSERT INTO t1 SET a=1;
+INSERT INTO t2 SET b=1;
+ALTER TABLE t2 ADD COLUMN a INT, DROP FOREIGN KEY t2_ibfk_1;
+ALTER TABLE t2 ADD INDEX(a);
+ALTER TABLE t1 ADD COLUMN b INT, ADD FOREIGN KEY(a) REFERENCES t2(a),
+ALGORITHM=INSTANT;
+ERROR 0A000: ALGORITHM=INSTANT is not supported. Reason: Adding foreign keys needs foreign_key_checks=OFF. Try ALGORITHM=COPY
+SET foreign_key_checks=0;
+ALTER TABLE t1 ADD COLUMN b INT, ADD FOREIGN KEY(a) REFERENCES t2(a),
+ALGORITHM=INSTANT;
+ALTER TABLE t2 ADD CONSTRAINT fk FOREIGN KEY(b) REFERENCES t1(a),
+ALGORITHM=INSTANT;
+SET foreign_key_checks=1;
+ALTER TABLE t2 COMMENT 'domestic keys only', DROP FOREIGN KEY fk;
+ALTER TABLE t1 DROP FOREIGN KEY t1_ibfk_1;
+DROP TABLE t2, t1;
 disconnect analyze;
 SELECT variable_value-@old_instant instants
 FROM information_schema.global_status
 WHERE variable_name = 'innodb_instant_alter_column';
 instants
-39
+45
 SET GLOBAL innodb_purge_rseg_truncate_frequency= @saved_frequency;
diff --git a/mysql-test/suite/innodb/t/instant_alter.test b/mysql-test/suite/innodb/t/instant_alter.test
@@ -319,6 +319,27 @@ UPDATE t1 SET c = 1;
 UPDATE t1 SET c = 2;
 DROP TABLE t1;
 
+# MDEV-16830 Crash in ALTER TABLE DROP FOREIGN KEY
+eval CREATE TABLE t1 (a INT PRIMARY KEY) $engine;
+eval CREATE TABLE t2 (b INT PRIMARY KEY, FOREIGN KEY(b) REFERENCES t1(a))
+$engine;
+INSERT INTO t1 SET a=1;
+INSERT INTO t2 SET b=1;
+ALTER TABLE t2 ADD COLUMN a INT, DROP FOREIGN KEY t2_ibfk_1;
+ALTER TABLE t2 ADD INDEX(a);
+--error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON
+ALTER TABLE t1 ADD COLUMN b INT, ADD FOREIGN KEY(a) REFERENCES t2(a),
+ALGORITHM=INSTANT;
+SET foreign_key_checks=0;
+ALTER TABLE t1 ADD COLUMN b INT, ADD FOREIGN KEY(a) REFERENCES t2(a),
+ALGORITHM=INSTANT;
+ALTER TABLE t2 ADD CONSTRAINT fk FOREIGN KEY(b) REFERENCES t1(a),
+ALGORITHM=INSTANT;
+SET foreign_key_checks=1;
+ALTER TABLE t2 COMMENT 'domestic keys only', DROP FOREIGN KEY fk;
+ALTER TABLE t1 DROP FOREIGN KEY t1_ibfk_1;
+DROP TABLE t2, t1;
+
 dec $format;
 }
 disconnect analyze;

diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
@@ -77,8 +77,8 @@ static const alter_table_operations INNOBASE_DEFAULTS
 static const alter_table_operations INNOBASE_ALTER_REBUILD
 	= ALTER_ADD_PK_INDEX
 	| ALTER_DROP_PK_INDEX
-	| ALTER_CHANGE_CREATE_OPTION
-	/* CHANGE_CREATE_OPTION needs to check create_option_need_rebuild() */
+	| ALTER_OPTIONS
+	/* ALTER_OPTIONS needs to check create_option_need_rebuild() */
 	| ALTER_COLUMN_NULLABLE
 	| INNOBASE_DEFAULTS
 	| ALTER_STORED_COLUMN_ORDER
@@ -498,8 +498,7 @@ static bool create_option_need_rebuild(
 	const Alter_inplace_info*	ha_alter_info,
 	const TABLE*			table)
 {
-	DBUG_ASSERT(ha_alter_info->handler_flags
-		    & ALTER_CHANGE_CREATE_OPTION);
+	DBUG_ASSERT(ha_alter_info->handler_flags & ALTER_OPTIONS);
 
 	if (ha_alter_info->create_info->used_fields
 	    & (HA_CREATE_USED_ROW_FORMAT
@@ -541,7 +540,7 @@ innobase_need_rebuild(
 {
 	if ((ha_alter_info->handler_flags & ~(INNOBASE_INPLACE_IGNORE
 					      | INNOBASE_ALTER_INSTANT))
-	    == ALTER_CHANGE_CREATE_OPTION) {
+	    == ALTER_OPTIONS) {
 		return create_option_need_rebuild(ha_alter_info, table);
 	}
 
@@ -662,8 +661,7 @@ instant_alter_column_possible(
 		return false;
 	}
 
-	if (~ha_alter_info->handler_flags
-	    & ALTER_ADD_STORED_BASE_COLUMN) {
+	if (~ha_alter_info->handler_flags & ALTER_ADD_STORED_BASE_COLUMN) {
 		return false;
 	}
 
@@ -686,13 +684,11 @@ instant_alter_column_possible(
 	columns. */
 	if (ha_alter_info->handler_flags
 	    & ((INNOBASE_ALTER_REBUILD | INNOBASE_ONLINE_CREATE)
-	       & ~ALTER_ADD_STORED_BASE_COLUMN
-	       & ~ALTER_CHANGE_CREATE_OPTION)) {
+	       & ~ALTER_ADD_STORED_BASE_COLUMN & ~ALTER_OPTIONS)) {
 		return false;
 	}
 
-	return !(ha_alter_info->handler_flags
-		 & ALTER_CHANGE_CREATE_OPTION)
+	return !(ha_alter_info->handler_flags & ALTER_OPTIONS)
 		|| !create_option_need_rebuild(ha_alter_info, table);
 }
 
@@ -5457,8 +5453,7 @@ prepare_inplace_alter_table_dict(
 			}
 		}
 
-		if (ha_alter_info->handler_flags
-		    & ALTER_CHANGE_CREATE_OPTION) {
+		if (ha_alter_info->handler_flags & ALTER_OPTIONS) {
 			const ha_table_option_struct& alt_opt=
 				*ha_alter_info->create_info->option_struct;
 			const ha_table_option_struct& opt=
@@ -6827,7 +6822,7 @@ ha_innobase::prepare_inplace_alter_table(
 	if (!(ha_alter_info->handler_flags & INNOBASE_ALTER_DATA)
 	    || ((ha_alter_info->handler_flags & ~(INNOBASE_INPLACE_IGNORE
 						  | INNOBASE_ALTER_INSTANT))
-		== ALTER_CHANGE_CREATE_OPTION
+		== ALTER_OPTIONS
 		&& !create_option_need_rebuild(ha_alter_info, table))) {
 
 		if (heap) {
@@ -7107,8 +7102,9 @@ ha_innobase::inplace_alter_table(
 		DBUG_RETURN(false);
 	}
 
-	if ((ha_alter_info->handler_flags & ~INNOBASE_INPLACE_IGNORE)
-	    == ALTER_CHANGE_CREATE_OPTION
+	if ((ha_alter_info->handler_flags & ~(INNOBASE_INPLACE_IGNORE
+					      | INNOBASE_ALTER_INSTANT))
+	    == ALTER_OPTIONS
 	    && !create_option_need_rebuild(ha_alter_info, table)) {
 		goto ok_exit;
 	}