Hello Sergei,

commit 5a362d486b30fdaf3c7a360737331767154b4ee8 Author: Sergei Golubchik Date: Mon Jul 18 22:53:27 2022 +0200

bugfix: DEFAULT NULL was allowed for NOT NULL columns

this was detected during parsing (so NOT NULL DEFAULT NULL was an error), but if a column was made NOT NULL later (e.g. as a part of primary key, in mysql_prepare_create_table()), DEFAULT NULL was accepted and ignored.

to fix this the NOT NULL DEFAULT NULL was moved into mysql_prepare_create_table().

I'm not sure that in case of auto_increment we should really disallow DEFAULT NULL under terms of this TIMESTAMP task. Look at this script: DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a INT NOT NULL AUTO_INCREMENT PRIMARY KEY); INSERT INTO t1 VALUES (NULL); Query OK, 1 row affected (0.012 sec) NULL can be inserted without errors. Yes, it's inserted with a side effect - it's replaced to the next auto increment value. But why this side effect makes NULL an invalid default? Note, I'm not against this change. Stricter behavior is good :) But I'm worried that: - It can break compatibility on one hand. - It goes without it's own MDEV on the other hand. This change deserves it's own MDEV (with motivation and consequences explained) and should be properly documented and mentioned in release notes. <cut>

index 712b8629c09..d8471894db9 100644 --- a/mysql-test/main/view.result +++ b/mysql-test/main/view.result @@ -4082,12 +4082,12 @@ DROP TABLE t1,t2; # MDEV-6251: SIGSEGV in query optimizer (in set_check_materialized # with MERGE view) # -CREATE TABLE t1 (a1 INT(11) NOT NULL DEFAULT NULL AUTO_INCREMENT PRIMARY KEY); -CREATE TABLE t2 (b1 INT(11) NOT NULL DEFAULT NULL AUTO_INCREMENT PRIMARY KEY); -CREATE TABLE t3 (c1 INT(11) NOT NULL DEFAULT NULL AUTO_INCREMENT PRIMARY KEY); -CREATE TABLE t4 (d1 INT(11) NOT NULL DEFAULT NULL AUTO_INCREMENT PRIMARY KEY); -CREATE TABLE t5 (e1 INT(11) NOT NULL DEFAULT NULL AUTO_INCREMENT PRIMARY KEY); -CREATE TABLE t6 (f1 INT(11) NOT NULL DEFAULT NULL AUTO_INCREMENT PRIMARY KEY); +CREATE TABLE t1 (a1 INT(11) NOT NULL AUTO_INCREMENT PRIMARY KEY); +CREATE TABLE t2 (b1 INT(11) NOT NULL AUTO_INCREMENT PRIMARY KEY); +CREATE TABLE t3 (c1 INT(11) NOT NULL AUTO_INCREMENT PRIMARY KEY); +CREATE TABLE t4 (d1 INT(11) NOT NULL AUTO_INCREMENT PRIMARY KEY); +CREATE TABLE t5 (e1 INT(11) NOT NULL AUTO_INCREMENT PRIMARY KEY); +CREATE TABLE t6 (f1 INT(11) NOT NULL AUTO_INCREMENT PRIMARY KEY);

It seems you removed all cases with the old syntax. So now it's not recorded in MTR what happens on this statement: CREATE TABLE t1 (a INT NOT NULL DEFAULT NULL AUTO_INCREMENT PRIMARY KEY); Please cover this scenario in some relevant *.test file. <cut>

diff --git a/sql/sql_table.cc b/sql/sql_table.cc index cf0bbfb1130..3eeabe20c71 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc

<cut>

@@ -3647,6 +3643,15 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, { Field::utype type= (Field::utype) MTYP_TYPENR(sql_field->unireg_check);

+ if (sql_field->default_value && + sql_field->default_value->expr->type() == Item::NULL_ITEM && + sql_field->flags & NOT_NULL_FLAG && + !(sql_field->flags & AUTO_INCREMENT_FLAG)) + { + my_error(ER_INVALID_DEFAULT, MYF(0), sql_field->field_name.str); + DBUG_RETURN(TRUE); + }

^^^ now obvious errors are caught only during EXECUTE, although it's already clear at the PREPARE stage that the DEFAULT is wrong: MariaDB [test]> PREPARE stmt FROM 'CREATE TABLE T1 (a INT NOT NULL DEFAULT NULL)'; Query OK, 0 rows affected (0.000 sec) Statement prepared MariaDB [test]> EXECUTE stmt; ERROR 1067 (42000): Invalid default value for 'a' I suggest we still catch non-unambiguous cases (when nothing depends on system variables) during PREPARE. Inside some existing or a new Type_handler method. Also, please add PS tests: - for unambiguous scenarios like here - for ambiguous scenarios, e.g. with TIMESTAMP columns changing OPTION_EXPLICIT_DEF_TIMESTAMP between PREPARE and EXECUTE. It seems this code needs to be changed:

/* Set NO_DEFAULT_VALUE_FLAG if this field doesn't have a default value and it is NOT NULL, not an AUTO_INCREMENT field, not a TIMESTAMP and not updated trough a NOW() function. */ if (!sql_field->default_value && !sql_field->has_default_function() && (sql_field->flags & NOT_NULL_FLAG) && (!sql_field->is_timestamp_type() || (thd->variables.option_bits & OPTION_EXPLICIT_DEF_TIMESTAMP))&& !sql_field->vers_sys_field()) { sql_field->flags|= NO_DEFAULT_VALUE_FLAG; sql_field->pack_flag|= FIELDFLAG_NO_DEFAULT; }

It can only add NO DEFAULT related flags. Shouldn't it also remove NO DEFAULT flags if the current PS EXECUTE time option OPTION_EXPLICIT_DEF_TIMESTAMP is unset? <cut>

diff --git a/storage/spider/mysql-test/spider/bg/t/spider_fixes.test b/storage/spider/mysql-test/spider/bg/t/spider_fixes.test index b222f494ba1..41f87e61416 100644 --- a/storage/spider/mysql-test/spider/bg/t/spider_fixes.test +++ b/storage/spider/mysql-test/spider/bg/t/spider_fixes.test @@ -294,14 +294,14 @@ if ($USE_CHILD_GROUP2) --disable_query_log echo CREATE TABLE ta_l ( a int(11) NOT NULL DEFAULT '0', - b char(1) DEFAULT NULL, - c datetime DEFAULT NULL, + b char(1), + c datetime, PRIMARY KEY (a, b, c) ) MASTER_1_ENGINE MASTER_1_CHARSET MASTER_1_COMMENT5_2_1; eval CREATE TABLE ta_l ( a int(11) NOT NULL DEFAULT '0', - b char(1) DEFAULT NULL, - c datetime DEFAULT NULL, + b char(1), + c datetime, PRIMARY KEY (a, b, c) ) $MASTER_1_ENGINE $MASTER_1_CHARSET $MASTER_1_COMMENT5_2_1; --enable_query_log

Why this change ^^^ ?

diff --git a/storage/spider/mysql-test/spider/t/spider_fixes.test b/storage/spider/mysql-test/spider/t/spider_fixes.test index 47bc225d614..75f108f1164 100644 --- a/storage/spider/mysql-test/spider/t/spider_fixes.test +++ b/storage/spider/mysql-test/spider/t/spider_fixes.test @@ -294,14 +294,14 @@ if ($USE_CHILD_GROUP2) --disable_query_log echo CREATE TABLE ta_l ( a int(11) NOT NULL DEFAULT '0', - b char(1) DEFAULT NULL, - c datetime DEFAULT NULL, + b char(1), + c datetime, PRIMARY KEY (a, b, c) ) MASTER_1_ENGINE MASTER_1_CHARSET MASTER_1_COMMENT5_2_1; eval CREATE TABLE ta_l ( a int(11) NOT NULL DEFAULT '0', - b char(1) DEFAULT NULL, - c datetime DEFAULT NULL, + b char(1), + c datetime, PRIMARY KEY (a, b, c) ) $MASTER_1_ENGINE $MASTER_1_CHARSET $MASTER_1_COMMENT5_2_1; --enable_query_log

Same question.