diff --git a/mysql-test/r/multi_update.result b/mysql-test/r/multi_update.result index 449333a4ae6..04bf7720c43 100644 --- a/mysql-test/r/multi_update.result +++ b/mysql-test/r/multi_update.result @@ -634,4 +634,15 @@ select count(*) from t3 /* must be 1 */; count(*) 1 drop table t1, t2, t3; +# +# Bug#49534: multitable IGNORE update with sql_safe_updates error +# causes debug assertion +# +CREATE TABLE t1( a INT, KEY( a ) ); +INSERT INTO t1 VALUES (1), (2), (3); +SET SESSION sql_safe_updates = 1; +# Must not cause failed assertion +UPDATE IGNORE t1, t1 t1a SET t1.a = 1 WHERE t1a.a = 1; +ERROR HY000: You are using safe update mode and you tried to update a table without a WHERE that uses a KEY column +DROP TABLE t1; end of tests diff --git a/mysql-test/t/multi_update.test b/mysql-test/t/multi_update.test index fc37fd6a27d..3c33a3dde35 100644 --- a/mysql-test/t/multi_update.test +++ b/mysql-test/t/multi_update.test @@ -637,5 +637,17 @@ drop table t1, t2, t3; # Add further tests from here # +--echo # +--echo # Bug#49534: multitable IGNORE update with sql_safe_updates error +--echo # causes debug assertion +--echo # +CREATE TABLE t1( a INT, KEY( a ) ); +INSERT INTO t1 VALUES (1), (2), (3); +SET SESSION sql_safe_updates = 1; +--echo # Must not cause failed assertion +--error ER_UPDATE_WITHOUT_KEY_IN_SAFE_MODE +UPDATE IGNORE t1, t1 t1a SET t1.a = 1 WHERE t1a.a = 1; +DROP TABLE t1; + --echo end of tests diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 7b37a3f6e93..b22aa41b40a 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -740,10 +740,12 @@ bool THD::handle_error(uint sql_errno, const char *message, } -void THD::pop_internal_handler() +Internal_error_handler *THD::pop_internal_handler() { DBUG_ASSERT(m_internal_handler != NULL); + Internal_error_handler *popped_handler= m_internal_handler; m_internal_handler= m_internal_handler->m_prev_internal_handler; + return popped_handler; } extern "C" diff --git a/sql/sql_class.h b/sql/sql_class.h index 77b4aa6c1d0..032985dc44e 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -2313,7 +2313,7 @@ public: /** Remove the error handler last pushed. */ - void pop_internal_handler(); + Internal_error_handler *pop_internal_handler(); /** Overloaded to guard query/query_length fields */ virtual void set_statement(Statement *stmt); diff --git a/sql/sql_update.cc b/sql/sql_update.cc index 32add8679ef..84610630d62 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -1188,6 +1188,56 @@ reopen_tables: } +/** + Implementation of the safe update options during UPDATE IGNORE. This syntax + causes an UPDATE statement to ignore all errors. In safe update mode, + however, we must never ignore the ER_UPDATE_WITHOUT_KEY_IN_SAFE_MODE. There + is a special hook in my_message_sql that will otherwise delete all errors + when the IGNORE option is specified. + + In the future, all IGNORE handling should be used with this class and all + traces of the hack outlined below should be removed. + + - The parser detects IGNORE option and sets thd->lex->ignore= 1 + + - In JOIN::optimize, if this is set, then + thd->lex->current_select->no_error gets set. + + - In my_message_sql(), if the flag above is set then any error is + unconditionally converted to a warning. + + We are moving in the direction of using Internal_error_handler subclasses + to do all such error tweaking, please continue this effort if new bugs + appear. + */ +class Safe_dml_handler : public Internal_error_handler { + +private: + bool m_handled_error; + +public: + explicit Safe_dml_handler() : m_handled_error(FALSE) {} + + bool handle_error(uint sql_errno, + const char *message, + MYSQL_ERROR::enum_warning_level level, + THD *thd) + { + if (level == MYSQL_ERROR::WARN_LEVEL_ERROR && + sql_errno == ER_UPDATE_WITHOUT_KEY_IN_SAFE_MODE) + + { + thd->main_da.set_error_status(thd, sql_errno, message); + m_handled_error= TRUE; + return TRUE; + } + return FALSE; + } + + bool handled_error() { return m_handled_error; } + +}; + /* Setup multi-update handling and call SELECT to do the join */ @@ -1216,18 +1266,35 @@ bool mysql_multi_update(THD *thd, MODE_STRICT_ALL_TABLES)); List total_list; + + Safe_dml_handler handler; + bool using_handler= thd->options & OPTION_SAFE_UPDATES; + if (using_handler) + thd->push_internal_handler(&handler); + res= mysql_select(thd, &select_lex->ref_pointer_array, - table_list, select_lex->with_wild, - total_list, - conds, 0, (ORDER *) NULL, (ORDER *)NULL, (Item *) NULL, - (ORDER *)NULL, - options | SELECT_NO_JOIN_CACHE | SELECT_NO_UNLOCK | - OPTION_SETUP_TABLES_DONE, - result, unit, select_lex); - DBUG_PRINT("info",("res: %d report_error: %d", res, - (int) thd->is_error())); + table_list, select_lex->with_wild, + total_list, + conds, 0, (ORDER *) NULL, (ORDER *)NULL, (Item *) NULL, + (ORDER *)NULL, + options | SELECT_NO_JOIN_CACHE | SELECT_NO_UNLOCK | + OPTION_SETUP_TABLES_DONE, + result, unit, select_lex); + + if (using_handler) + { + Internal_error_handler *top_handler= thd->pop_internal_handler(); + DBUG_ASSERT(&handler == top_handler); + } + + DBUG_PRINT("info",("res: %d report_error: %d", res, (int) thd->is_error())); res|= thd->is_error(); - if (unlikely(res)) + /* + Todo: remove below code and make Safe_dml_handler do error processing + instead. That way we can return the actual error instead of + ER_UNKNOWN_ERROR. + */ + if (unlikely(res) && (!using_handler || !handler.handled_error())) { /* If we had a another error reported earlier then this will be ignored */ result->send_error(ER_UNKNOWN_ERROR, ER(ER_UNKNOWN_ERROR));