diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2019-01-29 15:00:41 +0200 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2019-01-29 15:20:26 +0200 |
commit | 1522ee2949ae304ad9092894896a6272dc08bb39 (patch) | |
tree | dda9f0115e7872032a4d6c80a08265f2cb98c0b2 | |
parent | 6699cac0bf10464feab631ff3909ca8c66405628 (diff) | |
download | mariadb-git-1522ee2949ae304ad9092894896a6272dc08bb39.tar.gz |
MDEV-18016: Assertion failure on ALTER TABLE after foreign_key_checks=0
ha_innobase::commit_inplace_alter_table(): Do not crash if
innobase_update_foreign_cache() returns an error. It can return
an error on ALTER TABLE if an inconsistent FOREIGN KEY constraint
was created earlier when SET foreign_key_checks=0 was in effect.
Instead, report a warning to the client that constraints cannot
be loaded.
-rw-r--r-- | mysql-test/suite/innodb/r/foreign_key.result | 12 | ||||
-rw-r--r-- | mysql-test/suite/innodb/t/foreign_key.test | 8 | ||||
-rw-r--r-- | storage/innobase/handler/handler0alter.cc | 58 | ||||
-rw-r--r-- | storage/xtradb/handler/handler0alter.cc | 58 |
4 files changed, 68 insertions, 68 deletions
diff --git a/mysql-test/suite/innodb/r/foreign_key.result b/mysql-test/suite/innodb/r/foreign_key.result index 6573d744714..4e253261f2e 100644 --- a/mysql-test/suite/innodb/r/foreign_key.result +++ b/mysql-test/suite/innodb/r/foreign_key.result @@ -70,3 +70,15 @@ CREATE TABLE t2 (b INT PRIMARY KEY, FOREIGN KEY fk1 (b) REFERENCES t1 (a)) ENGINE=InnoDB; ALTER TABLE t2 DROP FOREIGN KEY fk1, DROP FOREIGN KEY fk1; DROP TABLE t2, t1; +CREATE TABLE t1 (f VARCHAR(256)) ENGINE=InnoDB; +SET SESSION FOREIGN_KEY_CHECKS = OFF; +ALTER TABLE t1 ADD FOREIGN KEY (f) REFERENCES non_existing_table (x); +SET SESSION FOREIGN_KEY_CHECKS = ON; +ALTER TABLE t1 ADD FULLTEXT INDEX ft1 (f); +Warnings: +Warning 124 InnoDB rebuilding table to add column FTS_DOC_ID +Warning 1088 failed to load FOREIGN KEY constraints +ALTER TABLE t1 ADD FULLTEXT INDEX ft2 (f); +Warnings: +Warning 1088 failed to load FOREIGN KEY constraints +DROP TABLE t1; diff --git a/mysql-test/suite/innodb/t/foreign_key.test b/mysql-test/suite/innodb/t/foreign_key.test index aa35e3abf00..b4e2ee1bbe7 100644 --- a/mysql-test/suite/innodb/t/foreign_key.test +++ b/mysql-test/suite/innodb/t/foreign_key.test @@ -96,3 +96,11 @@ CREATE TABLE t2 (b INT PRIMARY KEY, FOREIGN KEY fk1 (b) REFERENCES t1 (a)) ENGINE=InnoDB; ALTER TABLE t2 DROP FOREIGN KEY fk1, DROP FOREIGN KEY fk1; DROP TABLE t2, t1; + +CREATE TABLE t1 (f VARCHAR(256)) ENGINE=InnoDB; +SET SESSION FOREIGN_KEY_CHECKS = OFF; +ALTER TABLE t1 ADD FOREIGN KEY (f) REFERENCES non_existing_table (x); +SET SESSION FOREIGN_KEY_CHECKS = ON; +ALTER TABLE t1 ADD FULLTEXT INDEX ft1 (f); +ALTER TABLE t1 ADD FULLTEXT INDEX ft2 (f); +DROP TABLE t1; diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 17e2810b649..40a04c8848f 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -5638,7 +5638,6 @@ ha_innobase::commit_inplace_alter_table( Alter_inplace_info* ha_alter_info, bool commit) { - dberr_t error; ha_innobase_inplace_ctx* ctx0 = static_cast<ha_innobase_inplace_ctx*> (ha_alter_info->handler_ctx); @@ -5705,7 +5704,7 @@ ha_innobase::commit_inplace_alter_table( transactions collected during crash recovery could be holding InnoDB locks only, not MySQL locks. */ - error = row_merge_lock_table( + dberr_t error = row_merge_lock_table( prebuilt->trx, ctx->old_table, LOCK_X); if (error != DB_SUCCESS) { @@ -5890,9 +5889,9 @@ rollback_trx: file operations that will be performed in commit_cache_rebuild(), and if none, generate the redo log for these operations. */ - error = fil_mtr_rename_log(ctx->old_table, - ctx->new_table, - ctx->tmp_name, &mtr); + dberr_t error = fil_mtr_rename_log( + ctx->old_table, ctx->new_table, ctx->tmp_name, + &mtr); if (error != DB_SUCCESS) { /* Out of memory or a problem will occur when renaming files. */ @@ -6017,39 +6016,30 @@ rollback_trx: /* Rename the tablespace files. */ commit_cache_rebuild(ctx); - error = innobase_update_foreign_cache(ctx, user_thd); - if (error != DB_SUCCESS) { - goto foreign_fail; + if (innobase_update_foreign_cache(ctx, user_thd) + != DB_SUCCESS + && prebuilt->trx->check_foreigns) { +foreign_fail: + push_warning_printf( + user_thd, + Sql_condition::WARN_LEVEL_WARN, + ER_ALTER_INFO, + "failed to load FOREIGN KEY" + " constraints"); } } else { - error = innobase_update_foreign_cache(ctx, user_thd); + bool fk_fail = innobase_update_foreign_cache( + ctx, user_thd) != DB_SUCCESS; - if (error != DB_SUCCESS) { -foreign_fail: - /* The data dictionary cache - should be corrupted now. The - best solution should be to - kill and restart the server, - but the *.frm file has not - been replaced yet. */ - my_error(ER_CANNOT_ADD_FOREIGN, - MYF(0)); - sql_print_error( - "InnoDB: dict_load_foreigns()" - " returned %u for %s", - (unsigned) error, - thd_query_string(user_thd) - ->str); - ut_ad(0); - } else { - if (!commit_cache_norebuild( - ctx, table, trx)) { - ut_a(!prebuilt->trx->check_foreigns); - } + if (!commit_cache_norebuild(ctx, table, trx)) { + fk_fail = true; + ut_ad(!prebuilt->trx->check_foreigns); + } - innobase_rename_columns_cache( - ha_alter_info, table, - ctx->new_table); + innobase_rename_columns_cache(ha_alter_info, table, + ctx->new_table); + if (fk_fail && prebuilt->trx->check_foreigns) { + goto foreign_fail; } } DBUG_INJECT_CRASH("ib_commit_inplace_crash", diff --git a/storage/xtradb/handler/handler0alter.cc b/storage/xtradb/handler/handler0alter.cc index c27cd7f1b40..cd8fc8ad589 100644 --- a/storage/xtradb/handler/handler0alter.cc +++ b/storage/xtradb/handler/handler0alter.cc @@ -5654,7 +5654,6 @@ ha_innobase::commit_inplace_alter_table( Alter_inplace_info* ha_alter_info, bool commit) { - dberr_t error; ha_innobase_inplace_ctx* ctx0 = static_cast<ha_innobase_inplace_ctx*> (ha_alter_info->handler_ctx); @@ -5721,7 +5720,7 @@ ha_innobase::commit_inplace_alter_table( transactions collected during crash recovery could be holding InnoDB locks only, not MySQL locks. */ - error = row_merge_lock_table( + dberr_t error = row_merge_lock_table( prebuilt->trx, ctx->old_table, LOCK_X); if (error != DB_SUCCESS) { @@ -5906,9 +5905,9 @@ rollback_trx: file operations that will be performed in commit_cache_rebuild(), and if none, generate the redo log for these operations. */ - error = fil_mtr_rename_log(ctx->old_table, - ctx->new_table, - ctx->tmp_name, &mtr); + dberr_t error = fil_mtr_rename_log( + ctx->old_table, ctx->new_table, ctx->tmp_name, + &mtr); if (error != DB_SUCCESS) { /* Out of memory or a problem will occur when renaming files. */ @@ -6033,39 +6032,30 @@ rollback_trx: /* Rename the tablespace files. */ commit_cache_rebuild(ctx); - error = innobase_update_foreign_cache(ctx, user_thd); - if (error != DB_SUCCESS) { - goto foreign_fail; + if (innobase_update_foreign_cache(ctx, user_thd) + != DB_SUCCESS + && prebuilt->trx->check_foreigns) { +foreign_fail: + push_warning_printf( + user_thd, + Sql_condition::WARN_LEVEL_WARN, + ER_ALTER_INFO, + "failed to load FOREIGN KEY" + " constraints"); } } else { - error = innobase_update_foreign_cache(ctx, user_thd); + bool fk_fail = innobase_update_foreign_cache( + ctx, user_thd) != DB_SUCCESS; - if (error != DB_SUCCESS) { -foreign_fail: - /* The data dictionary cache - should be corrupted now. The - best solution should be to - kill and restart the server, - but the *.frm file has not - been replaced yet. */ - my_error(ER_CANNOT_ADD_FOREIGN, - MYF(0)); - sql_print_error( - "InnoDB: dict_load_foreigns()" - " returned %u for %s", - (unsigned) error, - thd_query_string(user_thd) - ->str); - ut_ad(0); - } else { - if (!commit_cache_norebuild( - ctx, table, trx)) { - ut_a(!prebuilt->trx->check_foreigns); - } + if (!commit_cache_norebuild(ctx, table, trx)) { + fk_fail = true; + ut_ad(!prebuilt->trx->check_foreigns); + } - innobase_rename_columns_cache( - ha_alter_info, table, - ctx->new_table); + innobase_rename_columns_cache(ha_alter_info, table, + ctx->new_table); + if (fk_fail && prebuilt->trx->check_foreigns) { + goto foreign_fail; } } DBUG_INJECT_CRASH("ib_commit_inplace_crash", |