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