maria-developers team mailing list archive

[Oleksandr Byelkin <sanja@xxxxxxxxxxx>]

The check of triggers (if present) need only if definition did not changed.
Before the check need all that if would be just "return true".

Sergei Golubchik <serg@xxxxxxxxxxx> schrieb am Mi., 28. Sept. 2022, 19:15:

> Hi, Oleksandr,
>
> On Sep 28, Oleksandr Byelkin wrote:
> > > > +bool TABLE_LIST::is_the_same_definition(THD* thd, TABLE_SHARE *s)
> > > > +{
> > > > +  enum enum_table_ref_type tp= s->get_table_ref_type();
> > > > +  if (m_table_ref_type == tp)
> > > > +  {
> > > > +    bool res= m_table_ref_version == s->get_table_ref_version();
> > > > +
> > > > +    /*
> > > > +      If definition is different check content version
> > > > +    */
> > >
> > > if res == true, why do you need to check tabledef_version?
> > >
> >
> > You are right, fixed:
> > --- a/sql/table.cc
> > +++ b/sql/table.cc
> > @@ -8881,10 +8881,11 @@ bool TABLE_LIST::is_the_same_definition(THD* thd,
> > TABLE_SHARE *s)
> >      /*
> >        If definition is different check content version
> >      */
> > -    if (tabledef_version.length &&
> > -        tabledef_version.length == s->tabledef_version.length &&
> > -        memcmp(tabledef_version.str, s->tabledef_version.str,
> > -               tabledef_version.length) == 0)
> > +    if (res ||
> > +        (tabledef_version.length &&
> > +         tabledef_version.length == s->tabledef_version.length &&
> > +         memcmp(tabledef_version.str, s->tabledef_version.str,
> > +                tabledef_version.length) == 0))
>
> I still don't understand. I'd expect something like
>
>    if (!res &&
>        (tabledef_version.length && ...
>
> that is, if m_table_ref_version matches, then you don't need to do any
> further checks, the table is fine. If m_table_ref_version differs,
> meaning, the table was reopened, then you check tabledef_version and
> triggers.
>
> Perhaps the simplest fix would be to remove res and instead do
>
>   if (m_table_ref_version == s->get_table_ref_version())
>     return TRUE;
>
>   if (tabledef_version.length && ...
>
> >      {
> >        if (table && table->triggers)
> >        {
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx
>