Bug#12623923 Server can crash after failure to create

             primary key with innodb tables

The bug was triggered if a single ALTER TABLE statement both
added and dropped indexes and ALTER TABLE failed during drop
(e.g. because the index was needed in a foreign key constraint).
In such cases, the server index information would get out of
sync with InnoDB - the added index would be present inside
InnoDB, but not in the server. This could then lead to InnoDB
error messages and/or server crashes.

The root cause is that new indexes are added before old indexes
are dropped. This means that if ALTER TABLE fails while dropping
indexes, index changes will be reverted in the server but not
inside InnoDB.

This patch fixes the problem by dropping any added indexes
if drop fails (for ALTER TABLE statements that both adds
and drops indexes). 

However, this won't work if we added a primary key as this
key might not be possible to drop inside InnoDB. Therefore,
we resort to the copy algorithm if a primary key is added
by an ALTER TABLE statement that also drops an index.

In 5.6 this bug is more properly fixed by the handler interface
changes done in the scope of WL#5534 "Online ALTER".

sql/sql_table.cc

@@ -6313,11 +6313,23 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
       the primary key is not added and dropped in the same statement.
       Otherwise we have to recreate the table.
       need_copy_table is no-zero at this place.
+
+      Also, in-place is not possible if we add a primary key
+      and drop another key in the same statement. If the drop fails,
+      we will not be able to revert adding of primary key.
     */
     if ( pk_changed < 2 )
     {
-      if ((alter_flags & needed_inplace_with_read_flags) ==
-          needed_inplace_with_read_flags)
+      if ((needed_inplace_with_read_flags & HA_INPLACE_ADD_PK_INDEX_NO_WRITE) &&
+          index_drop_count > 0)
+      {
+        /*
+          Do copy, not in-place ALTER.
+          Avoid setting ALTER_TABLE_METADATA_ONLY.
+        */
+      }
+      else if ((alter_flags & needed_inplace_with_read_flags) ==
+               needed_inplace_with_read_flags)
       {
         /* All required in-place flags to allow concurrent reads are present. */
         need_copy_table= ALTER_TABLE_METADATA_ONLY;
@@ -6579,17 +6591,38 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
         Tell the handler to prepare for drop indexes.
         This re-numbers the indexes to get rid of gaps.
       */
-      if ((error= table->file->prepare_drop_index(table, key_numbers,
-                                                  index_drop_count)))
+      error= table->file->prepare_drop_index(table, key_numbers,
+                                             index_drop_count);
+      if (!error)
       {
-        table->file->print_error(error, MYF(0));
-        goto err_new_table_cleanup;
+        /* Tell the handler to finally drop the indexes. */
+        error= table->file->final_drop_index(table);
       }
 
-      /* Tell the handler to finally drop the indexes. */
-      if ((error= table->file->final_drop_index(table)))
+      if (error)
       {
         table->file->print_error(error, MYF(0));
+        if (index_add_count) // Drop any new indexes added.
+        {
+          /*
+            Temporarily set table-key_info to include information about the
+            indexes added above that we now need to drop.
+          */
+          KEY *save_key_info= table->key_info;
+          table->key_info= key_info_buffer;
+          if ((error= table->file->prepare_drop_index(table, index_add_buffer,
+                                                      index_add_count)))
+            table->file->print_error(error, MYF(0));
+          else if ((error= table->file->final_drop_index(table)))
+            table->file->print_error(error, MYF(0));
+          table->key_info= save_key_info;
+        }
+
+        /*
+          Mark this TABLE instance as stale to avoid
+          out-of-sync index information.
+        */
+        table->m_needs_reopen= true;
         goto err_new_table_cleanup;
       }
     }

storage/innobase/handler/ha_innodb.cc

@@ -8010,10 +8010,15 @@ innobase_get_mysql_key_number_for_index(
 			}
 		}
 
-		/* Print an error message if we cannot find the index
-		** in the "index translation table". */
-		sql_print_error("Cannot find index %s in InnoDB index "
-				"translation table.", index->name);
+		/* If index_count in translation table is set to 0, it
+		is possible we are in the process of rebuilding table,
+		do not spit error in this case */
+		if (share->idx_trans_tbl.index_count) {
+			/* Print an error message if we cannot find the index
+			** in the "index translation table". */
+			sql_print_error("Cannot find index %s in InnoDB index "
+					"translation table.", index->name);
+		}
 	}
 
 	/* If we do not have an "index translation table", or not able

storage/innobase/handler/handler0alter.cc

@@ -773,7 +773,7 @@ ha_innobase::add_index(
 	row_mysql_lock_data_dictionary(trx);
 	dict_locked = TRUE;
 
-	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, FALSE));
+	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, TRUE));
 
 	/* If a new primary key is defined for the table we need
 	to drop the original table and rebuild all indexes. */
@@ -809,7 +809,7 @@ ha_innobase::add_index(
 			}
 
 			ut_d(dict_table_check_for_dup_indexes(prebuilt->table,
-							      FALSE));
+							      TRUE));
 			mem_heap_free(heap);
 			trx_general_rollback_for_mysql(trx, NULL);
 			row_mysql_unlock_data_dictionary(trx);
@@ -1061,7 +1061,7 @@ ha_innobase::final_add_index(
 		trx_commit_for_mysql(prebuilt->trx);
 	}
 
-	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, FALSE));
+	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, TRUE));
 	row_mysql_unlock_data_dictionary(trx);
 
 	trx_free_for_mysql(trx);
@@ -1104,7 +1104,7 @@ ha_innobase::prepare_drop_index(
 	/* Test and mark all the indexes to be dropped */
 
 	row_mysql_lock_data_dictionary(trx);
-	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, FALSE));
+	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, TRUE));
 
 	/* Check that none of the indexes have previously been flagged
 	for deletion. */
@@ -1275,7 +1275,7 @@ func_exit:
 		} while (index);
 	}
 
-	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, FALSE));
+	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, TRUE));
 	row_mysql_unlock_data_dictionary(trx);
 
 	DBUG_RETURN(err);
@@ -1322,7 +1322,7 @@ ha_innobase::final_drop_index(
 		prebuilt->table->flags, user_thd);
 
 	row_mysql_lock_data_dictionary(trx);
-	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, FALSE));
+	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, TRUE));
 
 	if (UNIV_UNLIKELY(err)) {
 
@@ -1366,7 +1366,7 @@ ha_innobase::final_drop_index(
 	share->idx_trans_tbl.index_count = 0;
 
 func_exit:
-	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, FALSE));
+	ut_d(dict_table_check_for_dup_indexes(prebuilt->table, TRUE));
 	trx_commit_for_mysql(trx);
 	trx_commit_for_mysql(prebuilt->trx);
 	row_mysql_unlock_data_dictionary(trx);