Bug#13593865 - 64037: CRASH IN HA_PARTITION::CREATE_HANDLERS ON

                      ALTER TABLE AFTER DROP PARTITION
Bug#13608188 - 64038: CRASH IN HANDLER::HA_THD ON ALTER TABLE AFTER
                      REPAIR NON-EXISTING PARTITION

Backport of bug#13357766 from -trunk to -5.5.

The state of some partitions was not reset on failure, leading
to invalid states of partitions in consequent statements.

Fixed by reverting back to original state for all partitions
if not all partition names was resolved.

Also adding extra security by forcing tables to be reopened
in case of error in mysql_alter_table.

(There is also removal of \r at the end of some lines.)

mysql-test/r/partition_error.result

 drop table if exists t1, t2;
 #
+# Bug#13608188 - 64038: CRASH IN HANDLER::HA_THD ON ALTER TABLE AFTER
+#                       REPAIR NON-EXISTING PARTITION
+#
+CREATE TABLE t1 ( a INT, b INT );
+INSERT INTO t1 VALUES (5,3),(5,6);
+ALTER TABLE t1 PARTITION BY KEY(b) PARTITIONS 3 ;
+ALTER TABLE t1 REPAIR PARTITION p1, p3;
+Table	Op	Msg_type	Msg_text
+test.t1	repair	error	Error in list of partitions to test.t1
+ALTER TABLE t1 ORDER BY b;
+DROP TABLE t1;
+#
+# Bug#13593865 - 64037: CRASH IN HA_PARTITION::CREATE_HANDLERS ON
+#                       ALTER TABLE AFTER DROP PARTITION
+#
+CREATE TABLE t1 (a INT)
+PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (0),
+PARTITION p1 VALUES LESS THAN MAXVALUE ) ;
+ALTER TABLE t1 DROP PARTITION p1;
+ALTER TABLE t1 ANALYZE PARTITION p0, p1;
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	error	Error in list of partitions to test.t1
+ALTER TABLE t1 COMMENT 'altered';
+DROP TABLE t1;
+#
 # Bug#57924: crash when creating partitioned table with
 #            multiple columns in the partition key
 #

mysql-test/t/partition_error.test

@@ -10,6 +10,34 @@ drop table if exists t1, t2;
 
 let $MYSQLD_DATADIR= `SELECT @@datadir`;
 
+--echo #
+--echo # Bug#13608188 - 64038: CRASH IN HANDLER::HA_THD ON ALTER TABLE AFTER
+--echo #                       REPAIR NON-EXISTING PARTITION
+--echo #
+CREATE TABLE t1 ( a INT, b INT );
+INSERT INTO t1 VALUES (5,3),(5,6);
+ALTER TABLE t1 PARTITION BY KEY(b) PARTITIONS 3 ;
+ALTER TABLE t1 REPAIR PARTITION p1, p3;
+ALTER TABLE t1 ORDER BY b;
+DROP TABLE t1;
+
+--echo #
+--echo # Bug#13593865 - 64037: CRASH IN HA_PARTITION::CREATE_HANDLERS ON
+--echo #                       ALTER TABLE AFTER DROP PARTITION
+--echo #
+
+CREATE TABLE t1 (a INT)
+PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (0),
+ PARTITION p1 VALUES LESS THAN MAXVALUE ) ;
+
+ALTER TABLE t1 DROP PARTITION p1;
+ALTER TABLE t1 ANALYZE PARTITION p0, p1;
+
+ALTER TABLE t1 COMMENT 'altered';
+
+DROP TABLE t1;
+
 --echo #
 --echo # Bug#57924: crash when creating partitioned table with
 --echo #            multiple columns in the partition key

sql/ha_partition.cc

@@ -3607,14 +3607,14 @@ int ha_partition::truncate_partition(Alter_info *alter_info, bool *binlog_stmt)
   uint num_parts= m_part_info->num_parts;
   uint num_subparts= m_part_info->num_subparts;
   uint i= 0;
-  uint num_parts_set= alter_info->partition_names.elements;
-  uint num_parts_found= set_part_state(alter_info, m_part_info,
-                                        PART_ADMIN);
   DBUG_ENTER("ha_partition::truncate_partition");
 
   /* Only binlog when it starts any call to the partitions handlers */
   *binlog_stmt= false;
 
+  if (set_part_state(alter_info, m_part_info, PART_ADMIN))
+    DBUG_RETURN(HA_ERR_NO_PARTITION_FOUND);
+
   /*
     TRUNCATE also means resetting auto_increment. Hence, reset
     it so that it will be initialized again at the next use.
@@ -3624,10 +3624,6 @@ int ha_partition::truncate_partition(Alter_info *alter_info, bool *binlog_stmt)
   table_share->ha_part_data->auto_inc_initialized= FALSE;
   unlock_auto_increment();
 
-  if (num_parts_set != num_parts_found &&
-      (!(alter_info->flags & ALTER_ALL_PARTITION)))
-    DBUG_RETURN(HA_ERR_NO_PARTITION_FOUND);
-
   *binlog_stmt= true;
 
   do
@@ -6059,7 +6055,7 @@ int ha_partition::extra(enum ha_extra_function operation)
     0                    Success
 
   DESCRIPTION
-    Called at end of each statement to reste buffers
+    Called at end of each statement to reset buffers
 */
 
 int ha_partition::reset(void)
@@ -6792,8 +6788,8 @@ int ha_partition::final_add_index(handler_add_index *add, bool commit)
   DBUG_RETURN(ret);
 err:
   uint j;
-  uint *key_numbers= NULL;
-  KEY *old_key_info= NULL;
+  uint *key_numbers= NULL;
+  KEY *old_key_info= NULL;
   uint num_of_keys= 0;
   int error;
   
@@ -6803,27 +6799,27 @@ int ha_partition::final_add_index(handler_add_index *add, bool commit)
   if (i > 0)
   {
     num_of_keys= part_add_index->num_of_keys;
-    key_numbers= (uint*) ha_thd()->alloc(sizeof(uint) * num_of_keys);
-    if (!key_numbers)
-    {
-      sql_print_error("Failed with error handling of adding index:\n"
-                      "committing index failed, and when trying to revert "
-                      "already committed partitions we failed allocating\n"
-                      "memory for the index for table '%s'",
-                      table_share->table_name.str);
-      DBUG_RETURN(HA_ERR_OUT_OF_MEM);
-    }
-    old_key_info= table->key_info;
-    /*
-      Use the newly added key_info as table->key_info to remove them.
-      Note that this requires the subhandlers to use name lookup of the
-      index. They must use given table->key_info[key_number], they cannot
-      use their local view of the keys, since table->key_info only include
-      the indexes to be removed here.
-    */
-    for (j= 0; j < num_of_keys; j++)
-      key_numbers[j]= j;
-    table->key_info= part_add_index->key_info;
+    key_numbers= (uint*) ha_thd()->alloc(sizeof(uint) * num_of_keys);
+    if (!key_numbers)
+    {
+      sql_print_error("Failed with error handling of adding index:\n"
+                      "committing index failed, and when trying to revert "
+                      "already committed partitions we failed allocating\n"
+                      "memory for the index for table '%s'",
+                      table_share->table_name.str);
+      DBUG_RETURN(HA_ERR_OUT_OF_MEM);
+    }
+    old_key_info= table->key_info;
+    /*
+      Use the newly added key_info as table->key_info to remove them.
+      Note that this requires the subhandlers to use name lookup of the
+      index. They must use given table->key_info[key_number], they cannot
+      use their local view of the keys, since table->key_info only include
+      the indexes to be removed here.
+    */
+    for (j= 0; j < num_of_keys; j++)
+      key_numbers[j]= j;
+    table->key_info= part_add_index->key_info;
   }
 
   for (j= 0; j < m_tot_parts; j++)
@@ -6831,15 +6827,15 @@ int ha_partition::final_add_index(handler_add_index *add, bool commit)
     if (j < i)
     {
       /* Remove the newly added index */
-      error= m_file[j]->prepare_drop_index(table, key_numbers, num_of_keys);
-      if (error || m_file[j]->final_drop_index(table))
-      {
-        sql_print_error("Failed with error handling of adding index:\n"
-                        "committing index failed, and when trying to revert "
-                        "already committed partitions we failed removing\n"
-                        "the index for table '%s' partition nr %d",
-                        table_share->table_name.str, j);
-      }
+      error= m_file[j]->prepare_drop_index(table, key_numbers, num_of_keys);
+      if (error || m_file[j]->final_drop_index(table))
+      {
+        sql_print_error("Failed with error handling of adding index:\n"
+                        "committing index failed, and when trying to revert "
+                        "already committed partitions we failed removing\n"
+                        "the index for table '%s' partition nr %d",
+                        table_share->table_name.str, j);
+      }
     }
     else if (j > i)
     {

sql/sql_admin.cc

@@ -406,12 +406,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
             my_error(ER_PARTITION_MGMT_ON_NONPARTITIONED, MYF(0));
             DBUG_RETURN(TRUE);
           }
-          uint num_parts_found;
-          uint num_parts_opt= alter_info->partition_names.elements;
-          num_parts_found= set_part_state(alter_info, table->table->part_info,
-                                          PART_ADMIN);
-          if (num_parts_found != num_parts_opt &&
-              (!(alter_info->flags & ALTER_ALL_PARTITION)))
+          if (set_part_state(alter_info, table->table->part_info, PART_ADMIN))
           {
             char buff[FN_REFLEN + MYSQL_ERRMSG_SIZE];
             size_t length;
@@ -872,6 +867,9 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
   DBUG_RETURN(FALSE);
 
 err:
+  /* Make sure this table instance is not reused after the failure. */
+  if (table && table->table)
+    table->table->m_needs_reopen= true;
   trans_rollback_stmt(thd);
   trans_rollback(thd);
   close_thread_tables(thd);			// Shouldn't be needed

sql/sql_partition.cc

@@ -4469,11 +4469,20 @@ static bool check_native_partitioned(HA_CREATE_INFO *create_info,bool *ret_val,
 }
 
 
-/*
-  Sets which partitions to be used in the command
+/**
+  Sets which partitions to be used in the command.
+
+  @param alter_info     Alter_info pointer holding partition names and flags.
+  @param tab_part_info  partition_info holding all partitions.
+  @param part_state     Which state to set for the named partitions.
+
+  @return Operation status
+    @retval false  Success
+    @retval true   Failure
 */
-uint set_part_state(Alter_info *alter_info, partition_info *tab_part_info,
-               enum partition_state part_state)
+
+bool set_part_state(Alter_info *alter_info, partition_info *tab_part_info,
+                    enum partition_state part_state)
 {
   uint part_count= 0;
   uint num_parts_found= 0;
@@ -4499,7 +4508,21 @@ uint set_part_state(Alter_info *alter_info, partition_info *tab_part_info,
     else
       part_elem->part_state= PART_NORMAL;
   } while (++part_count < tab_part_info->num_parts);
-  return num_parts_found;
+
+  if (num_parts_found != alter_info->partition_names.elements &&
+      !(alter_info->flags & ALTER_ALL_PARTITION))
+  {
+    /* Not all given partitions found, revert and return failure */
+    part_it.rewind();
+    part_count= 0;
+    do
+    {
+      partition_element *part_elem= part_it++;
+      part_elem->part_state= PART_NORMAL;
+    } while (++part_count < tab_part_info->num_parts);
+    return true;
+  }
+  return false;
 }
 
 
@@ -5018,11 +5041,7 @@ that are reorganised.
     }
     else if (alter_info->flags & ALTER_REBUILD_PARTITION)
     {
-      uint num_parts_found;
-      uint num_parts_opt= alter_info->partition_names.elements;
-      num_parts_found= set_part_state(alter_info, tab_part_info, PART_CHANGED);
-      if (num_parts_found != num_parts_opt &&
-          (!(alter_info->flags & ALTER_ALL_PARTITION)))
+      if (set_part_state(alter_info, tab_part_info, PART_CHANGED))
       {
         my_error(ER_DROP_PARTITION_NON_EXISTENT, MYF(0), "REBUILD");
         goto err;

sql/sql_partition.h

@@ -254,7 +254,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
                                 char *db,
                                 const char *table_name,
                                 TABLE  *fast_alter_table);
-uint set_part_state(Alter_info *alter_info, partition_info *tab_part_info,
+bool set_part_state(Alter_info *alter_info, partition_info *tab_part_info,
                     enum partition_state part_state);
 uint prep_alter_part_table(THD *thd, TABLE *table, Alter_info *alter_info,
                            HA_CREATE_INFO *create_info,