List:Commits« Previous MessageNext Message »
From:marko.makela Date:June 5 2012 7:29am
Subject:bzr push into mysql-trunk-wl6255 branch (marko.makela:3937 to 3940) WL#6255
View as plain text  
 3940 Marko Mäkelä	2012-06-05
      WL#6255 DROP COLUMN: Test FOREIGN KEY checks.

    modified:
      mysql-test/suite/innodb/r/innodb-index.result
      mysql-test/suite/innodb/t/innodb-index.test
 3939 Marko Mäkelä	2012-06-05
      WL#5526 bug fix for failed DROP INDEX
      
      When an ALTER TABLE that involves DROP INDEX fails, clear the
      index->to_be_dropped flags. This bug was mostly affecting failed
      table rebuild (before WL#6255, ADD PRIMARY KEY) that also involved
      dropping indexes.
      
      prepare_inplace_alter_table_dict(),
      ha_innobase::prepare_inplace_alter_table(): On failure, clear the
      to_be_dropped flags.
      
      rollback_inplace_alter_table(): Always clear the to_be_dropped flags.
      Allow the function to be invoked in every case, not only to roll back
      secondary index creation.
      
      ha_innobase::commit_inplace_alter_table(): Always invoke
      rollback_inplace_alter_table() to roll back changes. Clear the
      index->to_be_dropped flags on the old table when the table was
      rebuilt. Assert at the end that no index->to_be_dropped flag is set.

    modified:
      storage/innobase/handler/handler0alter.cc
 3938 Marko Mäkelä	2012-06-05
      Add some debug assertions.

    modified:
      storage/innobase/handler/handler0alter.cc
 3937 Marko Mäkelä	2012-06-05
      WL#6255 ALTER_COLUMN_ORDER and DROP_COLUMN (DROP COLUMN and column reorder)
      
      ha_innobase_inplace_ctx(): Add col_map for mapping old column numbers
      to new ones when rebuilding a table.
      
      innobase_build_col_map(): Map old column numbers to new ones. Dropped
      columns are indicated by ULINT_UNDEFINED.
      
      prepare_inplace_alter_table_dict(): Invoke innobase_build_col_map()
      whenever rebuilding the table.
      
      row_log_table_apply(), row_merge_build_indexes(), row_build(),
      row_log_table_apply_convert_mrec(), row_log_table_apply_insert(),
      row_log_table_apply_update(), row_log_table_apply_op(),
      row_log_table_apply_ops(), row_merge_build_indexes(): Add the
      parameter col_map.
      
      row_merge_buf_add(): Remove the tweaks for FTS_DOC_ID. They will now
      be handled by the col_map.
      
      row_merge_read_clustered_index(): Add the parameter col_map.  Remove
      the array nullable[]. No longer tweak the DATA_NOT_NULL field of the
      row tuple, because it will have been set up in row_build().

    modified:
      mysql-test/r/alter_table.result
      mysql-test/suite/innodb/r/innodb-table-online.result
      mysql-test/suite/innodb/t/innodb-table-online.test
      storage/innobase/handler/handler0alter.cc
      storage/innobase/include/row0log.h
      storage/innobase/include/row0merge.h
      storage/innobase/include/row0row.h
      storage/innobase/row/row0log.cc
      storage/innobase/row/row0merge.cc
      storage/innobase/row/row0row.cc
      storage/innobase/row/row0undo.cc
      storage/innobase/row/row0upd.cc
      storage/innobase/row/row0vers.cc
=== modified file 'mysql-test/suite/innodb/r/innodb-index.result'
--- a/mysql-test/suite/innodb/r/innodb-index.result	revid:marko.makela@stripped4214744-29tpakp75k2gp2kp
+++ b/mysql-test/suite/innodb/r/innodb-index.result	revid:marko.makela@stripped5072828-xyp11q7yo2zm6xpy
@@ -454,8 +454,18 @@ alter table t2 drop index b, drop index
 ERROR HY000: Cannot drop index 'b': needed in a foreign key constraint
 alter table t2 MODIFY b INT NOT NULL, ALGORITHM=COPY;
 ERROR HY000: Error on rename of '#sql-temporary' to './test/t2' (errno: 150 - Foreign key constraint is incorrectly formed)
-alter table t2 MODIFY b INT NOT NULL, ALGORITHM=INPLACE;
+alter table t2 MODIFY b INT NOT NULL;
 ERROR HY000: Column 'b' cannot be NOT NULL: needed in a foreign key constraint 'test/t2_ibfk_1' SET NULL
+SET FOREIGN_KEY_CHECKS=0;
+alter table t2 DROP COLUMN b, ALGORITHM=COPY;
+ERROR HY000: Error on rename of '#sql-temporary' to './test/t2' (errno: 150 - Foreign key constraint is incorrectly formed)
+alter table t2 DROP COLUMN b;
+ERROR HY000: Cannot drop column 'b': needed in a foreign key constraint 'test/t2_ibfk_1'
+alter table t1 DROP COLUMN b, ALGORITHM=COPY;
+ERROR HY000: Error on rename of '#sql-temporary' to './test/t1' (errno: 150 - Foreign key constraint is incorrectly formed)
+alter table t1 DROP COLUMN b;
+ERROR HY000: Cannot drop column 'b': needed in a foreign key constraint 'test/t2_ibfk_1' of table '"test"."t2"'
+SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS;
 create unique index dc on t2 (d,c);
 affected rows: 0
 info: Records: 0  Duplicates: 0  Warnings: 0

=== modified file 'mysql-test/suite/innodb/t/innodb-index.test'
--- a/mysql-test/suite/innodb/t/innodb-index.test	revid:marko.makela@stripped4744-29tpakp75k2gp2kp
+++ b/mysql-test/suite/innodb/t/innodb-index.test	revid:marko.makela@stripped28-xyp11q7yo2zm6xpy
@@ -177,7 +177,27 @@ alter table t2 drop index b, drop index
 --error ER_ERROR_ON_RENAME
 alter table t2 MODIFY b INT NOT NULL, ALGORITHM=COPY;
 --error ER_FK_COLUMN_NOT_NULL
-alter table t2 MODIFY b INT NOT NULL, ALGORITHM=INPLACE;
+alter table t2 MODIFY b INT NOT NULL;
+
+SET FOREIGN_KEY_CHECKS=0;
+# mysqltest first does replace_regex, then replace_result
+--replace_regex /'[^']*test\/#sql-[0-9a-f_]*'/'#sql-temporary'/
+# Embedded server doesn't chdir to data directory
+--replace_result $MYSQLD_DATADIR ./ master-data/ ''
+--error ER_ERROR_ON_RENAME
+alter table t2 DROP COLUMN b, ALGORITHM=COPY;
+--error ER_FK_COLUMN_CANNOT_DROP
+alter table t2 DROP COLUMN b;
+# mysqltest first does replace_regex, then replace_result
+--replace_regex /'[^']*test\/#sql-[0-9a-f_]*'/'#sql-temporary'/
+# Embedded server doesn't chdir to data directory
+--replace_result $MYSQLD_DATADIR ./ master-data/ ''
+--error ER_ERROR_ON_RENAME
+alter table t1 DROP COLUMN b, ALGORITHM=COPY;
+--error ER_FK_COLUMN_CANNOT_DROP_CHILD
+alter table t1 DROP COLUMN b;
+SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS;
+
 --enable_info
 # Apparently, the following makes mysql_alter_table() drop index d.
 create unique index dc on t2 (d,c);

=== modified file 'storage/innobase/handler/handler0alter.cc'
--- a/storage/innobase/handler/handler0alter.cc	revid:marko.makela@oracle.com-20120604214744-29tpakp75k2gp2kp
+++ b/storage/innobase/handler/handler0alter.cc	revid:marko.makela@oracle.com-20120605072828-xyp11q7yo2zm6xpy
@@ -2476,12 +2476,7 @@ col_fail:
 
 			online_retry_drop_indexes_with_trx(user_table, trx);
 
-			row_mysql_unlock_data_dictionary(trx);
-			mem_heap_free(heap);
-
-			trx_free_for_mysql(trx);
-			trx_commit_for_mysql(user_trx);
-			DBUG_RETURN(true);
+			goto err_exit;
 		}
 
 		col_map = innobase_build_col_map(
@@ -2721,11 +2716,22 @@ error_handled:
 
 	ut_d(dict_table_check_for_dup_indexes(user_table, CHECK_ALL_COMPLETE));
 	ut_ad(!user_table->drop_aborted);
+
+err_exit:
+	/* Clear the to_be_dropped flag in the data dictionary cache. */
+	for (ulint i = 0; i < n_drop_index; i++) {
+		DBUG_ASSERT(*drop_index[i]->name != TEMP_INDEX_PREFIX);
+		DBUG_ASSERT(drop_index[i]->to_be_dropped);
+		drop_index[i]->to_be_dropped = 0;
+	}
+
 	row_mysql_unlock_data_dictionary(trx);
 
 	trx_free_for_mysql(trx);
 	mem_heap_free(heap);
 
+	trx_commit_for_mysql(user_trx);
+
 	/* There might be work for utility threads.*/
 	srv_active_wake_master_thread();
 
@@ -2819,7 +2825,7 @@ ha_innobase::prepare_inplace_alter_table
 			    prebuilt->table->space != 0)) {
 			my_error(ER_ILLEGAL_HA_CREATE_OPTION, MYF(0),
 				 table_type(), invalid_opt);
-			DBUG_RETURN(true);
+			goto err_exit_no_heap;
 		}
 	}
 
@@ -2829,6 +2835,7 @@ ha_innobase::prepare_inplace_alter_table
 		    ha_alter_info->key_info_buffer,
 		    ha_alter_info->key_count)) {
 err_exit_no_heap:
+		DBUG_ASSERT(prebuilt->trx->dict_operation_lock_mode == 0);
 		online_retry_drop_indexes(prebuilt->table, user_thd);
 		DBUG_RETURN(true);
 	}
@@ -2925,6 +2932,7 @@ check_if_ok_to_rename:
 		}
 	}
 
+	n_drop_index = 0;
 	n_drop_fk = 0;
 
 	if (ha_alter_info->handler_flags
@@ -2982,8 +2990,6 @@ found_fk:
 		heap = NULL;
 	}
 
-	n_drop_index = 0;
-
 	if (ha_alter_info->index_drop_count) {
 		DBUG_ASSERT(ha_alter_info->handler_flags
 			    & (Alter_inplace_info::DROP_INDEX
@@ -3186,6 +3192,7 @@ index_needed:
 		}
 
 func_exit:
+		DBUG_ASSERT(prebuilt->trx->dict_operation_lock_mode == 0);
 		online_retry_drop_indexes(prebuilt->table, user_thd);
 		DBUG_RETURN(false);
 	}
@@ -3209,6 +3216,20 @@ func_exit:
 	if (num_fts_index > 1) {
 		my_error(ER_INNODB_FT_LIMIT, MYF(0));
 err_exit:
+		if (n_drop_index) {
+			row_mysql_lock_data_dictionary(prebuilt->trx);
+
+			/* Clear the to_be_dropped flags, which might
+			have been set at this point. */
+			for (ulint i = 0; i < n_drop_index; i++) {
+				DBUG_ASSERT(*drop_index[i]->name
+					    != TEMP_INDEX_PREFIX);
+				drop_index[i]->to_be_dropped = 0;
+			}
+
+			row_mysql_unlock_data_dictionary(prebuilt->trx);
+		}
+
 		if (heap) {
 			mem_heap_free(heap);
 		}
@@ -3453,13 +3474,10 @@ rollback_inplace_alter_table(
 
 	if (!ctx || !ctx->trx) {
 		/* If we have not started a transaction yet,
-		nothing has been or needs to be done. */
+		(almost) nothing has been or needs to be done. */
 		goto func_exit;
 	}
 
-	/* Roll back index creation. */
-	DBUG_ASSERT(ha_alter_info->handler_flags & INNOBASE_INPLACE_CREATE);
-
 	row_mysql_lock_data_dictionary(ctx->trx);
 
 	if (prebuilt->table != ctx->indexed_table) {
@@ -3507,15 +3525,21 @@ func_exit:
 		    == ONLINE_INDEX_COMPLETE);
 #endif /* !DBUG_OFF */
 
-	if (ctx && prebuilt->table == ctx->indexed_table) {
-		/* Clear the to_be_dropped flag in the data dictionary. */
-		for (ulint i = 0; i < ctx->num_to_drop; i++) {
-			dict_index_t*	index = ctx->drop[i];
-			DBUG_ASSERT(*index->name != TEMP_INDEX_PREFIX);
-			DBUG_ASSERT(index->table == prebuilt->table);
-			DBUG_ASSERT(index->to_be_dropped);
+	if (ctx) {
+		if (ctx->num_to_drop) {
+			row_mysql_lock_data_dictionary(prebuilt->trx);
 
-			index->to_be_dropped = 0;
+			/* Clear the to_be_dropped flag
+			in the data dictionary cache. */
+			for (ulint i = 0; i < ctx->num_to_drop; i++) {
+				dict_index_t*	index = ctx->drop[i];
+				DBUG_ASSERT(*index->name != TEMP_INDEX_PREFIX);
+				DBUG_ASSERT(index->to_be_dropped);
+
+				index->to_be_dropped = 0;
+			}
+
+			row_mysql_unlock_data_dictionary(prebuilt->trx);
 		}
 	}
 
@@ -3851,17 +3875,6 @@ ha_innobase::commit_inplace_alter_table(
 
 	DBUG_ENTER("commit_inplace_alter_table");
 
-	if (!(ha_alter_info->handler_flags & ~INNOBASE_INPLACE_IGNORE)) {
-		DBUG_ASSERT(!ctx);
-
-		/* Nothing to do */
-		if (!commit) {
-			goto ret;
-		}
-		/* We may want to update table attributes. */
-		goto func_exit;
-	}
-
 	if (!commit) {
 		/* A rollback is being requested. So far we may at
 		most have created some indexes. If any indexes were to
@@ -3871,6 +3884,12 @@ ha_innobase::commit_inplace_alter_table(
 				    ha_alter_info, table_share, prebuilt));
 	}
 
+	if (!(ha_alter_info->handler_flags & ~INNOBASE_INPLACE_IGNORE)) {
+		DBUG_ASSERT(!ctx);
+		/* We may want to update table attributes. */
+		goto func_exit;
+	}
+
 	trx_start_if_not_started_xa(prebuilt->trx);
 
 	if (!ctx || !ctx->trx) {
@@ -3910,6 +3929,14 @@ ha_innobase::commit_inplace_alter_table(
 		dberr_t	error;
 		char*	tmp_name;
 
+		/* Clear the to_be_dropped flag in the data dictionary. */
+		for (ulint i = 0; i < ctx->num_to_drop; i++) {
+			dict_index_t*	index = ctx->drop[i];
+			DBUG_ASSERT(*index->name != TEMP_INDEX_PREFIX);
+			DBUG_ASSERT(index->to_be_dropped);
+			index->to_be_dropped = 0;
+		}
+
 		/* We copied the table. Any indexes that were
 		requested to be dropped were not created in the copy
 		of the table. Apply any last bit of the rebuild log
@@ -4138,7 +4165,8 @@ ha_innobase::commit_inplace_alter_table(
 			if (ctx->add_fk[i]->referenced_table) {
 				UT_LIST_ADD_LAST(
 					referenced_list,
-					ctx->add_fk[i]->referenced_table->referenced_list,
+					ctx->add_fk[i]->referenced_table
+					->referenced_list,
 					ctx->add_fk[i]);
 			}
 
@@ -4257,14 +4285,6 @@ trx_commit:
 
 		ut_d(dict_table_check_for_dup_indexes(
 			     prebuilt->table, CHECK_ALL_COMPLETE));
-#ifdef UNIV_DEBUG
-		for (dict_index_t* index = dict_table_get_first_index(
-			     prebuilt->table);
-		     index;
-		     index = dict_table_get_next_index(index)) {
-			ut_ad(!index->to_be_dropped);
-		}
-#endif /* UNIV_DEBUG */
 		DBUG_ASSERT(new_clustered == !prebuilt->trx);
 
 		if (add_fts) {
@@ -4368,7 +4388,6 @@ func_exit:
 		dict_table_autoinc_unlock(prebuilt->table);
 	}
 
-ret:
 #ifndef DBUG_OFF
 	dict_index_t* clust_index = dict_table_get_first_index(
 		prebuilt->table);
@@ -4377,6 +4396,15 @@ ret:
 		    == ONLINE_INDEX_COMPLETE);
 #endif /* !DBUG_OFF */
 
+#ifdef UNIV_DEBUG
+	for (dict_index_t* index = dict_table_get_first_index(
+		     prebuilt->table);
+	     index;
+	     index = dict_table_get_next_index(index)) {
+		ut_ad(!index->to_be_dropped);
+	}
+#endif /* UNIV_DEBUG */
+
 	if (err == 0) {
 		MONITOR_ATOMIC_DEC(MONITOR_PENDING_ALTER_TABLE);
 	}

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk-wl6255 branch (marko.makela:3937 to 3940) WL#6255marko.makela5 Jun