Bug#17263 temporary tables and replication

The fix refines the algorithm of generating DROPs for binlog. 
Temp tables with common pseudo_thread_id are clustered into one query. 
Consequently one replication event per pseudo_thread_id is generated.

mysql-test/r/rpl_temporary.result

@@ -103,3 +103,17 @@ f
 1
 drop temporary table t4;
 drop table t5;
+set @session.pseudo_thread_id=100;
+create temporary table t101 (id int);
+create temporary table t102 (id int);
+set @session.pseudo_thread_id=200;
+create temporary table t201 (id int);
+create temporary table `#not_user_table_prefixed_with_hash_sign_no_harm` (id int);
+set @con1_id=connection_id();
+kill @con1_id;
+create table t1(f int);
+insert into t1 values (1);
+select * from t1 /* must be 1 */;
+f
+1
+drop table t1;

mysql-test/t/rpl_temporary.test

@@ -129,6 +129,8 @@ drop table t1,t2;
 create temporary table t3 (f int);
 sync_with_master;
 
+# The server will now close done
+
 #
 # Bug#17284 erroneous temp table cleanup on slave
 #
@@ -154,6 +156,32 @@ connection master;
 drop temporary table t4;
 drop table t5;
 
-# The server will now close done
+#
+# BUG#17263 incorrect generation DROP temp tables
+# Temporary tables of connection are dropped in batches
+# where a batch correspond to pseudo_thread_id
+# value was set up at the moment of temp table creation
+#
+connection con1;
+set @session.pseudo_thread_id=100;
+create temporary table t101 (id int);
+create temporary table t102 (id int);
+set @session.pseudo_thread_id=200;
+create temporary table t201 (id int);
+create temporary table `#not_user_table_prefixed_with_hash_sign_no_harm` (id int);
+set @con1_id=connection_id();
+kill @con1_id;
+
+#now do something to show that slave is ok after DROP temp tables
+connection master;
+create table t1(f int);
+insert into t1 values (1);
+
+sync_slave_with_master;
+#connection slave;
+select * from t1 /* must be 1 */;
+
+connection master; 
+drop table t1;
 
 # End of 5.0 tests

sql/sql_base.cc

@@ -607,13 +607,22 @@ void close_temporary(TABLE *table,bool delete_table)
   DBUG_VOID_RETURN;
 }
 
+/* close_temporary_tables' internal, 4 is due to uint4korr definition */
+static inline uint  tmpkeyval(THD *thd, TABLE *table)
+{
+  return uint4korr(table->s->table_cache_key + table->s->key_length - 4);
+}
+
+/* Creates one DROP TEMPORARY TABLE binlog event for each pseudo-thread */
 
 void close_temporary_tables(THD *thd)
 {
-  TABLE *table,*next;
-  char *query, *end;
-  uint query_buf_size; 
-  bool found_user_tables = 0;
+  TABLE *next,
+    *prev_table /* prev link is not maintained in TABLE's double-linked list */,
+    *table;
+  char *query= (gptr) 0, *end;
+  uint query_buf_size, max_names_len; 
+  bool found_user_tables;
 
   if (!thd->temporary_tables)
     return;
@@ -621,47 +630,122 @@ void close_temporary_tables(THD *thd)
   LINT_INIT(end);
   query_buf_size= 50;   // Enough for DROP ... TABLE IF EXISTS
 
-  for (table=thd->temporary_tables ; table ; table=table->next)
+  /* 
+     insertion sort of temp tables by pseudo_thread_id to build ordered list 
+     of sublists of equal pseudo_thread_id
+  */
+  for (prev_table= thd->temporary_tables, 
+         table= prev_table->next,
+         found_user_tables= (prev_table->s->table_name[0] != '#'); 
+       table;
+       prev_table= table, table= table->next)
+  {
+    TABLE *prev_sorted /* same as for prev_table */,
+      *sorted;
     /*
-      We are going to add 4 ` around the db/table names, so 1 does not look
-      enough; indeed it is enough, because table->key_length is greater (by 8,
-      because of server_id and thread_id) than db||table.
+      table not created directly by the user is moved to the tail. 
+      Fixme/todo: nothing (I checked the manual) prevents user to create temp
+      with `#' 
     */
-    query_buf_size+= table->s->key_length+1;
-
-  if ((query = alloc_root(thd->mem_root, query_buf_size)))
-    // Better add "if exists", in case a RESET MASTER has been done
-    end=strmov(query, "DROP /*!40005 TEMPORARY */ TABLE IF EXISTS ");
-
-  for (table=thd->temporary_tables ; table ; table=next)
+    if (table->s->table_name[0] == '#')
+      continue;
+    else 
+    {
+      found_user_tables = 1;
+    }
+    for (prev_sorted= NULL, sorted= thd->temporary_tables; sorted != table; 
+         prev_sorted= sorted, sorted= sorted->next)
+    {
+      if (sorted->s->table_name[0] == '#' || tmpkeyval(thd, sorted) > tmpkeyval(thd, table))
+      {
+        /* move into the sorted part of the list from the unsorted */
+        prev_table->next= table->next;
+        table->next= sorted;
+        if (prev_sorted) 
+        {
+          prev_sorted->next= table;
+        }
+        else
+        {
+          thd->temporary_tables= table;
+        }
+        table= prev_table;
+        break;
+      }
+    }
+  }  
+  /* 
+     calc query_buf_size as max per sublists, one sublist per pseudo thread id.
+     Also stop at first occurence of `#'-named table that starts 
+     all implicitly created temp tables
+  */
+  for (max_names_len= 0, table=thd->temporary_tables; 
+       table && table->s->table_name[0] != '#';
+       table=table->next)
   {
-    if (query) // we might be out of memory, but this is not fatal
+    uint tmp_names_len;
+    for (tmp_names_len= table->s->key_length + 1;
+         table->next && table->s->table_name[0] != '#' &&
+           tmpkeyval(thd, table) == tmpkeyval(thd, table->next);
+         table=table->next)
     {
-      // skip temporary tables not created directly by the user
-      if (table->s->table_name[0] != '#')
-	found_user_tables = 1;
-      end = strxmov(end,"`",table->s->db,"`.`",
-                    table->s->table_name,"`,", NullS);
+      /*
+        We are going to add 4 ` around the db/table names, so 1 might not look
+        enough; indeed it is enough, because table->key_length is greater (by 8,
+        because of server_id and thread_id) than db||table.
+      */
+      tmp_names_len += table->next->s->key_length + 1;
     }
-    next=table->next;
-    close_temporary(table, 1);
+    if (tmp_names_len > max_names_len) max_names_len= tmp_names_len;
   }
-  if (query && found_user_tables && mysql_bin_log.is_open())
+  
+  /* allocate */
+  if (found_user_tables && mysql_bin_log.is_open() &&
+      (query = alloc_root(thd->mem_root, query_buf_size+= max_names_len)))
+    // Better add "if exists", in case a RESET MASTER has been done
+    end= strmov(query, "DROP /*!40005 TEMPORARY */ TABLE IF EXISTS ");
+
+  /* scan sorted tmps to generate sequence of DROP */
+  for (table=thd->temporary_tables; table; table= next)
   {
-    /* The -1 is to remove last ',' */
-    thd->clear_error();
-    Query_log_event qinfo(thd, query, (ulong)(end-query)-1, 0, FALSE);
-    /*
-      Imagine the thread had created a temp table, then was doing a SELECT, and
-      the SELECT was killed. Then it's not clever to mark the statement above as
-      "killed", because it's not really a statement updating data, and there
-      are 99.99% chances it will succeed on slave.
-      If a real update (one updating a persistent table) was killed on the
-      master, then this real update will be logged with error_code=killed,
-      rightfully causing the slave to stop.
-    */
-    qinfo.error_code= 0;
-    mysql_bin_log.write(&qinfo);
+    if (query // we might be out of memory, but this is not fatal 
+        && table->s->table_name[0] != '#') 
+    {
+      char *end_cur;
+      /* Set pseudo_thread_id to be that of the processed table */
+      thd->variables.pseudo_thread_id= tmpkeyval(thd, table);
+      /* Loop forward through all tables within the sublist of
+         common pseudo_thread_id to create single DROP query */
+      for (end_cur= end;
+           table && table->s->table_name[0] != '#' &&
+             tmpkeyval(thd, table) == thd->variables.pseudo_thread_id;
+           table= next)
+      {
+        end_cur= strxmov(end_cur, "`", table->s->db, "`.`",
+                      table->s->table_name, "`,", NullS);
+        next= table->next;
+        close_temporary(table, 1);
+      }
+      thd->clear_error();
+      /* The -1 is to remove last ',' */
+      Query_log_event qinfo(thd, query, (ulong)(end_cur - query) - 1, 0, FALSE);
+      /*
+        Imagine the thread had created a temp table, then was doing a SELECT, and
+        the SELECT was killed. Then it's not clever to mark the statement above as
+        "killed", because it's not really a statement updating data, and there
+        are 99.99% chances it will succeed on slave.
+        If a real update (one updating a persistent table) was killed on the
+        master, then this real update will be logged with error_code=killed,
+        rightfully causing the slave to stop.
+      */
+      qinfo.error_code= 0;
+      mysql_bin_log.write(&qinfo);
+    }
+    else 
+    {
+      next= table->next;
+      close_temporary(table, 1);
+    }
   }
   thd->temporary_tables=0;
 }