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