From 0627a0f399ea5bcb4a892239571775304ca212b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 8 May 2017 17:01:38 +0300 Subject: [PATCH] =?UTF-8?q?MDEV-12586=20ALTER=20TABLE=E2=80=A6ALGORITHM=3D?= =?UTF-8?q?INPLACE=20fails=20with=20non-constant=20DEFAULT=20values?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../innodb/r/innodb-alter-timestamp.result | 27 +++++- .../innodb/t/innodb-alter-timestamp.test | 17 ++-- storage/innobase/handler/handler0alter.cc | 95 +++++++++++++------ 3 files changed, 101 insertions(+), 38 deletions(-) diff --git a/mysql-test/suite/innodb/r/innodb-alter-timestamp.result b/mysql-test/suite/innodb/r/innodb-alter-timestamp.result index 2ab81136d1f..96ce33ac097 100644 --- 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 index c0b17ee6440..935320fb553 100644 --- 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 index f2cfcce50f2..849096a2d83 100644 --- 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 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);