Hi, Alexander! On Oct 07, Alexander Barkov wrote:
Hi Sergei,
Please review a patch for MDEV-10802.
Thanks!
diff --git a/mysql-test/suite/sys_vars/inc/explicit_defaults_for_timestamp.inc b/mysql-test/suite/sys_vars/inc/explicit_defaults_for_timestamp.inc index 4cf3914..508363b 100644 --- a/mysql-test/suite/sys_vars/inc/explicit_defaults_for_timestamp.inc +++ b/mysql-test/suite/sys_vars/inc/explicit_defaults_for_timestamp.inc @@ -97,3 +97,17 @@ CREATE TABLE t1 (a INT); ALTER TABLE t1 ADD b TIMESTAMP; SHOW CREATE TABLE t1; DROP TABLE t1; + +if (`SELECT @@explicit_defaults_for_timestamp=1`) +{ + --echo # + --echo # MDEV-10802 TIMESTAMP NOT NULL field with explicit_defaults_for_timestamp and NO_ZERO_DATE shouldn't throw error + --echo # + + SET sql_mode='ANSI,NO_ZERO_DATE'; + CREATE TABLE t1 (a TIMESTAMP NOT NULL); + INSERT INTO t1 VALUES (); + SELECT * FROM t1; + DROP TABLE t1; + SET sql_mode=DEFAULT; +}
Why is it inside if()? 1. if you want it to run only when @@explicit_defaults_for_timestamp is set, you should put it directly into explicit_defaults_for_timestamp_on.test. 2. But it's better to run this test for either value of @@explicit_defaults_for_timestamp, so it rightfully belongs into this .inc file, but not inside if().
diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 2751c79..fa5eb2a 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -4161,7 +4161,8 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, if (!sql_field->def && !sql_field->has_default_function() && (sql_field->flags & NOT_NULL_FLAG) && - !is_timestamp_type(sql_field->sql_type)) + (!is_timestamp_type(sql_field->sql_type) || + opt_explicit_defaults_for_timestamp)) { sql_field->flags|= NO_DEFAULT_VALUE_FLAG; sql_field->pack_flag|= FIELDFLAG_NO_DEFAULT; @@ -4170,6 +4171,7 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, if (thd->variables.sql_mode & MODE_NO_ZERO_DATE && !sql_field->def && !sql_field->vcol_info && is_timestamp_type(sql_field->sql_type) && + !opt_explicit_defaults_for_timestamp &&
why opt_explicit_defaults_for_timestamp is relevant here?
(sql_field->flags & NOT_NULL_FLAG) && (type == Field::NONE || type == Field::TIMESTAMP_UN_FIELD)) {
Regards, Sergei Chief Architect MariaDB and security@mariadb.org