diff options
author | Sergei Golubchik <serg@mariadb.org> | 2021-07-01 13:33:38 +0200 |
---|---|---|
committer | Sergei Golubchik <serg@mariadb.org> | 2021-07-02 16:44:00 +0200 |
commit | b5f50e2de8dd8f43f0975a3a913d808a54e87c8d (patch) | |
tree | 2589f80e747b63cfa7066ec4126c4b5bdfb28028 | |
parent | 0ad8a825a8901d4b5e704fed3fc65e93cea18f18 (diff) | |
download | mariadb-git-b5f50e2de8dd8f43f0975a3a913d808a54e87c8d.tar.gz |
errors after altering a table has finished aren't fatal
We cannot revert the ALTER, so anything happening after
the point of no return should not be treated as an error. A
very unfortunate condition that a user needs to be warned about - yes,
but we cannot say "ALTER TABLE has failed" if the table was successfully
altered.
-rw-r--r-- | mysql-test/suite/s3/alter.result | 4 | ||||
-rw-r--r-- | mysql-test/suite/s3/alter.test | 1 | ||||
-rw-r--r-- | sql/sql_class.h | 19 | ||||
-rw-r--r-- | sql/sql_table.cc | 50 | ||||
-rw-r--r-- | sql/table.cc | 19 |
5 files changed, 39 insertions, 54 deletions
diff --git a/mysql-test/suite/s3/alter.result b/mysql-test/suite/s3/alter.result index da9ddb11ea7..c6f79e8b0f9 100644 --- a/mysql-test/suite/s3/alter.result +++ b/mysql-test/suite/s3/alter.result @@ -102,7 +102,9 @@ drop table t1; create table t1 (a int, b int) engine=aria select seq as a,seq+10 as b from seq_1_to_10; lock table t1 write; alter table t1 add column c int, engine=s3; -ERROR HY000: Table 't1' is read only +Warnings: +Warning 1036 Table 't1' is read only +Warning 1213 Deadlock found when trying to get lock; try restarting transaction unlock tables; select count(*), sum(a), sum(b), sum(c) from t1; count(*) sum(a) sum(b) sum(c) diff --git a/mysql-test/suite/s3/alter.test b/mysql-test/suite/s3/alter.test index 4504804c91a..7882d14e7b4 100644 --- a/mysql-test/suite/s3/alter.test +++ b/mysql-test/suite/s3/alter.test @@ -69,7 +69,6 @@ drop table t1; create table t1 (a int, b int) engine=aria select seq as a,seq+10 as b from seq_1_to_10; lock table t1 write; ---error ER_OPEN_AS_READONLY alter table t1 add column c int, engine=s3; unlock tables; select count(*), sum(a), sum(b), sum(c) from t1; diff --git a/sql/sql_class.h b/sql/sql_class.h index 713523b7d75..548fef3f67b 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -2015,6 +2015,25 @@ private: }; +class Turn_errors_to_warnings_handler : public Internal_error_handler +{ +public: + Turn_errors_to_warnings_handler() {} + bool handle_condition(THD *thd, + uint sql_errno, + const char* sqlstate, + Sql_condition::enum_warning_level *level, + const char* msg, + Sql_condition ** cond_hdl) + { + *cond_hdl= NULL; + if (*level == Sql_condition::WARN_LEVEL_ERROR) + *level= Sql_condition::WARN_LEVEL_WARN; + return(0); + } +}; + + /** Tables that were locked with LOCK TABLES statement. diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 9610e795f7a..88fe883c2a4 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -9286,10 +9286,11 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db, uint order_num, ORDER *order, bool ignore, bool if_exists) { - bool engine_changed, error, frm_is_created= false; + bool engine_changed, error, frm_is_created= false, error_handler_pushed= false; bool no_ha_table= true; /* We have not created table in storage engine yet */ TABLE *table, *new_table; DDL_LOG_STATE ddl_log_state; + Turn_errors_to_warnings_handler errors_to_warnings; #ifdef WITH_PARTITION_STORAGE_ENGINE bool partition_changed= false; @@ -10610,19 +10611,21 @@ do_continue:; } // ALTER TABLE succeeded, delete the backup of the old table. - error= quick_rm_table(thd, old_db_type, &alter_ctx.db, &backup_name, - FN_IS_TMP | - (engine_changed ? NO_HA_TABLE | NO_PAR_TABLE: 0)); + // a failure to delete isn't an error, as we cannot rollback ALTER anymore + thd->push_internal_handler(&errors_to_warnings); + error_handler_pushed=1; + + quick_rm_table(thd, old_db_type, &alter_ctx.db, &backup_name, + FN_IS_TMP | (engine_changed ? NO_HA_TABLE | NO_PAR_TABLE: 0)); debug_crash_here("ddl_log_alter_after_delete_backup"); if (engine_changed) { /* the .frm file was removed but not the original table */ - error|= quick_rm_table(thd, old_db_type, &alter_ctx.db, - &alter_ctx.table_name, - NO_FRM_RENAME | - (engine_changed ? 0 : FN_IS_TMP)); + quick_rm_table(thd, old_db_type, &alter_ctx.db, &alter_ctx.table_name, + NO_FRM_RENAME | (engine_changed ? 0 : FN_IS_TMP)); } + debug_crash_here("ddl_log_alter_after_drop_original_table"); if (binlog_as_create_select) { @@ -10637,21 +10640,15 @@ do_continue:; thd->binlog_xid= 0; } - if (error) - { - /* - The fact that deletion of the backup failed is not critical - error, but still worth reporting as it might indicate serious - problem with server. - */ - goto err_with_mdl_after_alter; - } - end_inplace: thd->variables.option_bits&= ~OPTION_BIN_COMMIT_OFF; - if (thd->locked_tables_list.reopen_tables(thd, false)) - goto err_with_mdl_after_alter; + if (!error_handler_pushed) + thd->push_internal_handler(&errors_to_warnings); + + thd->locked_tables_list.reopen_tables(thd, false); + + thd->pop_internal_handler(); THD_STAGE_INFO(thd, stage_end); DEBUG_SYNC(thd, "alter_table_before_main_binlog"); @@ -10763,19 +10760,6 @@ err_cleanup: } DBUG_RETURN(true); -err_with_mdl_after_alter: - DBUG_PRINT("error", ("err_with_mdl_after_alter")); - /* the table was altered. binlog the operation */ - DBUG_ASSERT(!(mysql_bin_log.is_open() && - thd->is_current_stmt_binlog_format_row() && - (create_info->tmp_table()))); - /* - We can't reset error as we will return 'true' below and the server - expects that error is set - */ - if (!binlog_as_create_select) - write_bin_log_with_if_exists(thd, FALSE, FALSE, log_if_exists); - err_with_mdl: ddl_log_complete(&ddl_log_state); /* diff --git a/sql/table.cc b/sql/table.cc index 73d701e0b2b..071a0ef1f1f 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -8574,25 +8574,6 @@ bool is_simple_order(ORDER *order) return TRUE; } -class Turn_errors_to_warnings_handler : public Internal_error_handler -{ -public: - Turn_errors_to_warnings_handler() {} - bool handle_condition(THD *thd, - uint sql_errno, - const char* sqlstate, - Sql_condition::enum_warning_level *level, - const char* msg, - Sql_condition ** cond_hdl) - { - *cond_hdl= NULL; - if (*level == Sql_condition::WARN_LEVEL_ERROR) - *level= Sql_condition::WARN_LEVEL_WARN; - return(0); - } -}; - - /* to satisfy marked_for_write_or_computed() Field's assert we temporarily mark field for write before storing the generated value in it |