Bug#19188: incorrect temporary table name of DROP query in replication

merge with 5.0.

specific test case in inside of #17263 section (grave quoted name).

mysql-test/r/rpl_temporary.result

@@ -88,15 +88,21 @@ f
 1
 drop temporary table t4;
 drop table t5;
-set @session.pseudo_thread_id=100;
+set @@session.pseudo_thread_id=100;
 create temporary table t101 (id int);
 create temporary table t102 (id int);
-set @session.pseudo_thread_id=200;
+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);
+create temporary table `t``201` (id int);
+create temporary table `#sql_not_user_table202` (id int);
+set @@session.pseudo_thread_id=300;
+create temporary table t301 (id int);
+create temporary table t302 (id int);
+create temporary table `#sql_not_user_table303` (id int);
 create table t1(f int);
 insert into t1 values (1);
 select * from t1 /* must be 1 */;
 f
 1
 drop table t1;
+End of 5.1 tests

mysql-test/t/rpl_temporary.test

@@ -165,24 +165,19 @@ drop table t5;
 # value was set up at the moment of temp table creation
 #
 connection con1;
-set @session.pseudo_thread_id=100;
+set @@session.pseudo_thread_id=100;
 create temporary table t101 (id int);
 create temporary table t102 (id int);
-set @session.pseudo_thread_id=200;
+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);
-
-#
-# Don't kill our own connection to the server as
-# the result code differs depending on platform.
-#
-# Select the id to kill into a variable of mysqltest
-let $con1_id= `select connection_id()`;
-# Switch connection to avoid killing our own connection
-connection master;
---disable_query_log
-eval kill $con1_id;
---enable_query_log
+create temporary table `t``201` (id int);
+# emulate internal temp table not to come to binlog
+create temporary table `#sql_not_user_table202` (id int);
+set @@session.pseudo_thread_id=300;
+create temporary table t301 (id int);
+create temporary table t302 (id int);
+create temporary table `#sql_not_user_table303` (id int);
+disconnect con1;
 
 #now do something to show that slave is ok after DROP temp tables
 connection master;
@@ -195,4 +190,5 @@ select * from t1 /* must be 1 */;
 
 connection master; 
 drop table t1;
-# End of 5.1 tests
+--echo End of 5.1 tests
+

sql/mysql_priv.h

@@ -1942,6 +1942,16 @@ inline int hexchar_to_int(char c)
   return -1;
 }
 
+/*
+  is_user_table()
+  return true if the table was created explicitly
+*/
+
+inline bool is_user_table(TABLE * table)
+{
+  const char *name= table->s->table_name.str;
+  return strncmp(name, tmp_file_prefix, tmp_file_prefix_length);
+}
 
 /*
   Some functions that are different in the embedded library and the normal

sql/sql_base.cc

@@ -1183,137 +1183,130 @@ static inline uint  tmpkeyval(THD *thd, TABLE *table)
 
 void close_temporary_tables(THD *thd)
 {
-  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;
-
+  TABLE *table;
   if (!thd->temporary_tables)
     return;
-  
-  LINT_INIT(end);
-  query_buf_size= 50;   // Enough for DROP ... TABLE IF EXISTS
 
-  /* 
-     insertion sort of temp tables by pseudo_thread_id to build ordered list 
+  if (!mysql_bin_log.is_open() || thd->current_stmt_binlog_row_based)
+  {
+    for (table= thd->temporary_tables; table; table= table->next)
+    {
+      close_temporary(table, 1, 1);
+    }
+    thd->temporary_tables= 0;
+    return;
+  }
+
+  TABLE *next,
+    *prev_table /* prev link is not maintained in TABLE's double-linked list */;
+  bool was_quote_show= true; /* to assume thd->options has OPTION_QUOTE_SHOW_CREATE */
+  // Better add "if exists", in case a RESET MASTER has been done
+  const char stub[]= "DROP /*!40005 TEMPORARY */ TABLE IF EXISTS ";
+  uint stub_len= sizeof(stub) - 1;
+  char buf[256];
+  memcpy(buf, stub, stub_len);
+  String s_query= String(buf, sizeof(buf), system_charset_info);
+  bool found_user_tables= false;
+  LINT_INIT(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.str[0] != '#'); 
+
+  for (prev_table= thd->temporary_tables, table= prev_table->next;
        table;
        prev_table= table, table= table->next)
   {
-    TABLE *prev_sorted /* same as for prev_table */,
-      *sorted;
-    /*
-      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 `#' 
-    */
-    if (table->s->table_name.str[0] == '#')
-      continue;
-    else 
-    {
-      found_user_tables = 1;
-    }
-    for (prev_sorted= NULL, sorted= thd->temporary_tables; sorted != table; 
-         prev_sorted= sorted, sorted= sorted->next)
+    TABLE *prev_sorted /* same as for prev_table */, *sorted;
+    if (is_user_table(table))
     {
-      if (sorted->s->table_name.str[0] == '#' || tmpkeyval(thd, sorted) > tmpkeyval(thd, table))
+      if (!found_user_tables)
+        found_user_tables= true;
+      for (prev_sorted= NULL, sorted= thd->temporary_tables; sorted != table;
+           prev_sorted= sorted, sorted= sorted->next)
       {
-        /* move into the sorted part of the list from the unsorted */
-        prev_table->next= table->next;
-        table->next= sorted;
-        if (prev_sorted) 
+        if (!is_user_table(sorted) ||
+            tmpkeyval(thd, sorted) > tmpkeyval(thd, table))
         {
-          prev_sorted->next= table;
-        }
-        else
-        {
-          thd->temporary_tables= 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;
         }
-        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.str[0] != '#';
-       table=table->next)
-  {
-    uint tmp_names_len;
-    for (tmp_names_len= table->s->table_cache_key.length + 1;
-         table->next && table->s->table_name.str[0] != '#' &&
-           tmpkeyval(thd, table) == tmpkeyval(thd, table->next);
-         table=table->next)
-    {
-      /*
-        We are going to add 4 ` around the db/table names, so 1 might not look
-        enough; indeed it is enough, because table->s->table_cache_key.length is
-        greater (by 8, because of server_id and thread_id) than db||table.
-      */
-      tmp_names_len += table->next->s->table_cache_key.length + 1;
-    }
-    if (tmp_names_len > max_names_len) max_names_len= tmp_names_len;
   }
-  
-  /* allocate */
-  if (found_user_tables && mysql_bin_log.is_open() &&
-      !thd->current_stmt_binlog_row_based &&
-      (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 ");
+
+  /* We always quote db,table names though it is slight overkill */
+  if (found_user_tables &&
+      !(was_quote_show= (thd->options & OPTION_QUOTE_SHOW_CREATE)))
+  {
+    thd->options |= OPTION_QUOTE_SHOW_CREATE;
+  }
 
   /* scan sorted tmps to generate sequence of DROP */
-  for (table=thd->temporary_tables; table; table= next)
+  for (table= thd->temporary_tables; table; table= next)
   {
-    if (query // we might be out of memory, but this is not fatal 
-        && table->s->table_name.str[0] != '#') 
+    if (is_user_table(table))
     {
-      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.str[0] != '#' &&
+      for (s_query.length(stub_len);
+           table && is_user_table(table) &&
              tmpkeyval(thd, table) == thd->variables.pseudo_thread_id;
            table= next)
       {
-        end_cur= strxmov(end_cur, "`", table->s->db.str, "`.`",
-                      table->s->table_name.str, "`,", NullS);
+        /*
+          We are going to add 4 ` around the db/table names and possible more
+          due to special characters in the names
+        */
+        append_identifier(thd, &s_query, table->s->db.str, strlen(table->s->db.str));
+        s_query.q_append('.');
+        append_identifier(thd, &s_query, table->s->table_name.str,
+                          strlen(table->s->table_name.str));
+        s_query.q_append(',');
         next= table->next;
         close_temporary(table, 1, 1);
       }
       thd->clear_error();
-      /* The -1 is to remove last ',' */
-      Query_log_event qinfo(thd, query, (ulong)(end_cur - query) - 1, 0, FALSE);
+      CHARSET_INFO *cs_save= thd->variables.character_set_client;
+      thd->variables.character_set_client= system_charset_info;
+      Query_log_event qinfo(thd, s_query.ptr(),
+                            s_query.length() - 1 /* to remove trailing ',' */,
+                            0, FALSE);
+      thd->variables.character_set_client= cs_save;
       /*
-        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.
+        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 
+    else
     {
       next= table->next;
       close_temporary(table, 1, 1);
     }
   }
+  if (!was_quote_show)
+    thd->options &= ~OPTION_QUOTE_SHOW_CREATE; /* restore option */
   thd->temporary_tables=0;
 }