Hi, Sergey! On Sep 24, Sergey Vojtovich wrote: > On Mon, Sep 24, 2018 at 03:02:40PM +0200, Sergei Golubchik wrote: > > On Sep 12, Sergey Vojtovich wrote: > > > > > 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?
Yes, absolutely > > > +*/ > > > + > > > +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. Please add a comment about it (in whatever code you'll end up having, two loops or it++ - still this is something that needs a comment). Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org _______________________________________________ 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