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 <[email protected]> > > 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 [email protected] _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
