bzr commit into mysql-5.5-bugteam branch (mattias.jonsson:3115) Bug#57778

[Mattias Jonsson]

#At file:///Users/mattiasj/mysql-bzr/b57778-55-bt/ based on revid:georgi.kodinov@stripped

 3115 Mattias Jonsson	2010-11-05
      Bug#57778: failed primary key add to partitioned innodb table inconsistent and crashes
      
      It was possible to issue an ALTER TABLE ADD PRIMARY KEY on
      an partitioned InnoDB table that failed and crashed the server.
      
      The problem was that it succeeded to create the PK on at least
      one partition, and then failed on a subsequent partition, due to
      duplicate key violation. Since the partitions that already had added
      the PK was not reverted all partitions was not consistent with the
      table definition, which caused the crash.
      
      The solution was to add a revert step to ha_partition::add_index()
      that dropped the index for the already succeeded partitions, on failure.
     @ mysql-test/r/partition.result
        updated result
     @ mysql-test/t/partition.test
        Added test
     @ sql/ha_partition.cc
        Only allow ADD/DROP flags in pairs, so that they can be reverted on failures.
        If add_index() fails for a partition, revert (drop the index) for the previous
        partitions.
     @ sql/handler.h
        Added some extra info in a comment.

    modified:
      mysql-test/r/partition.result
      mysql-test/t/partition.test
      sql/ha_partition.cc
      sql/handler.h
=== modified file 'mysql-test/r/partition.result'
--- a/mysql-test/r/partition.result	2010-10-01 14:06:10 +0000
+++ b/mysql-test/r/partition.result	2010-11-05 11:01:10 +0000
@@ -1,5 +1,38 @@
 drop table if exists t1, t2;
 #
+# Bug#57778: failed primary key add to partitioned innodb table
+#            inconsistent and crashes
+#
+CREATE TABLE t1 (a INT NOT NULL, b INT NOT NULL)
+PARTITION BY KEY (a) PARTITIONS 2;
+INSERT INTO t1 VALUES (0,1), (0,2);
+ALTER TABLE t1 ADD PRIMARY KEY (a);
+ERROR 23000: Duplicate entry '0' for key 'PRIMARY'
+SHOW CREATE TABLE t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` int(11) NOT NULL,
+  `b` int(11) NOT NULL
+) ENGINE=MyISAM DEFAULT CHARSET=latin1
+/*!50100 PARTITION BY KEY (a)
+PARTITIONS 2 */
+SELECT * FROM t1;
+a	b
+0	1
+0	2
+UPDATE t1 SET a = 1, b = 1 WHERE a = 0 AND b = 2;
+ALTER TABLE t1 ADD PRIMARY KEY (a);
+SELECT * FROM t1;
+a	b
+1	1
+0	1
+ALTER TABLE t1 DROP PRIMARY KEY;
+SELECT * FROM t1;
+a	b
+1	1
+0	1
+DROP TABLE t1;
+#
 # Bug#57113: ha_partition::extra(ha_extra_function):
 #            Assertion `m_extra_cache' failed
 CREATE TABLE t1

=== modified file 'mysql-test/t/partition.test'
--- a/mysql-test/t/partition.test	2010-10-01 14:06:10 +0000
+++ b/mysql-test/t/partition.test	2010-11-05 11:01:10 +0000
@@ -15,6 +15,24 @@ drop table if exists t1, t2;
 --enable_warnings
 
 --echo #
+--echo # Bug#57778: failed primary key add to partitioned innodb table
+--echo #            inconsistent and crashes
+--echo #
+CREATE TABLE t1 (a INT NOT NULL, b INT NOT NULL)
+PARTITION BY KEY (a) PARTITIONS 2;
+INSERT INTO t1 VALUES (0,1), (0,2);
+--error ER_DUP_ENTRY
+ALTER TABLE t1 ADD PRIMARY KEY (a);
+SHOW CREATE TABLE t1;
+SELECT * FROM t1;
+UPDATE t1 SET a = 1, b = 1 WHERE a = 0 AND b = 2;
+ALTER TABLE t1 ADD PRIMARY KEY (a);
+SELECT * FROM t1;
+ALTER TABLE t1 DROP PRIMARY KEY;
+SELECT * FROM t1;
+DROP TABLE t1;
+
+--echo #
 --echo # Bug#57113: ha_partition::extra(ha_extra_function):
 --echo #            Assertion `m_extra_cache' failed
 CREATE TABLE t1

=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc	2010-10-06 14:34:28 +0000
+++ b/sql/ha_partition.cc	2010-11-05 11:01:10 +0000
@@ -6375,9 +6375,42 @@ bool ha_partition::get_error_message(int
 */
 uint ha_partition::alter_table_flags(uint flags)
 {
+  uint flags_to_return, flags_to_check;
   DBUG_ENTER("ha_partition::alter_table_flags");
-  DBUG_RETURN(ht->alter_table_flags(flags) |
-              m_file[0]->alter_table_flags(flags)); 
+
+  flags_to_return= ht->alter_table_flags(flags);
+  flags_to_return|= m_file[0]->alter_table_flags(flags); 
+
+  /*
+    If one partition fails we must be able to revert the change for the other,
+    already altered, partitions. So both ADD and DROP can only be supported in
+    pairs.
+  */
+  flags_to_check= HA_ONLINE_ADD_INDEX_NO_WRITES;
+  flags_to_check|= HA_ONLINE_DROP_INDEX_NO_WRITES;
+  if ((flags_to_return & flags_to_check) != flags_to_check)
+    flags_to_return&= ~flags_to_check;
+  flags_to_check= HA_ONLINE_ADD_UNIQUE_INDEX_NO_WRITES;
+  flags_to_check|= HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES;
+  if ((flags_to_return & flags_to_check) != flags_to_check)
+    flags_to_return&= ~flags_to_check;
+  flags_to_check= HA_ONLINE_ADD_PK_INDEX_NO_WRITES;
+  flags_to_check|= HA_ONLINE_DROP_PK_INDEX_NO_WRITES;
+  if ((flags_to_return & flags_to_check) != flags_to_check)
+    flags_to_return&= ~flags_to_check;
+  flags_to_check= HA_ONLINE_ADD_INDEX;
+  flags_to_check|= HA_ONLINE_DROP_INDEX;
+  if ((flags_to_return & flags_to_check) != flags_to_check)
+    flags_to_return&= ~flags_to_check;
+  flags_to_check= HA_ONLINE_ADD_UNIQUE_INDEX;
+  flags_to_check|= HA_ONLINE_DROP_UNIQUE_INDEX;
+  if ((flags_to_return & flags_to_check) != flags_to_check)
+    flags_to_return&= ~flags_to_check;
+  flags_to_check= HA_ONLINE_ADD_PK_INDEX;
+  flags_to_check|= HA_ONLINE_DROP_PK_INDEX;
+  if ((flags_to_return & flags_to_check) != flags_to_check)
+    flags_to_return&= ~flags_to_check;
+  DBUG_RETURN(flags_to_return);
 }
 
 
@@ -6412,6 +6445,7 @@ int ha_partition::add_index(TABLE *table
   handler **file;
   int ret= 0;
 
+  DBUG_ENTER("ha_partition::add_index");
   /*
     There has already been a check in fix_partition_func in mysql_alter_table
     before this call, which checks for unique/primary key violations of the
@@ -6419,8 +6453,28 @@ int ha_partition::add_index(TABLE *table
   */
   for (file= m_file; *file; file++)
     if ((ret=  (*file)->add_index(table_arg, key_info, num_of_keys)))
-      break;
-  return ret;
+      goto err;
+  DBUG_RETURN(ret);
+err:
+  if (file > m_file)
+  {
+    uint *key_numbers= (uint*) ha_thd()->alloc(sizeof(uint) * num_of_keys);
+    uint old_num_of_keys= table_arg->s->keys;
+    uint i;
+    /* The newly created keys have the last id's */
+    for (i= 0; i < num_of_keys; i++)
+      key_numbers[i]= i + old_num_of_keys;
+    if (!table_arg->key_info)
+      table_arg->key_info= key_info;
+    while (--file >= m_file)
+    {
+      (void) (*file)->prepare_drop_index(table_arg, key_numbers, num_of_keys);
+      (void) (*file)->final_drop_index(table_arg);
+    }
+    if (table_arg->key_info == key_info)
+      table_arg->key_info= NULL;
+  }
+  DBUG_RETURN(ret);
 }
 
 

=== modified file 'sql/handler.h'
--- a/sql/handler.h	2010-10-06 14:34:28 +0000
+++ b/sql/handler.h	2010-11-05 11:01:10 +0000
@@ -174,6 +174,8 @@
 /*
   These bits are set if different kinds of indexes can be created
   off-line without re-create of the table (but with a table lock).
+  Partitioning needs both ADD and DROP to be supported by its underlying
+  handlers, due to error handling, see bug#57778.
 */
 #define HA_ONLINE_ADD_INDEX_NO_WRITES           (1L << 0) /*add index w/lock*/
 #define HA_ONLINE_DROP_INDEX_NO_WRITES          (1L << 1) /*drop index w/lock*/


Attachment: [text/bzr-bundle] bzr/[e-mail]