Bug #16051728 SERVER CRASHES IN ADD_IDENTIFIER ON CONCURRENT ALTER TABLE AND

SHOW ENGINE INNOD

Problem:

The purpose of explain_filename() is to provide useful additional
information regarding the partitions given the filename.  This function
was returning an error when it was not able to parse the given filename.
For example, within InnoDB, temporary files are created with #sql-
prefix.  But this function was not able to parse it correctly.

Solution:

It is not an error, if explain_filename() could not parse the given
filename.  If there is no partition information to explain, then silently
return from the function.

rb#1940 approved by mattiasj

sql/sql_table.cc

@@ -198,7 +198,6 @@ uint explain_filename(THD* thd,
                       uint to_length,
                       enum_explain_filename_mode explain_mode)
 {
-  uint res= 0;
   char *to_p= to;
   char *end_p= to_p + to_length;
   const char *db_name= NULL;
@@ -209,7 +208,8 @@ uint explain_filename(THD* thd,
   int  part_name_len= 0;
   const char *subpart_name= NULL;
   int  subpart_name_len= 0;
-  enum enum_file_name_type {NORMAL, TEMP, RENAMED} name_type= NORMAL;
+  enum enum_part_name_type {NORMAL, TEMP, RENAMED} part_type= NORMAL;
+
   const char *tmp_p;
   DBUG_ENTER("explain_filename");
   DBUG_PRINT("enter", ("from '%s'", from));
@@ -228,17 +228,18 @@ uint explain_filename(THD* thd,
     table_name= tmp_p;
   }
   tmp_p= table_name;
-  while (!res && (tmp_p= strchr(tmp_p, '#')))
+  /* Look if there are partition tokens in the table name. */
+  while ((tmp_p= strchr(tmp_p, '#')))
   {
     tmp_p++;
     switch (tmp_p[0]) {
     case 'P':
     case 'p':
       if (tmp_p[1] == '#')
+      {
         part_name= tmp_p + 2;
-      else
-        res= 1;
-      tmp_p+= 2;
+        tmp_p+= 2;
+      }
       break;
     case 'S':
     case 's':
@@ -248,49 +249,32 @@ uint explain_filename(THD* thd,
         subpart_name= tmp_p + 3;
 	tmp_p+= 3;
       }
-      else if ((tmp_p[1] == 'Q' || tmp_p[1] == 'q') &&
-               (tmp_p[2] == 'L' || tmp_p[2] == 'l') &&
-                tmp_p[3] == '-')
-      {
-        name_type= TEMP;
-        tmp_p+= 4; /* sql- prefix found */
-      }
-      else
-        res= 2;
       break;
     case 'T':
     case 't':
       if ((tmp_p[1] == 'M' || tmp_p[1] == 'm') &&
           (tmp_p[2] == 'P' || tmp_p[2] == 'p') &&
           tmp_p[3] == '#' && !tmp_p[4])
-        name_type= TEMP;
-      else
-        res= 3;
-      tmp_p+= 4;
+      {
+        part_type= TEMP;
+        tmp_p+= 4;
+      }
       break;
     case 'R':
     case 'r':
       if ((tmp_p[1] == 'E' || tmp_p[1] == 'e') &&
           (tmp_p[2] == 'N' || tmp_p[2] == 'n') &&
           tmp_p[3] == '#' && !tmp_p[4])
-        name_type= RENAMED;
-      else
-        res= 4;
-      tmp_p+= 4;
+      {
+        part_type= RENAMED;
+        tmp_p+= 4;
+      }
       break;
     default:
-      res= 5;
+      /* Not partition name part. */
+      ;
     }
   }
-  if (res)
-  {
-    /* Better to give something back if we fail parsing, than nothing at all */
-    DBUG_PRINT("info", ("Error in explain_filename: %u", res));
-    sql_print_warning("Invalid (old?) table or database name '%s'", from);
-    DBUG_RETURN(my_snprintf(to, to_length,
-                            "<result %u when explaining filename '%s'>",
-                            res, from));
-  }
   if (part_name)
   {
     table_name_len= part_name - table_name - 3;
@@ -298,7 +282,7 @@ uint explain_filename(THD* thd,
       subpart_name_len= strlen(subpart_name);
     else
       part_name_len= strlen(part_name);
-    if (name_type != NORMAL)
+    if (part_type != NORMAL)
     {
       if (subpart_name)
         subpart_name_len-= 5;
@@ -340,9 +324,9 @@ uint explain_filename(THD* thd,
       to_p= strnmov(to_p, " ", end_p - to_p);
     else
       to_p= strnmov(to_p, ", ", end_p - to_p);
-    if (name_type != NORMAL)
+    if (part_type != NORMAL)
     {
-      if (name_type == TEMP)
+      if (part_type == TEMP)
         to_p= strnmov(to_p, ER_THD_OR_DEFAULT(thd, ER_TEMPORARY_NAME),
                       end_p - to_p);
       else

unittest/mysys/CMakeLists.txt

-# Copyright (c) 2006, 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2006, 2013, Oracle and/or its affiliates. All rights reserved.
 # 
 # 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
@@ -30,3 +30,12 @@ ENDMACRO()
 FOREACH(testname bitmap base64 my_vsnprintf my_atomic my_rdtsc lf my_malloc)
   MY_ADD_TEST(${testname})
 ENDFOREACH()
+
+IF(WIN32)
+  ADD_EXECUTABLE(explain_filename-t explain_filename-t.cc
+                                    ../../sql/nt_servc.cc)
+ELSE()
+  ADD_EXECUTABLE(explain_filename-t explain_filename-t.cc)
+ENDIF()
+TARGET_LINK_LIBRARIES(explain_filename-t sql mytap)
+ADD_TEST(explain_filename explain_filename-t)

unittest/mysys/explain_filename-t.cc

+/*
+   Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
+
+   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
+*/
+
+/** Unit test case for the function explain_filename(). */
+
+#include <tap.h>
+#include <mysqld_error.h>
+#include <sql_class.h>
+#include <sql_table.h>
+
+#define BUFLEN 1000
+char to[BUFLEN];
+char from[BUFLEN];
+
+const char *error_messages[1000];
+
+int setup()
+{
+  system_charset_info    = &my_charset_utf8_bin;
+  my_default_lc_messages = &my_locale_en_US;
+
+  /* Populate the necessary error messages */
+  error_messages[ER_DATABASE_NAME - ER_ERROR_FIRST]     = "Database";
+  error_messages[ER_TABLE_NAME - ER_ERROR_FIRST]        = "Table";
+  error_messages[ER_PARTITION_NAME - ER_ERROR_FIRST]    = "Partition";
+  error_messages[ER_SUBPARTITION_NAME - ER_ERROR_FIRST] = "Subpartition";
+  error_messages[ER_TEMPORARY_NAME - ER_ERROR_FIRST]    = "Temporary";
+  error_messages[ER_RENAMED_NAME - ER_ERROR_FIRST]      = "Renamed";
+
+  my_default_lc_messages->errmsgs->errmsgs = error_messages;
+
+  return 0;
+}
+
+void test_1(const char *in, const char *exp, enum_explain_filename_mode mode)
+{
+  char out[BUFLEN];
+
+  uint len1 = explain_filename(0, in, out, BUFLEN, mode);
+
+  /* expected output and actual output must be same */
+  bool pass = (strcmp(exp, out) == 0);
+
+  /* length returned by explain_filename is fine */
+  bool length = (len1 == strlen(exp));
+
+  ok( (pass && length) , "(%d): %s => %s\n", mode, in, out);
+}
+
+int main()
+{
+  setup();
+  plan(NO_PLAN);
+
+  test_1("test/t1.ibd",
+         "Database \"test\", Table \"t1.ibd\"",
+         EXPLAIN_ALL_VERBOSE);
+
+  test_1("test/t1.ibd",
+         "\"test\".\"t1.ibd\"",
+         EXPLAIN_PARTITIONS_VERBOSE);
+
+  test_1("test/t1.ibd",
+         "\"test\".\"t1.ibd\"",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/t1#TMP#",
+         "Database \"test\", Table \"t1#TMP#\"",
+         EXPLAIN_ALL_VERBOSE);
+
+  test_1("test/#sql-2882.ibd",
+         "Database \"test\", Table \"#sql-2882.ibd\"",
+         EXPLAIN_ALL_VERBOSE);
+
+  test_1("test/t1#REN#",
+         "Database \"test\", Table \"t1#REN#\"",
+         EXPLAIN_ALL_VERBOSE);
+
+  test_1("test/t1@0023REN@0023",
+         "Database \"test\", Table \"t1#REN#\"",
+         EXPLAIN_ALL_VERBOSE);
+
+  test_1("test/t1#p#p1",
+         "Database \"test\", Table \"t1\", Partition \"p1\"",
+         EXPLAIN_ALL_VERBOSE);
+
+  test_1("test/t1#P#p1",
+         "\"test\".\"t1\" /* Partition \"p1\" */",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/t1#P#p1@00231",
+         "\"test\".\"t1\" /* Partition \"p1#1\" */",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/t1#P#p1#SP#sp1",
+         "\"test\".\"t1\" /* Partition \"p1\", Subpartition \"sp1\" */",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/t1#p1#SP#sp1",
+         "\"test\".\"t1#p1#SP#sp1\"",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/t1#p#p1@00232#SP#sp1@00231#REN#",
+         "\"test\".\"t1\" /* Renamed Partition \"p1#2\", Subpartition \"sp1#1\" */",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/t1#p#p1#SP#sp1#TMP#",
+         "\"test\".\"t1\" /* Temporary Partition \"p1\", Subpartition \"sp1\" */",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/#sql-t1#P#p1#SP#sp1#TMP#",
+         "\"test\".\"#sql-t1#P#p1#SP#sp1#TMP#\" /* Temporary Partition \"p1\", Subpartition \"sp1\" */",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/#sql-t1#P#p1#SP#sp1",
+         "\"test\".\"#sql-t1#P#p1#SP#sp1\" /* Partition \"p1\", Subpartition \"sp1\" */",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/#sqlx-33",
+         "\"test\".\"#sqlx-33\"",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/#mysql50#t",
+         "\"test\".\"#mysql50#t\"",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("#mysql50#t",
+         "\"#mysql50#t\"",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("@0023t",
+         "\"#t\"",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  test_1("test/t@0023",
+         "\"test\".\"t#\"",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  /*
+    If a character not allowed in my_charset_filename is encountered,
+    then it will not be converted to system_charset_info!
+  */
+  test_1("test/t@0023#",
+         "\"test\".\"t@0023#\"",
+         EXPLAIN_PARTITIONS_AS_COMMENT);
+
+  return 0;
+}
+