Bug #1081016: mysqldump --innodb-optimize-keys may produce invalid SQL

              with explicitly named FK constraints

The problem was that mysqldump --innodb-optimize-keys produced invalid
SQL for cases when there was an explicitly named FK constrained which
implied a implicit secondary index with the same name. An attempt to
skip such a key definition from table schema and add it later with an
ALTER TABLE statement resulted in an "Incorrect index name" error,
because an index with the same name has already been created implicitly.

Fixed by detecting such cases in skip_secondary_keys() and restoring
the secondary to table schema in case there's an explicitly named FK
constraint with the same name.

Percona-Server/client/mysqldump.c

@@ -2466,6 +2466,48 @@ static uint dump_routines_for_db(char *db)
   DBUG_RETURN(0);
 }
 
+/*
+  Find the first occurrence of a quoted identifier in a given string. Returns
+  the pointer to the opening quote, and stores the pointer to the closing quote
+  to the memory location pointed to by the 'end' argument,
+
+  If no quoted identifiers are found, returns NULL (and the value pointed to by
+  'end' is undefined in this case).
+*/
+
+static const char *parse_quoted_identifier(const char *str,
+                                            const char **end)
+{
+  const char *from;
+  const char *to;
+
+  if (!(from= strchr(str, '`')))
+    return NULL;
+
+  to= from;
+
+  while ((to= strchr(to + 1, '`'))) {
+    /*
+      Double backticks represent a backtick in identifier, rather than a quote
+      character.
+    */
+    if (to[1] == '`')
+    {
+      to++;
+      continue;
+    }
+
+    break;
+  }
+
+  if (to <= from + 1)
+    return NULL;                                /* Empty identifier */
+
+  *end= to;
+
+  return from;
+}
+
 /*
   Parse the specified key definition string and check if the key contains an
   AUTO_INCREMENT column as the first key part. We only check for the first key
@@ -2477,32 +2519,22 @@ static my_bool contains_autoinc_column(const char *autoinc_column,
                                        const char *keydef,
                                        key_type_t type)
 {
-  char *from, *to;
+  const char *from, *to;
   uint idnum;
 
   DBUG_ASSERT(type != KEY_TYPE_NONE);
 
-  if (autoinc_column == NULL || !(from= strchr(keydef, '`')))
+  if (autoinc_column == NULL)
     return FALSE;
 
-  to= from;
   idnum= 0;
 
-  while ((to= strchr(to + 1, '`')))
+  /*
+    There is only 1 iteration of the following loop for type == KEY_TYPE_PRIMARY
+    and 2 iterations for type == KEY_TYPE_UNIQUE / KEY_TYPE_NON_UNIQUE.
+  */
+  while ((from= parse_quoted_identifier(keydef, &to)))
   {
-    /*
-      Double backticks represent a backtick in identifier, rather than a quote
-      character.
-    */
-    if (to[1] == '`')
-    {
-      to++;
-      continue;
-    }
-
-    if (to <= from + 1)
-      break;                                    /* Broken key definition */
-
     idnum++;
 
     /*
@@ -2517,16 +2549,51 @@ static my_bool contains_autoinc_column(const char *autoinc_column,
       Check only the first (for PRIMARY KEY) or the second (for secondary keys)
       quoted identifier.
     */
-    if ((idnum == 1 + test(type != KEY_TYPE_PRIMARY)) ||
-        !(from= strchr(to + 1, '`')))
+    if ((idnum == 1 + test(type != KEY_TYPE_PRIMARY)))
       break;
 
-    to= from;
+    keydef= to + 1;
   }
 
   return FALSE;
 }
 
+/*
+  Find a node in the skipped keys list whose name matches a quoted
+  identifier specified as 'id_from' and 'id_to' arguments.
+*/
+
+static LIST *find_matching_skipped_key(const char *id_from,
+                                       const char *id_to)
+{
+  LIST *list;
+  size_t id_len;
+
+  id_len= id_to - id_from + 1;
+  DBUG_ASSERT(id_len > 2);
+
+  for (list= skipped_keys_list; list; list= list_rest(list))
+  {
+    const char *keydef;
+    const char *keyname_from;
+    const char *keyname_to;
+    size_t keyname_len;
+
+    keydef= list->data;
+
+    if ((keyname_from= parse_quoted_identifier(keydef, &keyname_to)))
+    {
+      keyname_len= keyname_to - keyname_from + 1;
+
+      if (id_len == keyname_len &&
+          !strncmp(keyname_from, id_from, id_len))
+        return list;
+    }
+  }
+
+  return NULL;
+}
+
 /*
   Remove secondary/foreign key definitions from a given SHOW CREATE TABLE string
   and store them into a temporary list to be used later.
@@ -2554,6 +2621,9 @@ static void skip_secondary_keys(char *create_str, my_bool has_pk)
   char *autoinc_column= NULL;
   my_bool has_autoinc= FALSE;
   key_type_t type;
+  const char *constr_from;
+  const char *constr_to;
+  LIST *keydef_node;
 
   strend= create_str + strlen(create_str);
 
@@ -2573,7 +2643,37 @@ static void skip_secondary_keys(char *create_str, my_bool has_pk)
     c= *tmp;
     *tmp= '\0'; /* so strstr() only processes the current line */
 
-    if (!strncmp(ptr, "UNIQUE KEY ", sizeof("UNIQUE KEY ") - 1))
+    if (!strncmp(ptr, "CONSTRAINT ", sizeof("CONSTRAINT ") - 1) &&
+        (constr_from= parse_quoted_identifier(ptr, &constr_to)) &&
+        (keydef_node= find_matching_skipped_key(constr_from, constr_to)))
+    {
+      char *keydef;
+      size_t keydef_len;
+
+      /*
+        There's a skipped key with the same name as the constraint name.  Let's
+        put it back before the current constraint definition and remove from the
+        skipped keys list.
+      */
+      keydef= keydef_node->data;
+      keydef_len= strlen(keydef) + 5;           /* ", \n  " */
+
+      memmove(orig_ptr + keydef_len, orig_ptr, strend - orig_ptr + 1);
+      memcpy(ptr, keydef, keydef_len - 5);
+      memcpy(ptr + keydef_len - 5, ", \n  ", 5);
+
+      skipped_keys_list= list_delete(skipped_keys_list, keydef_node);
+      my_free(keydef, MYF(0));
+      my_free(keydef_node, MYF(0));
+
+      strend+= keydef_len;
+      orig_ptr+= keydef_len;
+      ptr+= keydef_len;
+      tmp+= keydef_len;
+
+      type= KEY_TYPE_NONE;
+    }
+    else if (!strncmp(ptr, "UNIQUE KEY ", sizeof("UNIQUE KEY ") - 1))
       type= KEY_TYPE_UNIQUE;
     else if (!strncmp(ptr, "KEY ", sizeof("KEY ") - 1))
       type= KEY_TYPE_NON_UNIQUE;

Percona-Server/mysql-test/r/percona_mysqldump_innodb_optimize_keys.result

@@ -439,3 +439,50 @@ UNLOCK TABLES;
 
 ######################################
 DROP TABLE t1, t2;
+CREATE TABLE `t1` (
+`id` int(11) NOT NULL AUTO_INCREMENT,
+`a` int(11) NOT NULL,
+PRIMARY KEY (`id`),
+KEY `a` (`a`),
+CONSTRAINT `a` FOREIGN KEY (`a`) REFERENCES `t1` (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1;
+######################################
+
+/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */;
+/*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */;
+/*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */;
+/*!40101 SET NAMES utf8 */;
+/*!40103 SET @OLD_TIME_ZONE=@@TIME_ZONE */;
+/*!40103 SET TIME_ZONE='+00:00' */;
+/*!40014 SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0 */;
+/*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */;
+/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */;
+/*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */;
+DROP TABLE IF EXISTS `t1`;
+/*!40101 SET @saved_cs_client     = @@character_set_client */;
+/*!40101 SET character_set_client = utf8 */;
+CREATE TABLE `t1` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `a` int(11) NOT NULL,
+  PRIMARY KEY (`id`),
+  KEY `a` (`a`), 
+  CONSTRAINT `a` FOREIGN KEY (`a`) REFERENCES `t1` (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1;
+/*!40101 SET character_set_client = @saved_cs_client */;
+
+LOCK TABLES `t1` WRITE;
+/*!40000 ALTER TABLE `t1` DISABLE KEYS */;
+/*!40000 ALTER TABLE `t1` ENABLE KEYS */;
+UNLOCK TABLES;
+/*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */;
+
+/*!40101 SET SQL_MODE=@OLD_SQL_MODE */;
+/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;
+/*!40014 SET UNIQUE_CHECKS=@OLD_UNIQUE_CHECKS */;
+/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */;
+/*!40101 SET CHARACTER_SET_RESULTS=@OLD_CHARACTER_SET_RESULTS */;
+/*!40101 SET COLLATION_CONNECTION=@OLD_COLLATION_CONNECTION */;
+/*!40111 SET SQL_NOTES=@OLD_SQL_NOTES */;
+
+######################################
+DROP TABLE t1;

Percona-Server/mysql-test/t/percona_mysqldump_innodb_optimize_keys.test

@@ -223,5 +223,32 @@ CREATE TABLE t2 (
 
 DROP TABLE t1, t2;
 
+#############################################################################
+# Bug #1081016: mysqldump --innodb-optimize-keys may produce invalid SQL with
+#               explicitly named FK constraints
+#############################################################################
+
+CREATE TABLE `t1` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `a` int(11) NOT NULL,
+  PRIMARY KEY (`id`),
+  KEY `a` (`a`),
+  CONSTRAINT `a` FOREIGN KEY (`a`) REFERENCES `t1` (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1;
+
+--exec $MYSQL_DUMP --skip-comments --innodb-optimize-keys test t1 >$file
+
+--echo ######################################
+--cat_file $file
+--echo ######################################
+
+# Check that the resulting dump can be imported back
+
+--exec $MYSQL test < $file
+
+--remove_file $file
+
+DROP TABLE t1;
+
 # Wait till we reached the initial number of concurrent sessions
 --source include/wait_until_count_sessions.inc