maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexander!

On Sep 15, Alexander Barkov wrote:

> >> -              Lex->last_field->flags|= NOT_NULL_FLAG;
> >> +              if (!thd->variables.explicit_defaults_for_timestamp)
> >> +                Lex->last_field->flags|= NOT_NULL_FLAG;
> >
> > Hmm, I don't like that. You're making a run-time decision during the parsing.
> > Generally this is wrong.
...
> At this point I just merged the changes from MySQL,
> and the variable is read only.

Hmm, in that case why did you add it to thd->variables?

That only makes sense for session variables that can have different
values in different connections. Global (and read-only) variables don't
need to have a copy of itself in every THD.

> >> diff --git a/mysql-test/suite/sys_vars/t/explicit_defaults_for_timestamp_basic.test b/mysql-test/suite/sys_vars/t/explicit_defaults_for_timestamp_basic.test
> >> new file mode 100644
> >> index 0000000..372e37c
> >> --- /dev/null
> >> +++ b/mysql-test/suite/sys_vars/t/explicit_defaults_for_timestamp_basic.test
> >> @@ -0,0 +1,19 @@
> >
> > No need to add these boilerplate *_basic tests anymore.
> > if there's something worth testing there - yes, but if you just
> > copy another test like that and replace the variable name - don't bother
> 
> I put the test that it's a read-only variable here.
> I had to put it somewhere anyway. Would you like to
> to rename this test somehow?

I mean that we have sysvar_* tests now. These test shows already that
explicit_defaults_for_timestamp is a read-only variable. I'd say it's
sufficient.

Regards,
Sergei