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#14006907 | marko.makela | 7 Jun |
