30 Jul
2022
30 Jul
'22
12:34 p.m.
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. > 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. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org