List:Commits« Previous MessageNext Message »
From:marko.makela Date:May 10 2012 9:17am
Subject:bzr push into mysql-trunk branch (marko.makela:3812 to 3813) Bug#14006907
View as plain text  
 3813 Marko Mäkelä	2012-05-10
      Bug#14006907 FOREIGN KEY problems after DROP INDEX
      
      dict_index_t: Add the to_be_dropped flag to incidate when an index
      is about to be dropped.
      
      dict_foreign_find_index(): Assert that the caller is holding the
      dict_sys->mutex. Skip indexes that are flagged as to-be-dropped.
      
      dict_foreign_replace_index(): Assert that the index is to-be-dropped.
      
      ha_innobase::prepare_inplace_alter_table(): Lock the data dictionary
      while checking if any foreign key constraints depend on the indexes
      that are to be dropped. Flag them as such in the data dictionary cache.
      
      ha_innobase::inplace_alter_table(): Add a debug sync point for the
      DEBUG_SYNC test.
      
      rollback_inplace_alter_table(): Clear the index->to_be_dropped flag in
      the data dictionary cache.
      
      rb:1049 approved by Jimmy Yang

    added:
      mysql-test/suite/innodb/r/innodb_bug14006907.result
      mysql-test/suite/innodb/t/innodb_bug14006907.test
    modified:
      storage/innobase/dict/dict0dict.cc
      storage/innobase/handler/handler0alter.cc
      storage/innobase/include/dict0dict.h
      storage/innobase/include/dict0mem.h
 3812 Nisha Gopalakrishnan	2012-05-10
      Bug#11757486:49539: NON-DESCRIPTIVE ERR (ERROR 0 FROM STORAGE ENGINE) WITH
                          MULTI-TABLE UPDATE
            
      Analysis:
      ---------
      Multiple-table UPDATE statement with IGNORE keyword in strict mode 
      having invalid or missing values could trigger an assertion in debug
      mode. However on a release build, the query execution fails reporting 
      inappropriate errors.
      
      The multiple-table UPDATE does not test for IGNORE to decide
      whether the query should be aborted in case of any warning.This causes
      the warning to be converted to error(STRICT MODE behavior). However
      since the errors are to be suppressed due to the IGNORE keyword, the
      diagnostic area is not set to DA_ERROR.
      
      Since the diagnostic area remains DA_EMPTY, an ASSERT is triggered
      which causes the mysqld to crash on a debug build. On a release build
      the query execution fails with incorrect errors being reported. 
      
      Fix:
      ---
      To test for IGNORE during the execution of the multiple-table UPDATE 
      statement. This causes the successful execution of the query with
      appropriate warnings being reported.

    modified:
      mysql-test/r/multi_update.result
      mysql-test/r/multi_update_innodb.result
      mysql-test/t/multi_update.test
      mysql-test/t/multi_update_innodb.test
      sql/sql_update.cc
=== added file 'mysql-test/suite/innodb/r/innodb_bug14006907.result'
--- a/mysql-test/suite/innodb/r/innodb_bug14006907.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/innodb/r/innodb_bug14006907.result	revid:marko.makela@strippeds1r6j9j59n3
@@ -0,0 +1,23 @@
+CREATE TABLE t1 (c1 INT PRIMARY KEY, c2 INT, INDEX(c2)) ENGINE = InnoDB;
+CREATE TABLE t2 (
+c2 INT PRIMARY KEY,
+CONSTRAINT t2c2 FOREIGN KEY (c2) REFERENCES t1 (c2))
+ENGINE = InnoDB;
+DROP INDEX c2 ON t1;
+ERROR HY000: Cannot drop index 'c2': needed in a foreign key constraint
+DROP TABLE t2;
+SET DEBUG_SYNC = 'innodb_after_inplace_alter_table SIGNAL drop WAIT_FOR fk';
+DROP INDEX c2 ON t1;
+SET DEBUG_SYNC = 'now WAIT_FOR drop';
+CREATE TABLE t2 (
+c2 INT PRIMARY KEY,
+CONSTRAINT t2c2 FOREIGN KEY (c2) REFERENCES t1 (c2))
+ENGINE = InnoDB;
+ERROR HY000: Can't create table 'test.t2' (errno: 150)
+SET DEBUG_SYNC = 'now SIGNAL fk';
+CREATE TABLE t2 (
+c2 INT PRIMARY KEY,
+CONSTRAINT t2c2 FOREIGN KEY (c2) REFERENCES t1 (c2))
+ENGINE = InnoDB;
+ERROR HY000: Can't create table 'test.t2' (errno: 150)
+DROP TABLE t1;

=== added file 'mysql-test/suite/innodb/t/innodb_bug14006907.test'
--- a/mysql-test/suite/innodb/t/innodb_bug14006907.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/innodb/t/innodb_bug14006907.test	revid:marko.makela@oracle.com-20120510091615-anew3s1r6j9j59n3
@@ -0,0 +1,49 @@
+--source include/have_innodb.inc
+--source include/have_debug_sync.inc
+
+# Save the initial number of concurrent sessions.
+--source include/count_sessions.inc
+
+CREATE TABLE t1 (c1 INT PRIMARY KEY, c2 INT, INDEX(c2)) ENGINE = InnoDB;
+
+CREATE TABLE t2 (
+ c2 INT PRIMARY KEY,
+ CONSTRAINT t2c2 FOREIGN KEY (c2) REFERENCES t1 (c2))
+ENGINE = InnoDB;
+
+--error ER_DROP_INDEX_FK
+DROP INDEX c2 ON t1;
+
+DROP TABLE t2;
+
+SET DEBUG_SYNC = 'innodb_after_inplace_alter_table SIGNAL drop WAIT_FOR fk';
+--send
+DROP INDEX c2 ON t1;
+
+connect (con1,localhost,root,,);
+connection con1;
+
+SET DEBUG_SYNC = 'now WAIT_FOR drop';
+--error ER_CANT_CREATE_TABLE
+CREATE TABLE t2 (
+ c2 INT PRIMARY KEY,
+ CONSTRAINT t2c2 FOREIGN KEY (c2) REFERENCES t1 (c2))
+ENGINE = InnoDB;
+SET DEBUG_SYNC = 'now SIGNAL fk';
+
+disconnect con1;
+connection default;
+
+reap;
+
+--error ER_CANT_CREATE_TABLE
+CREATE TABLE t2 (
+ c2 INT PRIMARY KEY,
+ CONSTRAINT t2c2 FOREIGN KEY (c2) REFERENCES t1 (c2))
+ENGINE = InnoDB;
+
+DROP TABLE t1;
+
+# Check that all connections opened by test cases in this file are really
+# gone so execution of other tests won't be affected by their presence.
+--source include/wait_until_count_sessions.inc

=== modified file 'storage/innobase/dict/dict0dict.cc'
--- a/storage/innobase/dict/dict0dict.cc	revid:nisha.gopalakrishnan@strippedj8x7c7ychtvnl
+++ b/storage/innobase/dict/dict0dict.cc	revid:marko.makela@strippedn3
@@ -3051,12 +3051,15 @@ dict_foreign_find_index(
 {
 	dict_index_t*	index;
 
+	ut_ad(mutex_own(&dict_sys->mutex));
+
 	index = dict_table_get_first_index(table);
 
 	while (index != NULL) {
 		/* Ignore matches that refer to the same instance
 		(or the index is to be dropped) */
-		if (types_idx == index || index->type & DICT_FTS) {
+		if (types_idx == index || index->type & DICT_FTS
+		    || index->to_be_dropped) {
 
 			goto next_rec;
 
@@ -5581,7 +5584,6 @@ dict_table_get_index_on_name(
 
 }
 
-#if 1 /* TODO: enable this in WL#6049 (MDL for FK lookups) */
 /**********************************************************************//**
 Replace the index passed in with another equivalent index in the
 foreign key lists of the table. */
@@ -5595,6 +5597,8 @@ dict_foreign_replace_index(
 {
 	dict_foreign_t*	foreign;
 
+	ut_ad(index->to_be_dropped);
+
 	for (foreign = UT_LIST_GET_FIRST(table->foreign_list);
 	     foreign;
 	     foreign = UT_LIST_GET_NEXT(foreign_list, foreign)) {
@@ -5613,6 +5617,7 @@ dict_foreign_replace_index(
 			since this must have been checked earlier. */
 			ut_a(new_index || !trx->check_foreigns);
 			ut_ad(!new_index || new_index->table == index->table);
+			ut_ad(!new_index || !new_index->to_be_dropped);
 
 			foreign->foreign_index = new_index;
 		}
@@ -5636,12 +5641,12 @@ dict_foreign_replace_index(
 			since this must have been checked earlier. */
 			ut_a(new_index || !trx->check_foreigns);
 			ut_ad(!new_index || new_index->table == index->table);
+			ut_ad(!new_index || !new_index->to_be_dropped);
 
 			foreign->referenced_index = new_index;
 		}
 	}
 }
-#endif
 
 /**********************************************************************//**
 In case there is more than one index with the same name return the index

=== modified file 'storage/innobase/handler/handler0alter.cc'
--- a/storage/innobase/handler/handler0alter.cc	revid:nisha.gopalakrishnan@stripped3-ipdj8x7c7ychtvnl
+++ b/storage/innobase/handler/handler0alter.cc	revid:marko.makela@strippedew3s1r6j9j59n3
@@ -1183,13 +1183,21 @@ public:
 		drop (drop_arg), num_to_drop (num_to_drop_arg),
 		drop_fk (drop_fk_arg), num_to_drop_fk (num_to_drop_fk_arg),
 		online (online_arg), heap (heap_arg), trx (trx_arg),
-		indexed_table (indexed_table_arg) {}
+		indexed_table (indexed_table_arg) {
+#ifdef UNIV_DEBUG
+		for (ulint i = 0; i < num_to_add; i++) {
+			ut_ad(!add[i]->to_be_dropped);
+		}
+		for (ulint i = 0; i < num_to_drop; i++) {
+			ut_ad(drop[i]->to_be_dropped);
+		}
+#endif /* UNIV_DEBUG */
+	}
 	~ha_innobase_inplace_ctx() {
 		mem_heap_free(heap);
 	}
 };
 
-#if 1 /* TODO: enable this in WL#6049 (MDL for FK lookups) */
 /*******************************************************************//**
 Check if a foreign key constraint can make use of an index
 that is being created.
@@ -1245,7 +1253,6 @@ no_match:
 
 	return(NULL);
 }
-#endif
 
 /********************************************************************//**
 Drop any indexes that we were not able to free previously due to
@@ -1909,6 +1916,14 @@ ha_innobase::prepare_inplace_alter_table
 
 	MONITOR_ATOMIC_INC(MONITOR_PENDING_ALTER_TABLE);
 
+#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 (!(ha_alter_info->handler_flags & ~INNOBASE_INPLACE_IGNORE)) {
 		/* Nothing to do */
 		goto func_exit;
@@ -2088,7 +2103,7 @@ found_fk:
 		DBUG_ASSERT(ha_alter_info->handler_flags
 			    & (Alter_inplace_info::DROP_INDEX
 			       | Alter_inplace_info::DROP_UNIQUE_INDEX));
-		/* check which indexes to drop */
+		/* Check which indexes to drop. */
 		if (!heap) {
 			heap = mem_heap_create(1024);
 		}
@@ -2096,6 +2111,7 @@ found_fk:
 			mem_heap_alloc(
 				heap, (ha_alter_info->index_drop_count + 1)
 				* sizeof *drop_index));
+
 		for (uint i = 0; i < ha_alter_info->index_drop_count; i++) {
 			const KEY*	key
 				= ha_alter_info->index_drop_buffer[i];
@@ -2114,6 +2130,7 @@ found_fk:
 				my_error(ER_REQUIRES_PRIMARY_KEY, MYF(0));
 				goto err_exit;
 			} else {
+				ut_ad(!index->to_be_dropped);
 				drop_index[n_drop_index++] = index;
 			}
 		}
@@ -2132,6 +2149,7 @@ found_fk:
 			if fulltext indexes exist, unless the MySQL
 			and InnoDB data dictionaries are out of sync. */
 			DBUG_ASSERT(fts_doc_index != NULL);
+			DBUG_ASSERT(!fts_doc_index->to_be_dropped);
 
 			// Add some fault tolerance for non-debug builds.
 			if (fts_doc_index == NULL) {
@@ -2155,13 +2173,19 @@ found_fk:
 		}
 
 check_if_can_drop_indexes:
-		/* Check if the indexes can be dropped */
+		/* Check if the indexes can be dropped. */
+
+		/* Prevent a race condition between DROP INDEX and
+		CREATE TABLE adding FOREIGN KEY constraints. */
+		row_mysql_lock_data_dictionary(prebuilt->trx);
 
 		if (prebuilt->trx->check_foreigns) {
 			for (uint i = 0; i < n_drop_index; i++) {
 				dict_index_t*	index = drop_index[i];
 				dict_foreign_t*	foreign;
 
+				ut_ad(!index->to_be_dropped);
+
 				/* Check if the index is referenced. */
 				foreign = dict_table_get_referenced_constraint(
 					indexed_table, index);
@@ -2170,7 +2194,6 @@ check_if_can_drop_indexes:
 				      == foreign->referenced_table);
 
 				if (foreign
-#if 1 /* TODO: enable this in WL#6049 (MDL for FK lookups) */
 				    && !dict_foreign_find_index(
 					    indexed_table,
 					    foreign->referenced_col_names,
@@ -2183,12 +2206,13 @@ check_if_can_drop_indexes:
 					    ha_alter_info->key_info_buffer,
 					    ha_alter_info->index_add_buffer,
 					    ha_alter_info->index_add_count)
-#endif
 				    ) {
 index_needed:
 					prebuilt->trx->error_info = index;
 					print_error(HA_ERR_DROP_INDEX_FK,
 						    MYF(0));
+					row_mysql_unlock_data_dictionary(
+						prebuilt->trx);
 					goto err_exit;
 				}
 
@@ -2203,7 +2227,6 @@ index_needed:
 				if (foreign
 				    && !innobase_dropping_foreign(
 					    foreign, drop_fk, n_drop_fk)
-#if 1 /* TODO: enable this in WL#6049 (MDL for FK lookups) */
 				    && !dict_foreign_find_index(
 					    indexed_table,
 					    foreign->foreign_col_names,
@@ -2216,12 +2239,19 @@ index_needed:
 					    ha_alter_info->key_info_buffer,
 					    ha_alter_info->index_add_buffer,
 					    ha_alter_info->index_add_count)
-#endif
 				    ) {
 					goto index_needed;
 				}
 			}
 		}
+
+		/* Flag all indexes that are to be dropped. */
+		for (ulint i = 0; i < n_drop_index; i++) {
+			ut_ad(!drop_index[i]->to_be_dropped);
+			drop_index[i]->to_be_dropped = 1;
+		}
+
+		row_mysql_unlock_data_dictionary(prebuilt->trx);
 	} else {
 		drop_index = NULL;
 	}
@@ -2350,6 +2380,8 @@ ha_innobase::inplace_alter_table(
 #endif /* UNIV_SYNC_DEBUG */
 
 	if (!(ha_alter_info->handler_flags & INNOBASE_INPLACE_CREATE)) {
+ok_exit:
+		DEBUG_SYNC(user_thd, "innodb_after_inplace_alter_table");
 		DBUG_RETURN(false);
 	}
 
@@ -2394,8 +2426,7 @@ oom:
 		happens to be executing on this very table. */
 		DBUG_ASSERT(ctx->indexed_table == prebuilt->table
 			    || prebuilt->table->n_ref_count - 1 <= 1);
-		DEBUG_SYNC(user_thd, "innodb_after_inplace_alter_table");
-		DBUG_RETURN(false);
+		goto ok_exit;
 	case DB_DUPLICATE_KEY:
 		if (prebuilt->trx->error_key_num == ULINT_UNDEFINED) {
 			/* This should be the hidden index on FTS_DOC_ID. */
@@ -2502,6 +2533,18 @@ rollback_inplace_alter_table(
 	trx_free_for_mysql(ctx->trx);
 
 func_exit:
+	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);
+
+			index->to_be_dropped = 0;
+		}
+	}
+
 	trx_commit_for_mysql(prebuilt->trx);
 	srv_active_wake_master_thread();
 	MONITOR_ATOMIC_DEC(MONITOR_PENDING_ALTER_TABLE);
@@ -2900,6 +2943,7 @@ ha_innobase::commit_inplace_alter_table(
 			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);
 
 			error = row_merge_rename_index_to_drop(
 				trx, index->table->id, index->id);
@@ -3009,6 +3053,7 @@ trx_commit:
 				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);
 
 				/* Replace the indexes in foreign key
 				constraints if needed. */
@@ -3046,6 +3091,14 @@ 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) {

=== modified file 'storage/innobase/include/dict0dict.h'
--- a/storage/innobase/include/dict0dict.h	revid:nisha.gopalakrishnan@stripped
+++ b/storage/innobase/include/dict0dict.h	revid:marko.makela@oracle.com-20120510091615-anew3s1r6j9j59n3
@@ -398,7 +398,6 @@ dict_table_is_referenced_by_foreign_key(
 /*====================================*/
 	const dict_table_t*	table)	/*!< in: InnoDB table */
 	__attribute__((nonnull, warn_unused_result));
-#if 1 /* TODO: enable this in WL#6049 (MDL for FK lookups) */
 /**********************************************************************//**
 Replace the index passed in with another equivalent index in the
 foreign key lists of the table. */
@@ -410,7 +409,6 @@ dict_foreign_replace_index(
 	const dict_index_t*	index,	/*!< in: index to be replaced */
 	const trx_t*		trx)	/*!< in: transaction handle */
 	__attribute__((nonnull));
-#endif
 /**********************************************************************//**
 Determines whether a string starts with the specified keyword.
 @return TRUE if str starts with keyword */

=== modified file 'storage/innobase/include/dict0mem.h'
--- a/storage/innobase/include/dict0mem.h	revid:nisha.gopalakrishnan@stripped7ychtvnl
+++ b/storage/innobase/include/dict0mem.h	revid:marko.makela@stripped
@@ -483,6 +483,8 @@ struct dict_index_struct{
 	unsigned	n_nullable:10;/*!< number of nullable fields */
 	unsigned	cached:1;/*!< TRUE if the index object is in the
 				dictionary cache */
+	unsigned	to_be_dropped:1;
+				/*!< TRUE if the index is to be dropped */
 	unsigned	online_status:2;
 				/*!< enum online_index_status */
 	dict_field_t*	fields;	/*!< array of field descriptions */

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (marko.makela:3812 to 3813) Bug#14006907marko.makela7 Jun