Hi Sergei,
On Mon, Sep 24, 2018 at 03:02:40PM +0200, Sergei Golubchik wrote:
> Hi, Sergey!
>
> On Sep 12, Sergey Vojtovich wrote:
> > revision-id: e11e0e6c8b9a85ca7f16c6b9b28fb505d464a96e
> > (mariadb-10.3.7-186-ge11e0e6c8b9)
> > parent(s): f2f661b848c02ebd7c444ba645b26416d3e4d2ad
> > author: Sergey Vojtovich
> > committer: Sergey Vojtovich
> > timestamp: 2018-09-12 16:36:45 +0400
> > message:
> >
> > MDEV-17167 - InnoDB: Failing assertion: table->get_ref_count() == 0 upon
> > truncating a temporary table
> >
> > TRUNCATE expects only one TABLE instance (which is used by TRUNCATE
> > itself) to be open. However this requirement wasn't enforced after
> > "MDEV-5535: Cannot reopen temporary table".
> >
> > Fixed by closing unused table instances before performing TRUNCATE.
>
> > diff --git a/sql/sql_class.h b/sql/sql_class.h
> > index 6256522bf5c..e7dcd997e05 100644
> > --- a/sql/sql_class.h
> > +++ b/sql/sql_class.h
> > @@ -4612,6 +4612,7 @@ class THD :public Statement,
> >
> > TMP_TABLE_SHARE* save_tmp_table_share(TABLE *table);
> > void restore_tmp_table_share(TMP_TABLE_SHARE *share);
> > + void close_unused_temporary_table_instances(const TABLE_LIST *tl);
> >
> > private:
> > /* Whether a lock has been acquired? */
> > diff --git a/sql/sql_truncate.cc b/sql/sql_truncate.cc
> > index 201825d4593..695fcb538f9 100644
> > --- a/sql/sql_truncate.cc
> > +++ b/sql/sql_truncate.cc
> > @@ -401,6 +401,8 @@ bool Sql_cmd_truncate_table::truncate_table(THD *thd,
> > TABLE_LIST *table_ref)
> > /* In RBR, the statement is not binlogged if the table is temporary. */
> > binlog_stmt= !thd->is_current_stmt_binlog_format_row();
> >
> > + thd->close_unused_temporary_table_instances(table_ref);
>
> Is it only TRUNCATE that needs it? What about ALTER? REPAIR? Other
> similar commands?
Right, do you think it is worth to exapnd scope of this bug and look for
other possible issues?
> > +*/
> > +
> > +void THD::close_unused_temporary_table_instances(const TABLE_LIST *tl)
> > +{
> > + TMP_TABLE_SHARE *share= find_tmp_table_share(tl);
> > +
> > + if (share)
> > + {
> > + Share_free_tables::List purge_tables;
> > + All_share_tables_list::Iterator tables_it(share->all_tmp_tables);
> > +
> > + while (TABLE *table= tables_it++)
> > + {
> > + if (table->query_id == 0)
> > + purge_tables.push_front(table);
> > + }
> > +
> > + while (TABLE *table= purge_tables.pop_front())
> > + {
> > + share->all_tmp_tables.remove(table);
> > + free_temporary_table(table);
> > + }
>
> Why are you doing it in two loops? Because free_temporary_table()
> invalidates the iterator?
Not free_temporary_table(), but rather share->all_tmp_tables.remove(table).
OTOH it seems to be safe to remove current element with it++, but not with ++it.
I'll try to remove extra loop if it works.
Thanks,
Sergey
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
