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".
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@mariadb.org