MDEV-13176 ALTER TABLE…CHANGE col col TIMESTAMP NOT NULL DEFAULT… fails

When it comes to DEFAULT values of columns, InnoDB is imposing both
unnecessary and insufficient conditions on whether ALGORITHM=INPLACE
should be allowed for ALTER TABLE.

When changing an existing column to NOT NULL, any NULL values in the
columns only get a special treatment if the column is changed to an
AUTO_INCREMENT column (which is not supported by ALGORITHM=INPLACE)
or the column type is TIMESTAMP. In all other cases, an error
must be reported for the failure to convert a NULL value to NOT NULL.

InnoDB was unnecessarily interested in whether the DEFAULT value
is not constant when altering other than TIMESTAMP columns. Also,
when changing a TIMESTAMP column to NOT NULL, InnoDB was performing
an insufficient check, and it was incorrectly allowing a constant
DEFAULT value while not being able to replace NULL values with that
constant value.

Furthermore, in ADD COLUMN, InnoDB is unnecessarily rejecting certain
nondeterministic DEFAULT expressions (depending on the session
parameters or the current time).

3 files changed, 180 insertions, 46 deletions

diff --git a/mysql-test/suite/innodb/r/innodb-alter-timestamp.result b/mysql-test/suite/innodb/r/innodb-alter-timestamp.result
index 96ce33ac097..9659b03d6b2 100644
--- a/mysql-test/suite/innodb/r/innodb-alter-timestamp.result
+++ b/mysql-test/suite/innodb/r/innodb-alter-timestamp.result
@@ -1,7 +1,31 @@
-CREATE TABLE t1 (
-`i1` INT(10) UNSIGNED NOT NULL,
-`d1` TIMESTAMP NULL DEFAULT NULL
-) ENGINE=innodb;
+CREATE TABLE t1 (i1 INT UNSIGNED NULL DEFAULT 42) ENGINE=innodb;
+INSERT INTO t1 VALUES(NULL);
+ALTER TABLE t1 CHANGE i1 i1 INT UNSIGNED NOT NULL DEFAULT rand(),
+ALGORITHM=INPLACE;
+ERROR 22004: Invalid use of NULL value
+ALTER TABLE t1 CHANGE i1 i1 INT UNSIGNED NOT NULL DEFAULT rand(),
+ALGORITHM=COPY;
+ERROR 01000: Data truncated for column 'i1' at row 1
+ALTER TABLE t1 CHANGE i1 id INT UNSIGNED NOT NULL AUTO_INCREMENT,
+ADD PRIMARY KEY(id), ALGORITHM=INPLACE;
+ERROR 0A000: ALGORITHM=INPLACE is not supported. Reason: Cannot change column type INPLACE. Try ALGORITHM=COPY
+ALTER TABLE t1 ADD PRIMARY KEY(i1), ALGORITHM=INPLACE;
+ERROR 22004: Invalid use of NULL value
+ALTER TABLE t1 CHANGE i1 id INT UNSIGNED NOT NULL AUTO_INCREMENT,
+ADD PRIMARY KEY(id);
+affected rows: 1
+info: Records: 1  Duplicates: 0  Warnings: 0
+SELECT * FROM t1;
+id
+1
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=latin1
+DROP TABLE t1;
+CREATE TABLE t1 (i1 INT UNSIGNED NOT NULL, d1 TIMESTAMP NULL) ENGINE=InnoDB;
 SHOW CREATE TABLE t1;
 Table	Create Table
 t1	CREATE TABLE `t1` (
@@ -17,21 +41,56 @@ i1	d1
 4	NULL
 5	NULL
 set sql_mode = 'STRICT_ALL_TABLES,NO_ZERO_DATE';
-ALTER TABLE t1 CHANGE `d1` `d1` TIMESTAMP DEFAULT CURRENT_TIMESTAMP  NOT NULL;
+ALTER TABLE t1 CHANGE d1 d1 TIMESTAMP NULL DEFAULT '2017-05-08 16:23:45',
+ALGORITHM=INPLACE;
+SELECT DISTINCT d1 FROM t1;
+d1
+NULL
+ALTER TABLE t1 CHANGE d1 d1 TIMESTAMP NULL DEFAULT '2017-05-08 16:32:45',
+ALGORITHM=COPY;
+SELECT DISTINCT d1 FROM t1;
+d1
+NULL
+ALTER TABLE t1 CHANGE d1 d1 TIMESTAMP DEFAULT '2017-05-08 16:32:54';
+affected rows: 5
+info: Records: 5  Duplicates: 0  Warnings: 0
+# Note: NULL was changed to CURRENT_TIMESTAMP(),
+# not the specified constant DEFAULT value!
+SELECT COUNT(DISTINCT d1),COUNT(d1),COUNT(*) FROM t1;
+COUNT(DISTINCT d1)	COUNT(d1)	COUNT(*)
+1	5	5
+SELECT DISTINCT (CURRENT_TIMESTAMP()-d1) <= 60 FROM t1;
+(CURRENT_TIMESTAMP()-d1) <= 60
+1
 drop table t1;
 CREATE TABLE t1 (
 `i1` INT(10) UNSIGNED NOT NULL,
 `d1` TIMESTAMP NULL DEFAULT NULL
 ) ENGINE=innodb;
 INSERT INTO t1 (i1) VALUES (1), (2), (3), (4), (5);
-ALTER TABLE t1 CHANGE `d1` `d1` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;
+ALTER TABLE t1 CHANGE d1 d1 TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;
+affected rows: 5
+info: Records: 5  Duplicates: 0  Warnings: 0
+ALTER TABLE t1 ADD COLUMN w1 varchar(20) NULL DEFAULT USER();
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+ALTER TABLE t1 CHANGE w1 u1 varchar(30) NULL DEFAULT substr(USER(),1);
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+SELECT u1, COUNT(DISTINCT d1) FROM t1 GROUP BY u1;
+u1	COUNT(DISTINCT d1)
+root@localhost	1
 ALTER TABLE t1 ADD COLUMN d2 TIMESTAMP DEFAULT '2017-05-08 16:23:45',
 LOCK=NONE;
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
 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;
+affected rows: 5
+info: Records: 5  Duplicates: 0  Warnings: 0
 SELECT d1-d3, d2 FROM t1;
 d1-d3	d2
 0	2017-05-08 16:23:45
@@ -44,7 +103,17 @@ Table	Create Table
 t1	CREATE TABLE `t1` (
   `i1` int(10) unsigned NOT NULL,
   `d1` timestamp NOT NULL DEFAULT current_timestamp(),
+  `u1` varchar(30) DEFAULT substr(user(),1),
   `d2` timestamp NOT NULL DEFAULT '2017-05-08 16:23:45',
   `d3` timestamp NOT NULL DEFAULT `d1`
 ) ENGINE=InnoDB DEFAULT CHARSET=latin1
+ALTER TABLE t1 ADD COLUMN d4 TIMESTAMP DEFAULT CURRENT_TIMESTAMP;
+affected rows: 0
+info: Records: 0  Duplicates: 0  Warnings: 0
+SELECT COUNT(DISTINCT d4),COUNT(d4),COUNT(*) FROM t1;
+COUNT(DISTINCT d4)	COUNT(d4)	COUNT(*)
+1	5	5
+SELECT DISTINCT (CURRENT_TIMESTAMP()-d4) <= 60 FROM t1;
+(CURRENT_TIMESTAMP()-d4) <= 60
+1
 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
index 935320fb553..d8acc02cbdb 100644
--- a/mysql-test/suite/innodb/t/innodb-alter-timestamp.test
+++ b/mysql-test/suite/innodb/t/innodb-alter-timestamp.test
@@ -1,16 +1,46 @@
 --source include/have_innodb.inc
 
-CREATE TABLE t1 (
-  `i1` INT(10) UNSIGNED NOT NULL,
-  `d1` TIMESTAMP NULL DEFAULT NULL
-) ENGINE=innodb;
+CREATE TABLE t1 (i1 INT UNSIGNED NULL DEFAULT 42) ENGINE=innodb;
+INSERT INTO t1 VALUES(NULL);
+--enable_info
+--error ER_INVALID_USE_OF_NULL
+ALTER TABLE t1 CHANGE i1 i1 INT UNSIGNED NOT NULL DEFAULT rand(),
+ALGORITHM=INPLACE;
+--error WARN_DATA_TRUNCATED
+ALTER TABLE t1 CHANGE i1 i1 INT UNSIGNED NOT NULL DEFAULT rand(),
+ALGORITHM=COPY;
+--error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON
+ALTER TABLE t1 CHANGE i1 id INT UNSIGNED NOT NULL AUTO_INCREMENT,
+ADD PRIMARY KEY(id), ALGORITHM=INPLACE;
+--error ER_INVALID_USE_OF_NULL
+ALTER TABLE t1 ADD PRIMARY KEY(i1), ALGORITHM=INPLACE;
+ALTER TABLE t1 CHANGE i1 id INT UNSIGNED NOT NULL AUTO_INCREMENT,
+ADD PRIMARY KEY(id);
+--disable_info
+SELECT * FROM t1;
+SHOW CREATE TABLE t1;
+DROP TABLE t1;
+
+CREATE TABLE t1 (i1 INT UNSIGNED NOT NULL, d1 TIMESTAMP NULL) ENGINE=InnoDB;
 
 SHOW CREATE TABLE t1;
 
 INSERT INTO t1 (i1) VALUES (1), (2), (3), (4), (5);
 select * from t1;
 set sql_mode = 'STRICT_ALL_TABLES,NO_ZERO_DATE';
-ALTER TABLE t1 CHANGE `d1` `d1` TIMESTAMP DEFAULT CURRENT_TIMESTAMP  NOT NULL;
+ALTER TABLE t1 CHANGE d1 d1 TIMESTAMP NULL DEFAULT '2017-05-08 16:23:45',
+ALGORITHM=INPLACE;
+SELECT DISTINCT d1 FROM t1;
+ALTER TABLE t1 CHANGE d1 d1 TIMESTAMP NULL DEFAULT '2017-05-08 16:32:45',
+ALGORITHM=COPY;
+SELECT DISTINCT d1 FROM t1;
+--enable_info
+ALTER TABLE t1 CHANGE d1 d1 TIMESTAMP DEFAULT '2017-05-08 16:32:54';
+--disable_info
+--echo # Note: NULL was changed to CURRENT_TIMESTAMP(),
+--echo # not the specified constant DEFAULT value!
+SELECT COUNT(DISTINCT d1),COUNT(d1),COUNT(*) FROM t1;
+SELECT DISTINCT (CURRENT_TIMESTAMP()-d1) <= 60 FROM t1;
 drop table t1;
 
 CREATE TABLE t1 (
@@ -18,8 +48,14 @@ CREATE TABLE t1 (
   `d1` TIMESTAMP NULL DEFAULT NULL
 ) ENGINE=innodb;
 INSERT INTO t1 (i1) VALUES (1), (2), (3), (4), (5);
-ALTER TABLE t1 CHANGE `d1` `d1` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;
+--enable_info
+ALTER TABLE t1 CHANGE d1 d1 TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;
+ALTER TABLE t1 ADD COLUMN w1 varchar(20) NULL DEFAULT USER();
+ALTER TABLE t1 CHANGE w1 u1 varchar(30) NULL DEFAULT substr(USER(),1);
+--disable_info
+SELECT u1, COUNT(DISTINCT d1) FROM t1 GROUP BY u1;
 
+--enable_info
 ALTER TABLE t1 ADD COLUMN d2 TIMESTAMP DEFAULT '2017-05-08 16:23:45',
 LOCK=NONE;
 --error ER_ALTER_OPERATION_NOT_SUPPORTED
@@ -27,6 +63,12 @@ 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;
+--disable_info
 SELECT d1-d3, d2 FROM t1;
 SHOW CREATE TABLE t1;
+--enable_info
+ALTER TABLE t1 ADD COLUMN d4 TIMESTAMP DEFAULT CURRENT_TIMESTAMP;
+--disable_info
+SELECT COUNT(DISTINCT d4),COUNT(d4),COUNT(*) FROM t1;
+SELECT DISTINCT (CURRENT_TIMESTAMP()-d4) <= 60 FROM t1;
 DROP TABLE t1;
diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
index 2631f525dba..7c75711d2e3 100644
--- a/storage/innobase/handler/handler0alter.cc
+++ b/storage/innobase/handler/handler0alter.cc
@@ -1011,6 +1011,7 @@ ha_innobase::check_if_supported_inplace_alter(
 	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
@@ -1018,49 +1019,71 @@ ha_innobase::check_if_supported_inplace_alter(
 
 		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;
+			from NULL to NOT NULL. */
+			switch ((*af)->type()) {
+			case MYSQL_TYPE_TIMESTAMP:
+			case MYSQL_TYPE_TIMESTAMP2:
+				/* Inserting NULL into a TIMESTAMP column
+				would cause the DEFAULT value to be
+				replaced. Ensure that the DEFAULT
+				expression is not changing during
+				ALTER TABLE. */
+				if (!f->real_maybe_null()
+				    || (*af)->real_maybe_null()) {
+					/* The column was NOT NULL, or it
+					will allow NULL after ALTER TABLE. */
+					goto next_column;
 				}
-				if (!f->real_maybe_null()) {
-					/* The column already is NOT NULL. */
+
+				if (!(*af)->default_value
+				    && (*af)->is_real_null()) {
+					/* No DEFAULT value is
+					specified. We can report
+					errors for any NULL values for
+					the TIMESTAMP.
+
+					FIXME: Allow any DEFAULT
+					expression whose value does
+					not change during ALTER TABLE.
+					This would require a fix in
+					row_merge_read_clustered_index()
+					to try to replace the DEFAULT
+					value before reporting
+					DB_INVALID_NULL. */
 					goto next_column;
 				}
 				break;
+			default:
+				/* For any other data type, NULL
+				values are not converted.
+				(An AUTO_INCREMENT attribute cannot
+				be introduced to a column with
+				ALGORITHM=INPLACE.) */
+				ut_ad((MTYP_TYPENR((*af)->unireg_check)
+				       == Field::NEXT_NUMBER)
+				      == (MTYP_TYPENR(f->unireg_check)
+					  == Field::NEXT_NUMBER));
+				goto next_column;
 			}
 
-			/* 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);
+		} else if (!(*af)->default_value
+			   || !((*af)->default_value->flags
+				& ~(VCOL_SESSION_FUNC | VCOL_TIME_FUNC))) {
+			/* The added NOT NULL column lacks a DEFAULT value,
+			or the DEFAULT is the same for all rows.
+			(Time functions, such as CURRENT_TIMESTAMP(),
+			are evaluated from a timestamp that is assigned
+			at the start of the statement. Session
+			functions, such as USER(), always evaluate the
+			same within a statement.) */
+
+			/* Compute the DEFAULT values of non-constant columns
+			(VCOL_SESSION_FUNC | VCOL_TIME_FUNC). */
+			(*af)->set_default();
+			goto next_column;
 		}
 
 		DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);