MDEV-12586 ALTER TABLE…ALGORITHM=INPLACE fails with non-constant DEFA…

…ULT values ha_innobase::check_if_supported_inplace_alter(): For now, reject ALGORITHM=INPLACE when a non-constant DEFAULT expression is specified for ADD COLUMN or for changing a NULL column to NOT NULL. Later, we should evaluate the non-constant column values in these cases.
MariaDB · May 8, 2017 · 0627a0f · 0627a0f
1 parent 85ea327
commit 0627a0f
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 38 deletions.
diff --git a/mysql-test/suite/innodb/r/innodb-alter-timestamp.result b/mysql-test/suite/innodb/r/innodb-alter-timestamp.result
@@ -2,7 +2,7 @@ CREATE TABLE t1 (
 `i1` INT(10) UNSIGNED NOT NULL,
 `d1` TIMESTAMP NULL DEFAULT NULL
 ) ENGINE=innodb;
-show create table t1;
+SHOW CREATE TABLE t1;
 Table	Create Table
 t1	CREATE TABLE `t1` (
   `i1` int(10) unsigned NOT NULL,
@@ -25,5 +25,26 @@ CREATE TABLE t1 (
 ) ENGINE=innodb;
 INSERT INTO t1 (i1) VALUES (1), (2), (3), (4), (5);
 ALTER TABLE t1 CHANGE `d1` `d1` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;
-drop table t1;
-set sql_mode = '';
+ALTER TABLE t1 ADD COLUMN d2 TIMESTAMP DEFAULT '2017-05-08 16:23:45',
+LOCK=NONE;
+ALTER TABLE t1 ADD COLUMN d3 TIMESTAMP DEFAULT d1, LOCK=NONE;
+ERROR 0A000: LOCK=NONE is not supported for this operation. Try LOCK=SHARED
+ALTER TABLE t1 ADD COLUMN d3 TIMESTAMP DEFAULT d1, ALGORITHM=INPLACE;
+ERROR 0A000: ALGORITHM=INPLACE is not supported for this operation. Try ALGORITHM=COPY
+ALTER TABLE t1 ADD COLUMN d3 TIMESTAMP DEFAULT d1;
+SELECT d1-d3, d2 FROM t1;
+d1-d3	d2
+0	2017-05-08 16:23:45
+0	2017-05-08 16:23:45
+0	2017-05-08 16:23:45
+0	2017-05-08 16:23:45
+0	2017-05-08 16:23:45
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `i1` int(10) unsigned NOT NULL,
+  `d1` timestamp NOT NULL DEFAULT current_timestamp(),
+  `d2` timestamp NOT NULL DEFAULT '2017-05-08 16:23:45',
+  `d3` timestamp NOT NULL DEFAULT `d1`
+) ENGINE=InnoDB DEFAULT CHARSET=latin1
+DROP TABLE t1;
diff --git a/mysql-test/suite/innodb/t/innodb-alter-timestamp.test b/mysql-test/suite/innodb/t/innodb-alter-timestamp.test
@@ -5,7 +5,7 @@ CREATE TABLE t1 (
   `d1` TIMESTAMP NULL DEFAULT NULL
 ) ENGINE=innodb;
 
-show create table t1;
+SHOW CREATE TABLE t1;
 
 INSERT INTO t1 (i1) VALUES (1), (2), (3), (4), (5);
 select * from t1;
@@ -19,9 +19,14 @@ CREATE TABLE t1 (
 ) ENGINE=innodb;
 INSERT INTO t1 (i1) VALUES (1), (2), (3), (4), (5);
 ALTER TABLE t1 CHANGE `d1` `d1` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;
-drop table t1;
-set sql_mode = '';
-
-
-
 
+ALTER TABLE t1 ADD COLUMN d2 TIMESTAMP DEFAULT '2017-05-08 16:23:45',
+LOCK=NONE;
+--error ER_ALTER_OPERATION_NOT_SUPPORTED
+ALTER TABLE t1 ADD COLUMN d3 TIMESTAMP DEFAULT d1, LOCK=NONE;
+--error ER_ALTER_OPERATION_NOT_SUPPORTED
+ALTER TABLE t1 ADD COLUMN d3 TIMESTAMP DEFAULT d1, ALGORITHM=INPLACE;
+ALTER TABLE t1 ADD COLUMN d3 TIMESTAMP DEFAULT d1;
+SELECT d1-d3, d2 FROM t1;
+SHOW CREATE TABLE t1;
+DROP TABLE t1;
diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
@@ -722,35 +722,6 @@ ha_innobase::check_if_supported_inplace_alter(
 		}
 	}
 
-	/* If we have column that has changed from NULL -> NOT NULL
-	and column default has changed we need to do additional
-	check. */
-	if ((ha_alter_info->handler_flags
-			& Alter_inplace_info::ALTER_COLUMN_NOT_NULLABLE) &&
-	    (ha_alter_info->handler_flags
-		    & Alter_inplace_info::ALTER_COLUMN_DEFAULT)) {
-		Alter_info *alter_info = ha_alter_info->alter_info;
-		List_iterator<Create_field> def_it(alter_info->create_list);
-		Create_field *def;
-		while ((def=def_it++)) {
-
-			/* If this is first column definition whose SQL type
-			is TIMESTAMP and it is defined as NOT NULL and
-			it has either constant default or function default
-			we must use "Copy" method. */
-			if (is_timestamp_type(def->sql_type)) {
-				if ((def->flags & NOT_NULL_FLAG) != 0 && // NOT NULL
-					(def->default_value != NULL || // constant default ?
-					 def->unireg_check != Field::NONE)) { // function default
-					ha_alter_info->unsupported_reason = innobase_get_err_msg(
-						ER_ALTER_OPERATION_NOT_SUPPORTED_REASON_NOT_NULL);
-					DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
-				}
-				break;
-			}
-		}
-	}
-
 	ulint n_indexes = UT_LIST_GET_LEN((m_prebuilt->table)->indexes);
 
 	/* If InnoDB dictionary and MySQL frm file are not consistent
@@ -1035,6 +1006,72 @@ ha_innobase::check_if_supported_inplace_alter(
 		}
 	}
 
+	/* When changing a NULL column to NOT NULL and specifying a
+	DEFAULT value, ensure that the DEFAULT expression is a constant.
+	Also, in ADD COLUMN, for now we only support a
+	constant DEFAULT expression. */
+	cf_it.rewind();
+	Field **af = altered_table->field;
+	while (Create_field* cf = cf_it++) {
+		DBUG_ASSERT(cf->field
+			    || (ha_alter_info->handler_flags
+				& Alter_inplace_info::ADD_COLUMN));
+
+		if (const Field* f = cf->field) {
+			/* This could be changing an existing column
+			from NULL to NOT NULL. For now, ensure that
+			the DEFAULT is a constant. */
+			if (~ha_alter_info->handler_flags
+			    & (Alter_inplace_info::ALTER_COLUMN_NOT_NULLABLE
+			       | Alter_inplace_info::ALTER_COLUMN_DEFAULT)
+			    || (*af)->real_maybe_null()) {
+				/* This ALTER TABLE is not both changing
+				a column to NOT NULL and changing the
+				DEFAULT value of a column, or this column
+				does allow NULL after the ALTER TABLE. */
+				goto next_column;
+			}
+
+			/* Find the matching column in the old table. */
+			Field** fp;
+			for (fp = table->field; *fp; fp++) {
+				if (f != *fp) {
+					continue;
+				}
+				if (!f->real_maybe_null()) {
+					/* The column already is NOT NULL. */
+					goto next_column;
+				}
+				break;
+			}
+
+			/* The column must be found in the old table. */
+			DBUG_ASSERT(fp < &table->field[table->s->fields]);
+		}
+
+		if (!(*af)->default_value
+		    || (*af)->default_value->flags == 0) {
+			/* The NOT NULL column is not
+			carrying a non-constant DEFAULT. */
+			goto next_column;
+		}
+
+		/* TODO: Allow NULL column values to
+		be replaced with a non-constant DEFAULT. */
+		if (cf->field) {
+			ha_alter_info->unsupported_reason
+				= innobase_get_err_msg(
+					ER_ALTER_OPERATION_NOT_SUPPORTED_REASON_NOT_NULL);
+		}
+
+		DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
+
+next_column:
+		af++;
+	}
+
+	cf_it.rewind();
+
 	DBUG_RETURN(online
 		    ? HA_ALTER_INPLACE_NO_LOCK_AFTER_PREPARE
 		    : HA_ALTER_INPLACE_SHARED_LOCK_AFTER_PREPARE);