maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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