maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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@xxxxxxxxxxx