Hi, Alexander, I added tests and created an MDEV. Specific replies below: On Jul 29, Alexander Barkov wrote:
Hello Sergei,
commit 5a362d486b30fdaf3c7a360737331767154b4ee8 Author: Sergei Golubchik <serg@mariadb.org> 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.
Well, you don't even need AUTO_INCREMENT for that: create table t1 (a int not null); create trigger trg1 before insert on t1 for each row set new.a=1; insert into t1 values (null); NULL can be inserted without errors :)
@@ -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.
it's quite tricky. At most I can restore the old check and _some_ cases will be reported at prepare (explicit NOT NULL DEFAULT NULL, that is). But DEFAULT NULL PRIMARY KEY can only be detected at execute, as PRIMARY KEY -> NOT NULL adjustment happens only then.
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; }
nope, works fine. I've added tests.
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 ^^^ ?
PRIMARY KEY makes columns NOT NULL. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org