1 Aug
2022
1 Aug
'22
11:21 a.m.
Hello Sergei, On 7/30/22 14:34, Sergei Golubchik wrote: > Hi, Alexander, > > On Jul 29, Alexander Barkov wrote: >> Hello Sergei, >> >> I have a couple of suggestions: >> >>> commit 7b8304045272111a6f4d44196d6b37cbfef06f37 >>> Author: Sergei Golubchik <serg@mariadb.org> >>> Date: Wed Jul 20 17:31:48 2022 +0200 >>> >>> MDEV-29075 Changing explicit_defaults_for_timestamp within stored procedure works inconsistently >>> >>> move the OPTION_EXPLICIT_DEF_TIMESTAMP check from the parsing step >>> to the execution >> >>>> diff --git a/sql/field.cc b/sql/field.cc >>> index 2e0c70d3d13..23ebb07b7f7 100644 >>> --- a/sql/field.cc >>> +++ b/sql/field.cc >>> @@ -10824,6 +10824,7 @@ Column_definition::Column_definition( >>> comment= old_field->comment; >>> vcol_info= old_field->vcol_info; >>> option_list= old_field->option_list; >>> + explicitly_nullable= !(old_field->flags & NOT_NULL_FLAG); >> >> This introduces asymmetry in how NULL and NOT NULL attributes are handled. >> >> I suggest considering these ways: >> >> 1. Change "bool explicitly_nullable" to "bool explicit_nullability", >> which will be: >> >> - false if nothing was specified >> - true if NULL or NOT NULL was specified. > > I've done that, see commit 7120195ac1c, > but I don't like how it looks. When NOT_NULL_FLAG is set, the value is > redundant, it's not used anywhere and only adds few pointless > assignments to sql_yacc.yy. > > We only need to know how to interpret the case when NOT_NULL_FLAG is not > set, that's what explicitly_nullable was doing. I checked 7120195ac1c. Indeed the new version does not look better. I won't insist. > >> 2. Or don't store NOT_NULL_FLAG in Column_definition::flags at all. >> >> Add a new Column_definition enum member with three values for: >> - not specified >> - explicit NULL >> - explicit NOT NULL >> >> As discussed on slack, the latter will need some changes. >> But I think not that much, as only Column_definition needs changes, >> while Field does not seem to need at this point. > > I don't know how to do it in a robust way. Column_definition and Field > have both the same flags member, one typically just copies it back and > forth. Having just one bit out of Column_definition::flags is very > error-prone, I'd prefer all flags to be stored very differently in > Column_definition, so that one couldn't just copy them. But this > explicitly_nullable thing cannot justify such a big change. I agreed. I have no other suggestion. Thanks for your explanation. > > Regards, > Sergei > VP of MariaDB Server Engineering > and security@mariadb.org