Re: [Maria-developers] [Commits] c285dbe: MDEV-5535: Cannot reopen temporary table

19 May 2016

Hi, Nirbhay!

I like the approach, there were only few comments, but still there were
some :)

See below.

On May 16, Nirbhay Choubey wrote:
...
revision-id: c285dbece2881e1d864bb758d77edc899d568c04 (mariadb-10.1.8-82-gc285dbe)
parent(s): 222ca736f737e888115c732c8ecad84faf0a2529
author: Nirbhay Choubey
committer: Nirbhay Choubey
timestamp: 2016-05-16 23:59:10 -0400
message:
MDEV-5535: Cannot reopen temporary table
Would be nice to have a somewhat more verbose commit comment here :)
...

diff --git a/mysql-test/t/reopen_temp_table-master.test b/mysql-test/t/reopen_temp_table-master.test
new file mode 100644
index 0000000..5ac2ca8
--- /dev/null
+++ b/mysql-test/t/reopen_temp_table-master.test
@@ -0,0 +1 @@
+--tmpdir=$MYSQLTEST_VARDIR//tmp
Eh... I suppose you mean reopen_temp_table-master.opt
And, in fact, you can simply use reopen_temp_table.opt
Btw, why two slashes? And why do you need to set tmpdir at all?
I suspect you don't - because your tests apparently succeed without it :)
...
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index aa2cae5..f729733 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -4051,7 +4051,12 @@ static TABLE *create_table_from_items(THD *thd,
     }
     else
     {
-      if (open_temporary_table(thd, create_table))
+      /*
+        The pointer to the newly created temporary table has been stored in
+        table->create_info.
+      */
+      create_table->table= create_info->table;
Where was this happening before you've added an explicit assignment?
(note the assert below - it worked, so somewhere this must've been assigned)
...
+      if (!create_table->table)
       {
         /*
           This shouldn't happen as creation of temporary table should make
@@ -4060,7 +4065,6 @@ static TABLE *create_table_from_items(THD *thd,
         */
         DBUG_ASSERT(0);
       }
-      DBUG_ASSERT(create_table->table == create_info->table);
why?
...
}
   }
   else
@@ -4308,6 +4326,27 @@ bool select_create::send_eof()
     DBUG_RETURN(true);
   }
+  if (table->s->tmp_table)
+  {
+    /*
+      Now is good time to add the new table to THD temporary tables list.
+      But, before that we need to check if same table got created by the sub-
+      statement.
+    */
+    if (thd->temporary_tables.find_table_share(table->s->table_cache_key.str,
+                                               table->s->table_cache_key.length))
+    {
+      my_error(ER_TABLE_EXISTS_ERROR, MYF(0), table->alias.c_ptr());
ER_TABLE_EXISTS_ERROR, eh?
I'm not sure it's the best way to solve this. It's an error
that neither CREATE ... IF NOT EXISTS or CREATE OR REPLACE can fix.

But I don't have a good solution for this. If a concurrent thread
would've tried to create a table meanwhile (assuming, non-temporary),
it would wait on the metadata lock, that protects table creation.

so, logically, trying to create a table from inside create table
or trying to drop a used table from stored function (your ER_CANT_REOPEN_TABLE)
should fail with ER_LOCK_DEADLOCK or ER_LOCK_ABORTED - because it's, basically,
trying to get a conflicting metadata lock within the same thread, clearly a
case of the deadlock.

but somehow I believe that returning ER_LOCK_DEADLOCK on a purely myisam
test case with temporary tables - that will be confusing as hell :(

so, ok, let's keep your ER_TABLE_EXISTS_ERROR. But, if possible, please
commit this code (save_tmp_table_share and ER_CANT_REOPEN_TABLE) in a separate
commit, after the main MDEV-5535 commit. To have it clearly distinct
in the history, in case we'll want to change this behaviour later.
...
+      abort_result_set();
+      DBUG_RETURN(true);
+    }
+    else
+    {
+      DBUG_ASSERT(save_tmp_table_share);
+      thd->temporary_tables.relink_table_share(save_tmp_table_share);
+    }
+  }
+
   /*
     Do an implicit commit at end of statement for non-temporary
     tables.  This can fail, but we should unlock the table
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 40032c3..93fba14 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -2281,23 +2281,17 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
     */
     DBUG_ASSERT(!(thd->locked_tables_mode &&
                   table->open_type != OT_BASE_ONLY &&
-                  find_temporary_table(thd, table) &&
+                  thd->temporary_tables.find_table(table) &&
                   table->mdl_request.ticket != NULL));
-    /*
-      drop_temporary_table may return one of the following error codes:
-      .  0 - a temporary table was successfully dropped.
-      .  1 - a temporary table was not found.
-      . -1 - a temporary table is used by an outer statement.
-    */
     if (table->open_type == OT_BASE_ONLY || !is_temporary_table(table))
       error= 1;
     else
     {
       table_creation_was_logged= table->table->s->table_creation_was_logged;
-      if ((error= drop_temporary_table(thd, table->table, &is_trans)) == -1)
+      if (thd->temporary_tables.drop_table(table->table, &is_trans, true))
       {
-        DBUG_ASSERT(thd->in_sub_stmt);
+        error= 1;
         goto err;
Why are changes in this hunk? (comment, assert, -1, etc)
...
}
       table->table= 0;
diff --git a/sql/table.h b/sql/table.h
index ab39603..8b7c665 100644
--- a/sql/table.h
+++ b/sql/table.h
@@ -600,6 +601,7 @@ struct TABLE_STATISTICS_CB
 struct TABLE_SHARE
 {
   TABLE_SHARE() {}                    /* Remove gcc warning */
+  TABLE_SHARE *next, *prev;
Eh? Did you forget to remove this?
...
/** Category of this table. */
   TABLE_CATEGORY table_category;
diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc
new file mode 100644
index 0000000..ed5b6b8
--- /dev/null
+++ b/sql/temporary_tables.cc
@@ -0,0 +1,1457 @@
+/*
+  Copyright (c) 2016 MariaDB Corporation
+
+  This program is free software; you can redistribute it and/or modify
+  it under the terms of the GNU General Public License as published by
+  the Free Software Foundation; version 2 of the License.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+*/
+
+#include "sql_acl.h"                            /* TMP_TABLE_ACLS */
+#include "sql_base.h"                           /* free_io_cache,
+                                                   tdc_create_key */
+#include "lock.h"                               /* mysql_lock_remove */
+#include "log_event.h"                          /* Query_log_event */
+#include "sql_show.h"                           /* append_identifier */
+#include "sql_handler.h"                        /* mysql_ha_rm_temporary_tables */
+#include "temporary_tables.h"                   /* Temporary_tables */
+#include "rpl_rli.h"                            /* rpl_group_info */
+
+#define IS_USER_TABLE(A) ((A->tmp_table == TRANSACTIONAL_TMP_TABLE) || \
+                          (A->tmp_table == NON_TRANSACTIONAL_TMP_TABLE))
+
+
+/*
+  Initialize the Temporary_tables object. Currently it always returns
+  false (success).
+
+  @param thd [IN]                     Thread handle
+
+  @return false                       Success
+          true                        Error
+*/
+bool Temporary_tables::init(THD *thd)
+{
+  DBUG_ENTER("Temporary_tables::init");
+  this->m_thd= thd;
+  DBUG_RETURN(false);
+}
+
+
+/*
+  Check whether temporary tables exist. The decision is made based on the
+  existence of TMP_TABLE_SHAREs.
+
+  @param check_slave [IN]             Also check the slave temporary tables.
+
+  @return false                       Temporary tables exist
+          true                        No temporary table exist
+*/
+bool Temporary_tables::is_empty(bool check_slave)
not a very good idea. You always use it as is_empty(true) or is_empty(false).
that is, the caller always knows whether to check slave or not.
but then you use a common function and it needs to do a completely
unnecessary  if() inside. Better to have two functions, like
is_empty() and is_empty_slave(). Or at least make this function inline, then
the compiler will have a chance to optimize it.
...
+{
+  DBUG_ENTER("Temporary_tables::is_empty");
+
+  bool result;
+
+  if (!m_thd)
+  {
+    DBUG_RETURN(true);
+  }
+
+  rpl_group_info *rgi_slave= m_thd->rgi_slave;
+
+  if (check_slave && rgi_slave)
+  {
+    result= (rgi_slave->rli->save_temp_table_shares == NULL) ? true : false;
+  }
+  else
+  {
+    result= (m_table_shares == NULL) ? true : false;
+  }
+
+  DBUG_RETURN(result);
+}
+
+
+ /*
+  Reset the Temporary_tables object. Currently, it always returns
+  false (success).
+
+  @return false                       Success
+          true                        Error
+*/
+bool Temporary_tables::reset()
+{
+  DBUG_ENTER("Temporary_tables::reset");
+  m_table_shares= 0;
+  DBUG_RETURN(false);
+}
+
+
+/*
+  Create a temporary table, open it and return the TABLE handle.
+
+  @param hton [IN]                    Handlerton
+  @param frm  [IN]                    Binary frm image
+  @param path [IN]                    File path (without extension)
+  @param db   [IN]                    Schema name
+  @param table_name [IN]              Table name
+  @param open_in_engine [IN]          Whether open table in SE
+  @param created [OUT]                Whether table was created?
can it ever happen for *create==true but a return value is NULL?
or the other way around? how?
...
+
+
+  @return Success                     A pointer to table object
+          Failure                     NULL
+*/
+TABLE *Temporary_tables::create_and_open_table(handlerton *hton,
+                                               LEX_CUSTRING *frm,
+                                               const char *path,
+                                               const char *db,
+                                               const char *table_name,
+                                               bool open_in_engine,
+                                               bool *created)
+{
+  DBUG_ENTER("Temporary_tables::create_and_open_table");
+
+  TMP_TABLE_SHARE *share;
+  TABLE *table= NULL;
+  bool locked;
+
+  *created= false;
+
+  if (wait_for_prior_commit())
+  {
+    DBUG_RETURN(NULL);
+  }
+
+  locked= lock_tables();
old code didn't seem to have it here. how comes?
...
+
+  if ((share= create_table(hton, frm, path, db, table_name)))
+  {
+    *created= true;
+    table= open_table(share, table_name, open_in_engine);
+  }
+
+  if (locked)
+  {
+    DBUG_ASSERT(m_locked);
+    unlock_tables();
+  }
+
+  DBUG_RETURN(table);
+}
+
+/*
+  Check whether an open table with db/table name is in use.
+
+  @param db [IN]                      Database name
+  @param table_name [IN]              Table name
+
+  @return Success                     Pointer to first used table instance.
+          Failure                     NULL
+*/
+TABLE *Temporary_tables::find_table(const char *db,
+                                    const char *table_name)
+{
+  DBUG_ENTER("Temporary_tables::find_table");
+
+  TABLE *table;
+  char key[MAX_DBKEY_LENGTH];
+  uint key_length;
+  bool locked;
+
+  if (is_empty(true))
+  {
+    DBUG_RETURN(NULL);
+  }
+
+  key_length= create_table_def_key(key, db, table_name);
+
+  if (wait_for_prior_commit())
+  {
+    DBUG_RETURN(NULL);
+  }
+
+  locked= lock_tables();
+  table = find_table(key, key_length, TABLE_IN_USE);
+  if (locked)
+  {
+    DBUG_ASSERT(m_locked);
+    unlock_tables();
+  }
+
+  DBUG_RETURN(table);
+}
+
+
+/*
+  Check whether an open table specified in TABLE_LIST is in use.
+
+  @return tl [IN]                     TABLE_LIST
+
+  @return Success                     Pointer to first used table instance.
+          Failure                     NULL
+*/
+TABLE *Temporary_tables::find_table(const TABLE_LIST *tl)
+{
+  DBUG_ENTER("Temporary_tables::find_table");
+  TABLE *table= find_table(tl->get_db_name(), tl->get_table_name());
+  DBUG_RETURN(table);
+}
+
+
+/*
+  Check whether an open table with the specified key is in use.
+  The key, in this case, is not the usual key used for temporary tables.
+  It does not contain server_id & pseudo_thread_id. This function is
+  essentially used use to check whether there is any temporary table
+  which _shadows_ a base table.
+  (see: Query_cache::send_result_to_client())
+
+  @return Success                     A pointer to table share object
+          Failure                     NULL
+*/
+TABLE *Temporary_tables::find_table_reduced_key_length(const char *key,
+                                                       uint key_length)
+{
+  DBUG_ENTER("Temporary_tables::find_table_reduced_key_length");
+
+  TABLE *result= NULL;
+  bool locked;
+
+  locked= lock_tables();
+
+  for (TMP_TABLE_SHARE *share= m_table_shares; share; share= share->next)
+  {
+    if ((share->table_cache_key.length - TMP_TABLE_KEY_EXTRA) == key_length
+        && !memcmp(share->table_cache_key.str, key, key_length))
+    {
+      /*
+        A matching TMP_TABLE_SHARE is found. We now need to find a TABLE
+        instance in use.
+      */
+      for (TABLE *table= share->table; table; table= table->next)
+      {
+        if (table->query_id != 0)
+        {
+          result= table;
+          break;
+        }
+      }
+    }
+  }
+
+  if (locked)
+  {
+    DBUG_ASSERT(m_locked);
+    unlock_tables();
+  }
+
+  DBUG_RETURN(result);
+}
+
+
+/*
+  Lookup the TMP_TABLE_SHARE using the given db/table_name.The server_id and
+  pseudo_thread_id used to generate table definition key is taken from m_thd
+  (see create_table_def_key()). Return NULL is none found.
+
+  @return Success                     A pointer to table share object
+          Failure                     NULL
+*/
+TMP_TABLE_SHARE *Temporary_tables::find_table_share(const char *db,
+                                                    const char *table_name)
+{
+  DBUG_ENTER("Temporary_tables::find_table_share");
+
+  TMP_TABLE_SHARE *share;
+  char key[MAX_DBKEY_LENGTH];
+  uint key_length;
+
+  key_length= create_table_def_key(key, db, table_name);
+  share= find_table_share(key, key_length);
+
+  DBUG_RETURN(share);
+}
+
+
+/*
+  Lookup TMP_TABLE_SHARE using the specified TABLE_LIST element.
+  Return NULL is none found.
+
+  @return Success                     A pointer to table share object
+          Failure                     NULL
+*/
+TMP_TABLE_SHARE *Temporary_tables::find_table_share(const TABLE_LIST *tl)
+{
+  DBUG_ENTER("Temporary_tables::find_table_share");
+  TMP_TABLE_SHARE *share= find_table_share(tl->get_db_name(),
+                                           tl->get_table_name());
+  DBUG_RETURN(share);
+}
+
+
+/*
+  Lookup TMP_TABLE_SHARE using the specified table definition key.
+  Return NULL is none found.
+
+  @return Success                     A pointer to table share object
+          Failure                     NULL
+*/
+TMP_TABLE_SHARE *Temporary_tables::find_table_share(const char *key,
+                                                    uint key_length)
+{
+  DBUG_ENTER("Temporary_tables::find_table_share");
+
+  TMP_TABLE_SHARE *share;
+  TMP_TABLE_SHARE *result= NULL;
+  bool locked;
+
+  if (wait_for_prior_commit())
+  {
+    DBUG_RETURN(NULL);
+  }
+
+  locked= lock_tables();
+
+  for (share= m_table_shares; share; share= share->next)
+  {
+    if (share->table_cache_key.length == key_length &&
+        !(memcmp(share->table_cache_key.str, key, key_length)))
+    {
+      result= share;
+      break;
+    }
+  }
+
+  if (locked)
+  {
+    DBUG_ASSERT(m_locked);
+    unlock_tables();
+  }
+
+  DBUG_RETURN(result);
+}
+
+
+/*
+  Find a temporary table specified by TABLE_LIST instance in the open table
+  list and prepare its TABLE instance for use. If
+
+  This function tries to resolve this table in the list of temporary tables
+  of this thread. Temporary tables are thread-local and "shadow" base
+  tables with the same name.
+
+  @note In most cases one should use Temporary_tables::open_tables() instead
+        of this call.
+
+  @note One should finalize process of opening temporary table for table
+        list element by calling open_and_process_table(). This function
+        is responsible for table version checking and handling of merge
+        tables.
+
+  @note We used to check global_read_lock before opening temporary tables.
+        However, that limitation was artificial and is removed now.
+
+  @param tl [IN]                      TABLE_LIST
+
+  @return Error status.
+    @retval false On success. If a temporary table exists for the given
+                  key, tl->table is set.
+    @retval TRUE  On error. my_error() has been called.
letter case is a bit weird, 'false', but 'TRUE' :)
...
+*/
+bool Temporary_tables::open_table(TABLE_LIST *tl)
+{
+  DBUG_ENTER("Temporary_tables::open_table");
+
+  TMP_TABLE_SHARE *share;
+  TABLE *table= NULL;
+  bool locked;
+
+  /*
+    Code in open_table() assumes that TABLE_LIST::table can be non-zero only
+    for pre-opened temporary tables.
+  */
+  DBUG_ASSERT(tl->table == NULL);
+
+  /*
+    This function should not be called for cases when derived or I_S
+    tables can be met since table list elements for such tables can
+    have invalid db or table name.
+    Instead Temporary_tables::open_tables() should be used.
+  */
+  DBUG_ASSERT(!tl->derived && !tl->schema_table);
+
+  if (tl->open_type == OT_BASE_ONLY || is_empty(true))
+  {
+    DBUG_PRINT("info", ("skip_temporary is set or no temporary tables"));
+    DBUG_RETURN(false);
+  }
+
+  if (wait_for_prior_commit())
this wasn't in the original code
...
+  {
+    DBUG_RETURN(true);
+  }
+
+  locked= lock_tables();
neither was this
...
+
+  /*
+    First check if there is a reusable open table available in the
+    open table list.
+  */
+  if (find_and_use_table(tl, &table))
+  {
+    if (locked)
+    {
+      DBUG_ASSERT(m_locked);
+      unlock_tables();
+    }
+    DBUG_RETURN(true);                          /* Error */
+  }
+
+  /*
+    No reusable table was found. We will have to open a new instance.
+  */
+  if (!table && (share= find_table_share(tl)))
+  {
+    table= open_table(share, tl->get_table_name(), true);
+  }
+
+  if (locked)
+  {
+    DBUG_ASSERT(m_locked);
+    unlock_tables();
+  }
+
+  if (!table)
+  {
+    if (tl->open_type == OT_TEMPORARY_ONLY &&
+        tl->open_strategy == TABLE_LIST::OPEN_NORMAL)
+    {
+      my_error(ER_NO_SUCH_TABLE, MYF(0), tl->db, tl->table_name);
+      DBUG_RETURN(true);
+    }
+    DBUG_RETURN(false);
+  }
+
+#ifdef WITH_PARTITION_STORAGE_ENGINE
+  if (tl->partition_names)
+  {
+    /* Partitioned temporary tables is not supported. */
+    DBUG_ASSERT(!table->part_info);
+    my_error(ER_PARTITION_CLAUSE_ON_NONPARTITIONED, MYF(0));
+    DBUG_RETURN(true);
+  }
+#endif
+
+  table->query_id= m_thd->query_id;
+  m_thd->thread_specific_used= true;
+
+  /* It is neither a derived table nor non-updatable view. */
+  tl->updatable= true;
+  tl->table= table;
+
+  table->init(m_thd, tl);
+
+  DBUG_PRINT("info", ("Using temporary table"));
+  DBUG_RETURN(false);
+}
+
+
+/*
+  Pre-open temporary tables corresponding to table list elements.
+
+  @note One should finalize process of opening temporary tables
+        by calling open_tables(). This function is responsible
+        for table version checking and handling of merge tables.
what kind of version checking can there be for temporary tables?
...
+
+  @param tl [IN]                      TABLE_LIST
+
+  @return false                       On success. If a temporary table exists
+                                      for the given element, tl->table is set.
+          true                        On error. my_error() has been called.
+*/
+bool Temporary_tables::open_tables(TABLE_LIST *tl)
+{
+  DBUG_ENTER("Temporary_tables::open_tables");
+
+  TABLE_LIST *first_not_own= m_thd->lex->first_not_own_table();
+
+  for (TABLE_LIST *table= tl; table && table != first_not_own;
+       table= table->next_global)
+  {
+    if (table->derived || table->schema_table)
+    {
+      /*
+        Derived and I_S tables will be handled by a later call to open_tables().
+      */
+      continue;
+    }
+
+    if ((m_thd->temporary_tables.open_table(table)))
eh? couldn't you simply do

  if (open_table(table))

? why do you need this "m_thd->temporary_tables." ?
...
+    {
+      DBUG_RETURN(true);
+    }
+  }
+
+  DBUG_RETURN(false);
+}
+
+
+/*
+  Close all temporary tables created by 'CREATE TEMPORARY TABLE' for thread
+  creates one DROP TEMPORARY TABLE binlog event for each pseudo-thread.
+
+  Temporary tables created in a sql slave is closed by
+  Relay_log_info::close_temporary_tables().
+
+  @return false                       Success
+          true                        Failure
+*/
+bool Temporary_tables::close_tables()
+{
+  DBUG_ENTER("Temporary_tables::close_tables");
+
+  TMP_TABLE_SHARE *share;
+  TMP_TABLE_SHARE *share_next;
+  TABLE *table, *table_next;
+  bool error= false;
+
+  if (!m_table_shares)
+  {
+    DBUG_RETURN(false);
+  }
+  DBUG_ASSERT(!m_thd->rgi_slave);
+
+  /*
+    Ensure we don't have open HANDLERs for tables we are about to close.
+    This is necessary when Temporary_tables::close_tables() is called as
+    part of execution of BINLOG statement (e.g. for format description event).
+  */
+  mysql_ha_rm_temporary_tables(m_thd);
+
+  /* Close all open temporary tables. */
+  for (TMP_TABLE_SHARE *share= m_table_shares; share; share= share->next)
+  {
+    /* Traverse the table list. */
+    table= share->table;
+    while (table)
+    {
+      table_next= table->next;
+      free_table(table);
+      table= table_next;
+    }
+  }
+
+  // Write DROP TEMPORARY TABLE query log events to binary log.
+  if (mysql_bin_log.is_open())
+  {
+    error= log_events_and_free_shares();
+  }
+  else
+  {
+    share= m_table_shares;
+    while (share)
+    {
+      share_next= share->next;
+      free_table_share(share, true);
+      share= share_next;
+    }
+  }
+  reset();
+
+  DBUG_RETURN(error);
+}
+
+
+/*
+  Rename a temporary table.
+
+  @param table [IN]                   Table handle
+  @param db [IN]                      New schema name
+  @param table_name [IN]              New table name
+
+  @return false                       Success
+          true                        Error
+*/
+bool Temporary_tables::rename_table(TABLE *table,
+                                    const char *db,
+                                    const char *table_name)
+{
+  DBUG_ENTER("Temporary_tables::rename_table");
+
+  char *key;
+  uint key_length;
+
+  TMP_TABLE_SHARE *share= static_cast<TMP_TABLE_SHARE *>(table->s);
+
+  if (!(key= (char *) alloc_root(&share->mem_root, MAX_DBKEY_LENGTH)))
+  {
+    DBUG_RETURN(true);
+  }
+
+  /*
+    Temporary tables are renamed by simply changing their table definition key.
+  */
+  key_length= create_table_def_key(key, db, table_name);
+  share->set_table_cache_key(key, key_length);
+
+  DBUG_RETURN(false);
+}
+
+
+/*
+  Drop a temporary table.
+
+  Try to locate the table in the list of open temporary tables.
+  If the table is found:
+   - If the table is locked with LOCK TABLES or by prelocking,
+     unlock it and remove it from the list of locked tables
+     (THD::lock). Currently only transactional temporary tables
+     are locked.
+   - Close the temporary table, remove its .FRM.
+   - Remove the table share from the list of temporary table shares.
+
+  This function is used to drop user temporary tables, as well as
+  internal tables created in CREATE TEMPORARY TABLE ... SELECT
+  or ALTER TABLE.
+
+  @param table [IN]                   Temporary table to be deleted
+  @param is_trans [OUT]               Is set to the type of the table:
+                                      transactional (e.g. innodb) as true or
+                                      non-transactional (e.g. myisam) as false.
+  @paral delete_table [IN]            Whether to delete the table files.
+
+  @return false                       Table was dropped
+          true                        Error
+*/
+bool Temporary_tables::drop_table(TABLE *table,
+                                  bool *is_trans,
+                                  bool delete_table)
+{
+  DBUG_ENTER("Temporary_tables::drop_table");
+
+  TMP_TABLE_SHARE *share;
+  TABLE *tab;
+  TABLE *tab_next;
+  bool result, locked;
+
+  DBUG_ASSERT(table);
+  DBUG_ASSERT(table->query_id);
+  DBUG_PRINT("tmptable", ("Dropping table: '%s'.'%s'",
+                          table->s->db.str, table->s->table_name.str));
+
+  if (wait_for_prior_commit())
+  {
+    DBUG_RETURN(true);
+  }
old code didn't seem to have that. Why do you?
...
+
+  locked= lock_tables();
+
+  share= static_cast<TMP_TABLE_SHARE *>(table->s);
suggestion: have a function:

  static inline TMP_TABLE_SHARE *tmp_table_share(TABLE *t) {
     DBUG_ASSERT(t->s->tmp_table);
     return static_cast<TMP_TABLE_SHARE *>(table->s);
  }
...
+
+  /* Table might be in use by some outer statement. */
+  for (tab= share->table; tab; tab= tab->next)
+  {
+    if (tab != table && tab->query_id != 0)
+    {
+      /* Found a table instance in use. This table cannot be be dropped. */
+      my_error(ER_CANT_REOPEN_TABLE, MYF(0), table->alias.c_ptr());
+      result= true;
+      goto end;
+    }
+  }
+
+  if (is_trans)
+    *is_trans= table->file->has_transactions();
+
+  /*
+    Iterate over the list of open tables and close all tables referencing the
+    same table share.
the comment sounds a bit odd now, when all open tables are in the
list inside the share.
...
+  */
+  tab= share->table;
+  while (tab)
+  {
+    tab_next= tab->next;
+    if ((result= free_table(tab))) goto end;
+    tab= tab_next;
+  }
+
+  result= free_table_share(share, delete_table);
+
+end:
+  if (locked)
+  {
+    DBUG_ASSERT(m_locked);
+    unlock_tables();
+  }
+
+  DBUG_RETURN(result);
+}
+
+
+/**
+  Delete the temporary table files.
+
+  @param base [IN]                    Handlerton for table to be deleted.
+  @param path [IN]                    Path to the table to be deleted (i.e. path
+                                      to its .frm without an extension).
+
+  @return false                       Success
+          true                        Error
+*/
+bool Temporary_tables::remove_table(handlerton *base, const char *path)
+{
+  DBUG_ENTER("Temporary_tables::remove_table");
+
+  bool error= false;
+  handler *file;
+  char frm_path[FN_REFLEN + 1];
+
+  strxnmov(frm_path, sizeof(frm_path) - 1, path, reg_ext, NullS);
+  if (mysql_file_delete(key_file_frm, frm_path, MYF(0)))
+  {
+    error= true;
+  }
+  file= get_new_handler((TABLE_SHARE*) 0, current_thd->mem_root, base);
+  if (file && file->ha_delete_table(path))
+  {
+    error= true;
+    sql_print_warning("Could not remove temporary table: '%s', error: %d",
+                      path, my_errno);
+  }
+
+  delete file;
+  DBUG_RETURN(error);
+}
+
+
+/*
+  Mark all temporary tables which were used by the current statement or
+  sub-statement as free for reuse, but only if the query_id can be cleared.
+
+  @remark For temp tables associated with a open SQL HANDLER the query_id
+          is not reset until the HANDLER is closed.
+*/
+void Temporary_tables::mark_tables_as_free_for_reuse()
+{
+  DBUG_ENTER("Temporary_tables::mark_tables_as_free_for_reuse");
+
+  bool locked;
+
+  if (m_thd->query_id == 0)
+  {
+    /*
+      Thread has not executed any statement and has not used any
+      temporary tables.
+    */
+    DBUG_VOID_RETURN;
+  }
+
+  locked= lock_tables();
+
+  for (TMP_TABLE_SHARE *share= m_table_shares; share; share= share->next)
+  {
+    for (TABLE *table= share->table; table; table= table->next)
+    {
+      if ((table->query_id == m_thd->query_id) && !table->open_by_handler)
+      {
+        mark_table_as_free_for_reuse(table);
+      }
+    }
+  }
+
+  if (locked)
+  {
+    DBUG_ASSERT(m_locked);
+    unlock_tables();
+  }
+
+  DBUG_VOID_RETURN;
+}
+
+
+/*
+  Reset a single temporary table. Effectively this "closes" one temporary
+  table in a session.
+
+  @param table              Temporary table
+*/
+void Temporary_tables::mark_table_as_free_for_reuse(TABLE *table)
+{
+  DBUG_ENTER("Temporary_tables::mark_table_as_free_for_reuse");
+
+  DBUG_ASSERT(table->s->tmp_table);
+
+  table->query_id= 0;
+  table->file->ha_reset();
+
+  /* Detach temporary MERGE children from temporary parent. */
+  DBUG_ASSERT(table->file);
+  table->file->extra(HA_EXTRA_DETACH_CHILDREN);
+
+  /*
+    Reset temporary table lock type to it's default value (TL_WRITE).
+
+    Statements such as INSERT INTO .. SELECT FROM tmp, CREATE TABLE
+    .. SELECT FROM tmp and UPDATE may under some circumstances modify
+    the lock type of the tables participating in the statement. This
+    isn't a problem for non-temporary tables since their lock type is
+    reset at every open, but the same does not occur for temporary
+    tables for historical reasons.
+
+    Furthermore, the lock type of temporary tables is not really that
+    important because they can only be used by one query at a time.
+    Nonetheless, it's safer from a maintenance point of view to reset
+    the lock type of this singleton TABLE object as to not cause problems
+    when the table is reused.
+
+    Even under LOCK TABLES mode its okay to reset the lock type as
+    LOCK TABLES is allowed (but ignored) for a temporary table.
+  */
+  table->reginfo.lock_type= TL_WRITE;
+  DBUG_VOID_RETURN;
+}
+
+
+TMP_TABLE_SHARE *Temporary_tables::unlink_table_share(TABLE_SHARE *share)
+{
+  DBUG_ENTER("Temporary_tables::unlink_table");
+  TMP_TABLE_SHARE *tmp_table_share;
+
+  lock_tables();
+  tmp_table_share= static_cast<TMP_TABLE_SHARE *>(share);
+  unlink<TMP_TABLE_SHARE>(&m_table_shares, tmp_table_share);
+  unlock_tables();
+
+  DBUG_RETURN(tmp_table_share);
+}
+
+
+void Temporary_tables::relink_table_share(TMP_TABLE_SHARE *share)
+{
+  DBUG_ENTER("Temporary_tables::relink_table");
+
+  lock_tables();
+  link<TMP_TABLE_SHARE>(&m_table_shares, share);
+  unlock_tables();
+
+  DBUG_VOID_RETURN;
+}
+
+
+/*
+  Create a table definition key.
+
+  @param key [OUT]                    Buffer for the key to be created (must
+                                      be of size MAX_DBKRY_LENGTH)
+  @param db [IN]                      Database name
+  @param table_name [IN]              Table name
+
+  @return                             Key length.
+
+  @note
+    The table key is create from:
+    db + \0
+    table_name + \0
+
+    Additionally, we add the following to make each temporary table unique on
+    the slave.
+
+    4 bytes of master thread id
+    4 bytes of pseudo thread id
+*/
+
+uint Temporary_tables::create_table_def_key(char *key, const char *db,
+                                            const char *table_name)
+{
+  DBUG_ENTER("Temporary_tables::create_table_def_key");
+
+  uint key_length;
+
+  key_length= tdc_create_key(key, db, table_name);
+  int4store(key + key_length, m_thd->variables.server_id);
+  int4store(key + key_length + 4, m_thd->variables.pseudo_thread_id);
+  key_length += TMP_TABLE_KEY_EXTRA;
+
+  DBUG_RETURN(key_length);
+}
+
+
+/*
+  Create a temporary table.
+
+  @param hton [IN]                    Handlerton
+  @param frm  [IN]                    Binary frm image
+  @param path [IN]                    File path (without extension)
+  @param db   [IN]                    Schema name
+  @param table_name [IN]              Table name
+
+  @return Success                     A pointer to table share object
+          Failure                     NULL
+*/
+TMP_TABLE_SHARE *Temporary_tables::create_table(handlerton *hton,
+                                                LEX_CUSTRING *frm,
+                                                const char *path,
+                                                const char *db,
+                                                const char *table_name)
+{
+  DBUG_ENTER("Temporary_tables::create_table");
+
+  TMP_TABLE_SHARE *share;
+  char key_cache[MAX_DBKEY_LENGTH];
+  char *saved_key_cache;
+  char *tmp_path;
+  uint key_length;
+  int res;
+
+  if (wait_for_prior_commit())
you're doing it in almost every method. And when one method calls another,
you're doing it twice. Or thrice. Isn't is too much? (I don't know)
...
+  {
+    DBUG_RETURN(NULL);
+  }
+
+  /* Create the table definition key for the temporary table. */
+  key_length= create_table_def_key(key_cache, db, table_name);
+
+  if (!(share= (TMP_TABLE_SHARE *) my_malloc(sizeof(TMP_TABLE_SHARE) +
+                                             strlen(path) +
+                                             1 + key_length, MYF(MY_WME))))
+  {
+    DBUG_RETURN(NULL);                          /* Out of memory */
+  }
+
+  tmp_path= (char *)(share + 1);
+  saved_key_cache= strmov(tmp_path, path) + 1;
+  memcpy(saved_key_cache, key_cache, key_length);
+
+  init_tmp_table_share(m_thd, share, saved_key_cache, key_length,
+                       strend(saved_key_cache) + 1, tmp_path);
+
+  share->table= 0;
+  share->db_plugin= ha_lock_engine(m_thd, hton);
+
+  /*
+    Prefer using frm image over file. The image might not be available in
+    ALTER TABLE, when the discovering engine took over the ownership (see
+    TABLE::read_frm_image).
+  */
+  res= (frm->str)
+    ? share->init_from_binary_frm_image(m_thd, false, frm->str, frm->length)
+    : open_table_def(m_thd, share, GTS_TABLE | GTS_USE_DISCOVERY);
+
+  if (res)
+  {
+    /*
+      No need to lock share->mutex as this is not needed for temporary tables.
+    */
+    ::free_table_share(share);
+    my_free(share);
+    DBUG_RETURN(NULL);
+  }
+
+  share->m_psi= PSI_CALL_get_table_share(true, share);
+
+  /* Add share to the head of the temporary table share list. */
+  link<TMP_TABLE_SHARE>(&m_table_shares, share);
+
+  DBUG_RETURN(share);
+}
+
+
+/*
+  Find a table with the specified key.
+
+  @param key [IN]                     Key
+  @param key_length [IN]              Key length
+  @param state [IN]                   Open table state to look for
+
+  @return Success                     Pointer to the table instance.
+          Failure                     NULL
+*/
+TABLE *Temporary_tables::find_table(const char *key, uint key_length,
+                                    Table_state state)
+{
+  DBUG_ENTER("Temporary_tables::find_table");
+
+  for (TMP_TABLE_SHARE *share= m_table_shares; share; share= share->next)
+  {
+    if (share->table_cache_key.length == key_length &&
+        !(memcmp(share->table_cache_key.str, key, key_length)))
+    {
+      /* A matching TMP_TABLE_SHARE is found. */
+      for (TABLE *table= share->table; table; table= table->next)
+      {
+        switch (state)
+        {
+        case TABLE_IN_USE:
+          if (table->query_id > 0) DBUG_RETURN(table);
+          break;
+        case TABLE_NOT_IN_USE:
+          if (table->query_id == 0) DBUG_RETURN(table);
+          break;
+        case TABLE_ANY:
+          DBUG_RETURN(table);
+        default:                                /* Invalid */
+          DBUG_ASSERT(0);
+          DBUG_RETURN(NULL);
+        }
+      }
+    }
+  }
+
+  DBUG_RETURN(NULL);
+}
+
+
+/*
+  Find a reusable table in the open table list using the specified TABLE_LIST.
+
+  @param tl [IN]                      Table list
+  @param out_table [OUT]              Pointer to the requested TABLE object
+
+  @return Success                     false
+          Failure                     true
+*/
+bool Temporary_tables::find_and_use_table(const TABLE_LIST *tl,
+                                          TABLE **out_table)
+{
+  DBUG_ENTER("Temporary_tables::find_and_use_table");
+
+  char key[MAX_DBKEY_LENGTH];
+  uint key_length;
+  bool result;
+
+  key_length= create_table_def_key(key, tl->get_db_name(),
+                                   tl->get_table_name());
+  result= use_table(find_table(key, key_length, TABLE_NOT_IN_USE), out_table);
+
+  DBUG_RETURN(result);
+}
+
+
+/*
+  Open a table from the specified TABLE_SHARE with the given alias.
+
+  @param share [IN]                   Table share
+  @param alias [IN]                   Table alias
+  @param open_in_engine [IN]          Whether open table in SE
+
+  @return Success                     A pointer to table object
+          Failure                     NULL
+*/
+TABLE *Temporary_tables::open_table(TMP_TABLE_SHARE *share,
+                                    const char *alias,
+                                    bool open_in_engine)
+{
+  DBUG_ENTER("Temporary_tables::open_table");
+
+  TABLE *table;
+
+  if (wait_for_prior_commit())
+  {
+    DBUG_RETURN(NULL);
+  }
+
+  if (!(table= (TABLE *) my_malloc(sizeof(TABLE), MYF(MY_WME))))
+  {
+    DBUG_RETURN(NULL);                          /* Out of memory */
+  }
+
+  if (open_table_from_share(m_thd, share, alias,
+                            (open_in_engine) ?
+                            (uint) (HA_OPEN_KEYFILE | HA_OPEN_RNDFILE |
+                                    HA_GET_INDEX) : 0,
+                            (uint) (READ_KEYINFO | COMPUTE_TYPES |
+                                    EXTRA_RECORD),
+                            ha_open_options,
+                            table,
+                            open_in_engine ? false : true))
+  {
+    my_free(table);
+    DBUG_RETURN(NULL);
+  }
+
+  table->reginfo.lock_type= TL_WRITE;           /* Simulate locked */
+  table->grant.privilege= TMP_TABLE_ACLS;
+  share->tmp_table= (table->file->has_transactions() ?
+                     TRANSACTIONAL_TMP_TABLE : NON_TRANSACTIONAL_TMP_TABLE);
+
+  table->pos_in_table_list= 0;
+  table->query_id= m_thd->query_id;
+
+  /* Add table to the head of table list. */
+  link<TABLE>(&share->table, table);
I don't know if that's correct. You use TABLE::next and TABLE::prev pointers
to link all tables of a given TMP_TABLE_SHARE into a list. But these
pointers are used to link all tables into a THD::open_tables list
(and may be for other purposes too?).
In fact, TABLE has dedicated pointers for this list that you want:

 /**
    Links for the list of all TABLE objects for this share.
    Declared as private to avoid direct manipulation with those objects.
    One should use methods of I_P_List template instead.
 */
 TABLE *share_all_next, **share_all_prev;
...
+
+  /* Increment Slave_open_temp_table_definitions status variable count. */
+  if (m_thd->rgi_slave)
+  {
+    thread_safe_increment32(&slave_open_temp_tables);
+  }
+
+  DBUG_PRINT("tmptable", ("Opened table: '%s'.'%s' 0x%lx", table->s->db.str,
+                          table->s->table_name.str, (long) table));
+  DBUG_RETURN(table);
+}
+
+
function comment?
...
+bool Temporary_tables::use_table(TABLE *table, TABLE **out_table)
+{
+  DBUG_ENTER("Temporary_tables::use_table");
+
+  *out_table= table;
+  if (!table)
+    DBUG_RETURN(false);
+
old code had a block here about "Temporary tables are not safe for parallel
replication". it's in wait_for_prior_commit now, but you aren't calling it
here. why?
...
+  /*
+    We need to set the THD as it may be different in case of
+    parallel replication
+  */
+  if (table->in_use != m_thd)
+  {
+    table->in_use= m_thd;
+  }
+
+  DBUG_RETURN(false);
+}
+
+
+/*
+  Close a temporary table.
+
+  @param table [IN]                   Table handle
+
+  @return false                       Success
+          true                        Error
+*/
+bool Temporary_tables::close_table(TABLE *table)
+{
+  DBUG_ENTER("Temporary_tables::close_table");
+
+  DBUG_PRINT("tmptable", ("closing table: '%s'.'%s' 0x%lx  alias: '%s'",
+                          table->s->db.str, table->s->table_name.str,
+                          (long) table, table->alias.c_ptr()));
+
+  free_io_cache(table);
don't forget to remove free_io_cache() calls when rebasing your
work on top of the latest 10.2
(they were removed from close_temporary() in 260dd476b05 commit)
...
+  closefrm(table, false);
+  my_free(table);
+
+  /* Decrement Slave_open_temp_table_definitions status variable count. */
+  if (m_thd->rgi_slave)
+  {
+    thread_safe_decrement32(&slave_open_temp_tables);
+  }
+
+  DBUG_RETURN(false);
+}
+
+
+bool Temporary_tables::wait_for_prior_commit()
+{
+  DBUG_ENTER("Temporary_tables::wait_for_prior_commit");
+
+  /*
+    Temporary tables are not safe for parallel replication. They were
+    designed to be visible to one thread only, so have no table locking.
+    Thus there is no protection against two conflicting transactions
+    committing in parallel and things like that.
+
+    So for now, anything that uses temporary tables will be serialised
+    with anything before it, when using parallel replication.
+
+    TODO: We might be able to introduce a reference count or something
+    on temp tables, and have slave worker threads wait for it to reach
+    zero before being allowed to use the temp table. Might not be worth
+    it though, as statement-based replication using temporary tables is
+    in any case rather fragile.
+  */
+  if (m_thd->rgi_slave &&
+      m_thd->rgi_slave->rli->save_temp_table_shares &&
+      m_thd->rgi_slave->is_parallel_exec &&
+      m_thd->wait_for_prior_commit())
+    DBUG_RETURN(true);
+
+  DBUG_RETURN(false);
+}
+
+
+/*
+  Write query log events with "DROP TEMPORARY TABLES .." for each pseudo
+  thread to the binary log.
+
+  @return false                       Success
+          true                        Error
+*/
+bool Temporary_tables::log_events_and_free_shares()
+{
+  DBUG_ENTER("Temporary_tables::log_events_and_free_shares");
+
+  DBUG_ASSERT(!m_thd->rgi_slave);
+
+  TMP_TABLE_SHARE *share;
+  TMP_TABLE_SHARE *next;
+  TMP_TABLE_SHARE *prev_share;
+  // Assume thd->variables.option_bits has OPTION_QUOTE_SHOW_CREATE.
+  bool was_quote_show= true;
+  bool error= false;
+  bool found_user_tables= false;
+  // Better add "IF EXISTS" in case a RESET MASTER has been done.
+  const char stub[]= "DROP /*!40005 TEMPORARY */ TABLE IF EXISTS ";
+  char buf[FN_REFLEN];
+
+  String s_query(buf, sizeof(buf), system_charset_info);
+  s_query.copy(stub, sizeof(stub) - 1, system_charset_info);
+
+  /*
+    Insertion sort of temporary tables by pseudo_thread_id to build ordered
+    list of sublists of equal pseudo_thread_id.
+  */
+
+  for (prev_share= m_table_shares, share= prev_share->next;
+       share;
+       prev_share= share, share= share->next)
+  {
+    TMP_TABLE_SHARE *prev_sorted;                   /* Same as for prev_share */
+    TMP_TABLE_SHARE *sorted;
+
+    if (IS_USER_TABLE(share))
+    {
+      if (!found_user_tables)
+        found_user_tables= true;
+
+      for (prev_sorted= NULL, sorted= m_table_shares;
+           sorted != share;
+           prev_sorted= sorted, sorted= sorted->next)
+      {
+        if (!IS_USER_TABLE(sorted) ||
+            tmpkeyval(sorted) > tmpkeyval(share))
+        {
+          /*
+            Move into the sorted part of the list from the unsorted.
+          */
+          prev_share->next= share->next;
+          share->next= sorted;
+          if (prev_sorted)
+          {
+            prev_sorted->next= share;
+          }
+          else
+          {
+            m_table_shares= share;
+          }
+          share= prev_share;
+          break;
+        }
+      }
+    }
+  }
+
+  /*
+    We always quote db & table names.
+  */
+  if (found_user_tables &&
+      !(was_quote_show= MY_TEST(m_thd->variables.option_bits &
+                                OPTION_QUOTE_SHOW_CREATE)))
+  {
+    m_thd->variables.option_bits |= OPTION_QUOTE_SHOW_CREATE;
+  }
+
+  /*
+    Scan sorted temporary tables to generate sequence of DROP.
+  */
+  for (share= m_table_shares; share; share= next)
+  {
+    if (IS_USER_TABLE(share))
+    {
+      bool save_thread_specific_used= m_thd->thread_specific_used;
+      my_thread_id save_pseudo_thread_id= m_thd->variables.pseudo_thread_id;
+      char db_buf[FN_REFLEN];
+      String db(db_buf, sizeof(db_buf), system_charset_info);
+
+      /*
+        Set pseudo_thread_id to be that of the processed table.
+      */
+      m_thd->variables.pseudo_thread_id= tmpkeyval(share);
+
+      db.copy(share->db.str, share->db.length, system_charset_info);
+      /*
+        Reset s_query() if changed by previous loop.
+      */
+      s_query.length(sizeof(stub) - 1);
+
+      /*
+        Loop forward through all tables that belong to a common database
+        within the sublist of common pseudo_thread_id to create single
+        DROP query.
+      */
+      for (;
+           share &&
+           IS_USER_TABLE(share) &&
+           tmpkeyval(share) == m_thd->variables.pseudo_thread_id &&
+           share->db.length == db.length() &&
+           memcmp(share->db.str, db.ptr(), db.length()) == 0;
+           share= next)
+      {
+        /*
+          We are going to add ` around the table names and possible more
+          due to special characters.
+        */
+        append_identifier(m_thd, &s_query, share->table_name.str,
+                          share->table_name.length);
+        s_query.append(',');
+        next= share->next;
+        remove_table(share->db_type(), share->path.str);
+        ::free_table_share(share);
+        my_free(share);
+      }
+
+      m_thd->clear_error();
+      CHARSET_INFO *cs_save= m_thd->variables.character_set_client;
+      m_thd->variables.character_set_client= system_charset_info;
+      m_thd->thread_specific_used= true;
+
+      Query_log_event qinfo(m_thd, s_query.ptr(),
+                            s_query.length() - 1 /* to remove trailing ',' */,
+                            false, true, false, 0);
+      qinfo.db= db.ptr();
+      qinfo.db_len= db.length();
+      m_thd->variables.character_set_client= cs_save;
+
+      m_thd->get_stmt_da()->set_overwrite_status(true);
+      if ((error= (mysql_bin_log.write(&qinfo) || error)))
+      {
+        /*
+          If we're here following THD::cleanup, thence the connection
+          has been closed already. So lets print a message to the
+          error log instead of pushing yet another error into the
+          stmt_da.
+
+          Also, we keep the error flag so that we propagate the error
+          up in the stack. This way, if we're the SQL thread we notice
+          that Temporary_tables::close_tables failed. (Actually, the SQL
+          thread only calls Temporary_tables::close_tables while applying
+          old Start_log_event_v3 events.)
+        */
+        sql_print_error("Failed to write the DROP statement for "
+                        "temporary tables to binary log");
+      }
+
+      m_thd->get_stmt_da()->set_overwrite_status(false);
+      m_thd->variables.pseudo_thread_id= save_pseudo_thread_id;
+      m_thd->thread_specific_used= save_thread_specific_used;
+    }
+    else
+    {
+      next= share->next;
+      free_table_share(share, true);
+    }
+  }
+
+  if (!was_quote_show)
+  {
+    /*
+      Restore option.
+    */
+    m_thd->variables.option_bits&= ~OPTION_QUOTE_SHOW_CREATE;
+  }
+
+  DBUG_RETURN(error);
+}
+
+/*
+  Delete the files and free the specified table share.
+*/
+bool Temporary_tables::free_table_share(TMP_TABLE_SHARE *share,
+                                        bool delete_table)
+{
+  DBUG_ENTER("Temporary_tables::free_table_share");
+
+  bool result= false;
+
+  /* Delete the share from table share list */
+  unlink<TMP_TABLE_SHARE>(&m_table_shares, share);
+
+  if (delete_table)
+  {
+    result= remove_table(share->db_type(), share->path.str);
+  }
+
+  ::free_table_share(share);
+  my_free(share);
+
+  DBUG_RETURN(result);
+}
+
+
+/*
+  Free the specified table object.
+*/
+bool Temporary_tables::free_table(TABLE *table)
+{
+  DBUG_ENTER("Temporary_tables::free_table");
+
+  bool result= false;
+
+  /* Delete the table from table list */
+  unlink<TABLE>(&static_cast<TMP_TABLE_SHARE *>(table->s)->table, table);
+
+  /*
+    If LOCK TABLES list is not empty and contains this table, unlock the table
+    and remove the table from this list.
+  */
+  mysql_lock_remove(m_thd, m_thd->lock, table);
+
+  result= close_table(table);
+
+  DBUG_RETURN(result);
+}
+
+
+bool Temporary_tables::lock_tables()
+{
+  /* Do not proceed if a lock has already been taken. */
+  if (m_locked)
+  {
+    return false;
+  }
+
+  rpl_group_info *rgi_slave= m_thd->rgi_slave;
+  if (rgi_slave)
+  {
+    mysql_mutex_lock(&rgi_slave->rli->data_lock);
+    m_table_shares= rgi_slave->rli->save_temp_table_shares;
+    m_locked= true;
+  }
+  return m_locked;
+}
+
+
+void Temporary_tables::unlock_tables()
+{
+  if (!m_locked)
+  {
+    return;
+  }
+
+  rpl_group_info *rgi_slave= m_thd->rgi_slave;
+  if (rgi_slave)
+  {
+    rgi_slave->rli->save_temp_table_shares= m_table_shares;
+    mysql_mutex_unlock(&rgi_slave->rli->data_lock);
+    m_locked= false;
+    /*
+      Temporary tables are shared with other by sql execution threads.
+      As a safety measure, clear the pointer to the common area.
+    */
+    reset();
+  }
+  return;
+}
+
diff --git a/sql/temporary_tables.h b/sql/temporary_tables.h
new file mode 100644
index 0000000..5a5ed0d
--- /dev/null
+++ b/sql/temporary_tables.h
@@ -0,0 +1,150 @@
+#ifndef TEMPORARY_TABLES_H
+#define TEMPORARY_TABLES_H
+/*
+  Copyright (c) 2016 MariaDB Corporation
+
+  This program is free software; you can redistribute it and/or modify
+  it under the terms of the GNU General Public License as published by
+  the Free Software Foundation; version 2 of the License.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+*/
+
+#define TMP_TABLE_KEY_EXTRA 8
+
+struct TMP_TABLE_SHARE: public TABLE_SHARE
+{
+  /* List of TABLE instances created out of this TABLE_SHARE. */
+  TABLE *table;
Hmm, if you will use TABLE::share_all_prev and TABLE::share_all_next, as
I've written above, you should also use TDC_element that keeps the list
of TABLE's per TABLE_SHARE. And also it has next/prev pointers
to link all TABLE_SHARE's in a list... Perhaps you won't need
TMP_TABLE_SHARE after all? Could you talk with Svoj about it, please?
perhaps you can simply reuse existing TABLE_SHARE code
for linking TABLE_SHARE's into a list and linking TABLE's into
a per-TABLE_SHARE list? TDC_element is a bit of an overkill for
temporary tables, we can either live with that or extract those
bits that you need into a "mini-TDC_element" ? Dunno, best to discuss it
with Svoj.
...
+
+  /* Pointers to access TMP_TABLE_SHARE instances. */
+  TMP_TABLE_SHARE *next;
+  TMP_TABLE_SHARE *prev;
+};
+
+
+class Temporary_tables
+{
+public:
+  Temporary_tables() : m_thd(0), m_table_shares(0), m_locked(false)
+  {}
+  bool init(THD *thd);
+  bool is_empty(bool check_slave);
+  bool reset();
+
+  TABLE *create_and_open_table(handlerton *hton, LEX_CUSTRING *frm,
+                               const char *path, const char *db,
+                               const char *table_name, bool open_in_engine,
+                               bool *created);
+
+  TABLE *find_table(const char *db, const char *table_name);
+  TABLE *find_table(const TABLE_LIST *tl);
+  TABLE *find_table_reduced_key_length(const char *key, uint key_length);
+
+  TMP_TABLE_SHARE *find_table_share(const char *db, const char *table_name);
+  TMP_TABLE_SHARE *find_table_share(const TABLE_LIST *tl);
+  TMP_TABLE_SHARE *find_table_share(const char *key, uint key_length);
+
+  bool open_table(TABLE_LIST *tl);
+  bool open_tables(TABLE_LIST *tl);
+
+  bool close_tables();
+  bool rename_table(TABLE *table, const char *db, const char *table_name);
+  bool drop_table(TABLE *table, bool *is_trans, bool delete_table);
+  bool remove_table(handlerton *hton, const char *path);
+  void mark_tables_as_free_for_reuse();
+  void mark_table_as_free_for_reuse(TABLE *table);
+
+  TMP_TABLE_SHARE *unlink_table_share(TABLE_SHARE *share);
+  void relink_table_share(TMP_TABLE_SHARE *share);
+
+private:
+  /* THD handler */
+  THD *m_thd;
already discussed
...
+
+  /* List of temporary table shares */
+  TMP_TABLE_SHARE *m_table_shares;
+
+  /* Whether a lock has been acquired. */
+  bool m_locked;
+
+  /* Opened table states. */
+  enum Table_state {
+    TABLE_IN_USE,
+    TABLE_NOT_IN_USE,
+    TABLE_ANY
+  };
+
+  uint create_table_def_key(char *key,
+                            const char *db,
+                            const char *table_name);
+
+  TMP_TABLE_SHARE *create_table(handlerton *hton,
+                                LEX_CUSTRING *frm,
+                                const char *path,
+                                const char *db,
+                                const char *table_name);
+
+  TABLE *find_table(const char *key, uint key_length, Table_state state);
+
+  bool find_and_use_table(const TABLE_LIST *tl, TABLE **out_table);
+
+  TABLE *open_table(TMP_TABLE_SHARE *share, const char *alias,
+                    bool open_in_engine);
+
+  bool use_table(TABLE *table, TABLE **out_table);
+  bool close_table(TABLE *table);
+  bool wait_for_prior_commit();
+  bool log_events_and_free_shares();
+
+  bool free_table_share(TMP_TABLE_SHARE *share, bool delete_table);
+  bool free_table(TABLE *table);
+
+  bool lock_tables();
+  void unlock_tables();
+
+  /* List operations */
+  template <class T>
+  void link(T **list, T *element)
+  {
+    element->next= *list;
+    if (element->next)
+      element->next->prev= element;
+    *list= element;
+    (*list)->prev= 0;
+  }
+
+  template <class T>
+  void unlink(T **list, T *element)
+  {
+    if (element->prev)
+    {
+      element->prev->next= element->next;
+      if (element->prev->next)
+        element->next->prev= element->prev;
+    }
+    else
+    {
+      DBUG_ASSERT(element == *list);
+
+      *list= element->next;
+      if (*list)
+        element->next->prev= 0;
+    }
+  }
No need to reinvent the wheel, you can use I_P_List for that.
...
+
+  uint tmpkeyval(TMP_TABLE_SHARE *share)
+  {
+    return uint4korr(share->table_cache_key.str +
+                     share->table_cache_key.length - 4);
+  }
+};
+
+#endif /* TEMPORARY_TABLES_H */
Regards,
Sergei
Chief Architect MariaDB
and security@mariadb.org

    

Re: [Maria-developers] [Commits] c285dbe: MDEV-5535: Cannot reopen temporary table

Sergei Golubchik