From: Date: July 18 2007 3:28pm Subject: bk commit into 5.0 tree (evgen:1.2500) BUG#24989 List-Archive: http://lists.mysql.com/commits/31088 X-Bug: 24989 Message-Id: <20070718132818.6E59322CC45@moonbone.moonbone.local> Below is the list of changes that have just been committed into a local 5.0 repository of evgen. When evgen does a push these changes will be propagated to the main repository and, within 24 hours after the push, to the public repository. For information on how to access the public repository see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html ChangeSet@stripped, 2007-07-18 17:28:14+04:00, evgen@stripped +9 -0 Bug#24989: The DEADLOCK error is improperly handled by InnoDB. When innodb detects a deadlock it calls ha_rollback_trans() to rollback the main transaction. But such action isn't allowed from inside of triggers and functions. When it happen the 'Explicit or implicit commit' error is thrown even if there is no commit/rollback statements in the trigger/function. This leads to the user confusion. Now the convert_error_code_to_mysql() function doesn't call the ha_rollback_trans() function directly but rather calls the mark_transaction_to_rollback function and returns an error. The sp_rcontext::find_handler() now doesn't allow errors to be caught by the trigger/function error handlers when the thd->is_fatal_sub_stmt_error flag is set. Instead it tries to find a most inner procedure that isn't called directly or indirectly from any function/trigger. Procedures are still allowed to catch such errors. The sp_rcontext::find_handler function now accepts a THD handle as a parameter. The transaction_rollback_request and the is_fatal_sub_stmt_error flags are added to the THD class. The are initialized by the THD class constructor. Now the ha_autocommit_or_rollback function rolls back main transaction when not in a sub statement and the thd->transaction_rollback_request is set. The THD::restore_sub_statement_state function now resets the thd->is_fatal_sub_stmt_error flag on exit from a sub-statement. mysql-test/r/innodb_mysql.result@stripped, 2007-07-18 17:27:18+04:00, evgen@stripped +62 -0 Added a test case for the bug#24989: The DEADLOCK error is improperly handled by InnoDB. mysql-test/t/innodb_mysql.test@stripped, 2007-07-18 17:27:19+04:00, evgen@stripped +112 -0 Added a test case for the bug#24989: The DEADLOCK error is improperly handled by InnoDB. sql/ha_innodb.cc@stripped, 2007-07-18 17:27:15+04:00, evgen@stripped +3 -9 Bug#24989: The DEADLOCK error is improperly handled by InnoDB. Now the convert_error_code_to_mysql() function doesn't call the ha_rollback_trans() function directly but rather calls the mark_transaction_to_rollback function and returns an error. sql/handler.cc@stripped, 2007-07-18 17:27:17+04:00, evgen@stripped +5 -0 Bug#24989: The DEADLOCK error is improperly handled by InnoDB. Now the ha_autocommit_or_rollback function rolls back main transaction when not in a sub statement and the thd->transaction_rollback_request is set. sql/sp_head.cc@stripped, 2007-07-18 17:27:17+04:00, evgen@stripped +0 -8 Bug#24989: The DEADLOCK error is improperly handled by InnoDB. The sp_head::execute function now allows execution of a continue handler after fatal error but aborts right after it. sql/sp_rcontext.cc@stripped, 2007-07-18 17:27:35+04:00, evgen@stripped +33 -3 Bug#24989: The DEADLOCK error is improperly handled by InnoDB. The sp_rcontext::find_handler() now doesn't allow errors to be caught by the trigger/function error handlers when the thd->is_fatal_sub_stmt_error flag is set. Instead it tries to find a most inner procedure that isn't called directly or indirectly from any function/trigger. Procedures are still allowed to catch such errors. The sp_rcontext::find_handler function now accepts a THD handle as a parameter. sql/sp_rcontext.h@stripped, 2007-07-18 17:27:17+04:00, evgen@stripped +1 -3 Bug#24989: The DEADLOCK error is improperly handled by InnoDB. The sp_rcontext::find_handler function now accepts a THD handle as a parameter. sql/sql_class.cc@stripped, 2007-07-18 17:27:18+04:00, evgen@stripped +21 -1 Bug#24989: The DEADLOCK error is improperly handled by InnoDB. Initialization of the transaction_rollback_request and the is_fatal_sub_stmt_error flags are added to the THD class constructor. The mark_transaction_to_rollback function is added. The THD::restore_sub_statement_state function now resets the thd->is_fatal_sub_stmt_error flag on exit from a sub-statement. sql/sql_class.h@stripped, 2007-07-18 17:27:18+04:00, evgen@stripped +4 -1 Bug#24989: The DEADLOCK error is improperly handled by InnoDB. The transaction_rollback_request and the is_fatal_sub_stmt_error flags are added to the THD class. # This is a BitKeeper patch. What follows are the unified diffs for the # set of deltas contained in the patch. The rest of the patch, the part # that BitKeeper cares about, is below these diffs. # User: evgen # Host: moonbone.local # Root: /mnt/gentoo64/work/24989-bug-5.0-opt-mysql --- 1.233/sql/handler.cc 2007-05-07 23:12:13 +04:00 +++ 1.234/sql/handler.cc 2007-07-18 17:27:17 +04:00 @@ -821,6 +821,9 @@ int ha_rollback_trans(THD *thd, bool all } } #endif /* USING_TRANSACTIONS */ + if (all) + thd->transaction_rollback_request= FALSE; + /* If a non-transactional table was updated, warn; don't warn if this is a slave thread (because when a slave thread executes a ROLLBACK, it has @@ -858,6 +861,8 @@ int ha_autocommit_or_rollback(THD *thd, if (ha_commit_stmt(thd)) error=1; } + else if (thd->transaction_rollback_request && !thd->in_sub_stmt) + (void) ha_rollback(thd); else (void) ha_rollback_stmt(thd); --- 1.270/sql/sql_class.cc 2007-04-24 19:25:50 +04:00 +++ 1.271/sql/sql_class.cc 2007-07-18 17:27:18 +04:00 @@ -173,6 +173,7 @@ THD::THD() Open_tables_state(refresh_version), lock_id(&main_lock_id), user_time(0), in_sub_stmt(0), global_read_lock(0), is_fatal_error(0), + transaction_rollback_request(0), is_fatal_sub_stmt_error(0), rand_used(0), time_zone_used(0), last_insert_id_used(0), last_insert_id_used_bin_log(0), insert_id_used(0), clear_next_insert_id(0), in_lock_tables(0), bootstrap(0), @@ -967,7 +968,7 @@ void select_send::abort() { DBUG_ENTER("select_send::abort"); if (status && thd->spcont && - thd->spcont->find_handler(thd->net.last_errno, + thd->spcont->find_handler(thd, thd->net.last_errno, MYSQL_ERROR::WARN_LEVEL_ERROR)) { /* @@ -2150,6 +2151,8 @@ void THD::restore_sub_statement_state(Su limit_found_rows= backup->limit_found_rows; sent_row_count= backup->sent_row_count; client_capabilities= backup->client_capabilities; + if (!in_sub_stmt) + is_fatal_sub_stmt_error= FALSE; if ((options & OPTION_BIN_LOG) && is_update_query(lex->sql_command)) mysql_bin_log.stop_union_events(this); @@ -2163,6 +2166,23 @@ void THD::restore_sub_statement_state(Su } +/* + Mark transaction to rollback. + + SYNOPSIS + mark_transaction_to_rollback() + thd Thread handle + all TRUE <=> rollback main transaction. + + DESCRIPTION + Mark transaction to rollback and mark error as fatal to a sub-statement. +*/ + +void mark_transaction_to_rollback(THD *thd, bool all) +{ + thd->is_fatal_sub_stmt_error= TRUE; + thd->transaction_rollback_request= all; +} /*************************************************************************** Handling of XA id cacheing ***************************************************************************/ --- 1.331/sql/sql_class.h 2007-05-15 13:56:04 +04:00 +++ 1.332/sql/sql_class.h 2007-07-18 17:27:18 +04:00 @@ -1406,7 +1406,8 @@ public: bool slave_thread, one_shot_set; bool locked, some_tables_deleted; bool last_cuted_field; - bool no_errors, password, is_fatal_error; + bool no_errors, password, is_fatal_error, transaction_rollback_request; + bool is_fatal_sub_stmt_error; bool query_start_used, rand_used, time_zone_used; /* @@ -2338,3 +2339,5 @@ public: /* Functions in sql_class.cc */ void add_to_status(STATUS_VAR *to_var, STATUS_VAR *from_var); +void mark_transaction_to_rollback(THD *thd, bool all); + --- 1.21/mysql-test/r/innodb_mysql.result 2007-05-16 00:10:55 +04:00 +++ 1.22/mysql-test/r/innodb_mysql.result 2007-07-18 17:27:18 +04:00 @@ -617,4 +617,66 @@ EXPLAIN SELECT COUNT(*) FROM t2 WHERE st id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE t2 range idx1,idx2 idx1 9 NULL 2 Using where; Using index DROP TABLE t1,t2; +CREATE TABLE t1 (f1 int NOT NULL) ENGINE=InnoDB; +CREATE TABLE t2 (f2 int(11) NOT NULL PRIMARY KEY AUTO_INCREMENT) ENGINE=InnoDB; +CREATE TRIGGER t1_bi before INSERT +ON t1 FOR EACH ROW +BEGIN +DECLARE CONTINUE HANDLER FOR SQLSTATE '40001' SET @a:= 'deadlock'; +DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET @a:= 'exception'; +INSERT INTO t2 (f2) VALUES (1); +DELETE FROM t2 WHERE f2 = 1; +END;| +CREATE PROCEDURE proc24989() +BEGIN +DECLARE CONTINUE HANDLER FOR SQLSTATE '40001' SET @b:= 'deadlock'; +DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET @a:= 'exception'; +INSERT INTO t2 (f2) VALUES (1); +DELETE FROM t2 WHERE f2 = 1; +END;| +start transaction; +select get_lock("lock24989",10); +get_lock("lock24989",10) +1 +insert into t1 values(1); +start transaction; +insert into t2 values(123); +select get_lock("lock24989",10); +get_lock("lock24989",10) +0 +insert into t1 values(1); +select release_lock("lock24989"); +release_lock("lock24989") +1 +insert into t1 values(1); +ERROR 40001: Deadlock found when trying to get lock; try restarting transaction +select @a; +@a +NULL +select * from t2; +f2 +commit; +start transaction; +select get_lock("lock24989",10); +get_lock("lock24989",10) +1 +insert into t1 values(1); +start transaction; +insert into t2 values(123); +select get_lock("lock24989",10); +get_lock("lock24989",10) +0 +call proc24989(); +select release_lock("lock24989"); +release_lock("lock24989") +1 +insert into t1 values(1); +select @a,@b; +@a @b +exception deadlock +select * from t2; +f2 +commit; +drop procedure proc24989; +drop table t1,t2; End of 5.0 tests --- 1.21/mysql-test/t/innodb_mysql.test 2007-05-16 00:10:55 +04:00 +++ 1.22/mysql-test/t/innodb_mysql.test 2007-07-18 17:27:19 +04:00 @@ -597,4 +597,116 @@ EXPLAIN SELECT COUNT(*) FROM t2 WHERE st DROP TABLE t1,t2; +# +# Bug#24989: The DEADLOCK error is improperly handled by InnoDB. +# +CREATE TABLE t1 (f1 int NOT NULL) ENGINE=InnoDB; +CREATE TABLE t2 (f2 int(11) NOT NULL PRIMARY KEY AUTO_INCREMENT) ENGINE=InnoDB; +DELIMITER |; +CREATE TRIGGER t1_bi before INSERT + ON t1 FOR EACH ROW +BEGIN + DECLARE CONTINUE HANDLER FOR SQLSTATE '40001' SET @a:= 'deadlock'; + DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET @a:= 'exception'; + INSERT INTO t2 (f2) VALUES (1); + DELETE FROM t2 WHERE f2 = 1; +END;| + +CREATE PROCEDURE proc24989() +BEGIN + DECLARE CONTINUE HANDLER FOR SQLSTATE '40001' SET @b:= 'deadlock'; + DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET @a:= 'exception'; + INSERT INTO t2 (f2) VALUES (1); + DELETE FROM t2 WHERE f2 = 1; +END;| + +create procedure proc24989_2() + deterministic +begin + declare continue handler for sqlexception + select 'Outer handler' as 'exception'; + + insert into t1 values(1); + select "continued"; +end| + +DELIMITER ;| + +connect (con1,localhost,root,,); +connect (con2,localhost,root,,); + +connection con1; +start transaction; +select get_lock("lock24989",10); +insert into t1 values(1); + +connection con2; +start transaction; +insert into t2 values(123); +select get_lock("lock24989",10); +send insert into t1 values(1); + +connection con1; +select release_lock("lock24989"); +insert into t1 values(1); + +connection con2; +--error 1213 +reap; +select @a; +# check that the whole transaction was rolled back +select * from t2; + +connection con1; +commit; +start transaction; +select get_lock("lock24989",10); +insert into t1 values(1); + +connection con2; +start transaction; +insert into t2 values(123); +select get_lock("lock24989",10); +send call proc24989(); + +connection con1; +select release_lock("lock24989"); +insert into t1 values(1); + +connection con2; +reap; +select @a,@b; +# check that the whole transaction was rolled back +select * from t2; + +connection con1; +commit; +start transaction; +select get_lock("lock24989",10); +insert into t1 values(1); + +connection con2; +start transaction; +insert into t2 values(123); +select get_lock("lock24989",10); +send call proc24989_2(); + +connection con1; +select release_lock("lock24989"); +insert into t1 values(1); +commit; + +connection con2; +reap; +# check that the whole transaction was rolled back +select * from t2; + +disconnect con1; +disconnect con2; +connection default; +drop procedure proc24989; +drop procedure proc24989_2; +drop table t1,t2; + + --echo End of 5.0 tests --- 1.313/sql/ha_innodb.cc 2007-04-29 17:40:41 +04:00 +++ 1.314/sql/ha_innodb.cc 2007-07-18 17:27:15 +04:00 @@ -455,9 +455,7 @@ convert_error_code_to_mysql( tell it also to MySQL so that MySQL knows to empty the cached binlog for this transaction */ - if (thd) { - ha_rollback(thd); - } + mark_transaction_to_rollback(thd, TRUE); return(HA_ERR_LOCK_DEADLOCK); @@ -467,9 +465,7 @@ convert_error_code_to_mysql( latest SQL statement in a lock wait timeout. Previously, we rolled back the whole transaction. */ - if (thd && row_rollback_on_timeout) { - ha_rollback(thd); - } + mark_transaction_to_rollback(thd, row_rollback_on_timeout); return(HA_ERR_LOCK_WAIT_TIMEOUT); @@ -521,9 +517,7 @@ convert_error_code_to_mysql( tell it also to MySQL so that MySQL knows to empty the cached binlog for this transaction */ - if (thd) { - ha_rollback(thd); - } + mark_transaction_to_rollback(thd, TRUE); return(HA_ERR_LOCK_TABLE_FULL); } else { --- 1.246/sql/sp_head.cc 2007-05-16 09:51:57 +04:00 +++ 1.247/sql/sp_head.cc 2007-07-18 17:27:17 +04:00 @@ -1303,9 +1303,7 @@ sp_head::execute_trigger(THD *thd, const goto err_with_cleanup; } -#ifndef DBUG_OFF nctx->sp= this; -#endif thd->spcont= nctx; @@ -1414,9 +1412,7 @@ sp_head::execute_function(THD *thd, Item */ thd->restore_active_arena(&call_arena, &backup_arena); -#ifndef DBUG_OFF nctx->sp= this; -#endif /* Pass arguments. */ for (arg_no= 0; arg_no < argcount; arg_no++) @@ -1600,9 +1596,7 @@ sp_head::execute_procedure(THD *thd, Lis DBUG_RETURN(TRUE); } -#ifndef DBUG_OFF octx->sp= 0; -#endif thd->spcont= octx; /* set callers_arena to thd, for upper-level function to work */ @@ -1616,9 +1610,7 @@ sp_head::execute_procedure(THD *thd, Lis thd->spcont= save_spcont; DBUG_RETURN(TRUE); } -#ifndef DBUG_OFF nctx->sp= this; -#endif if (params > 0) { --- 1.44/sql/sp_rcontext.cc 2006-12-23 22:04:26 +03:00 +++ 1.45/sql/sp_rcontext.cc 2007-07-18 17:27:35 +04:00 @@ -191,7 +191,7 @@ sp_rcontext::set_return_value(THD *thd, */ bool -sp_rcontext::find_handler(uint sql_errno, +sp_rcontext::find_handler(THD *thd, uint sql_errno, MYSQL_ERROR::enum_warning_level level) { if (m_hfound >= 0) @@ -200,6 +200,36 @@ sp_rcontext::find_handler(uint sql_errno const char *sqlstate= mysql_errno_to_sqlstate(sql_errno); int i= m_hcount, found= -1; + /* + Don't allow fatal sub statement errors to be caught in a sub statement. + Appropriate context to handle such errors would be a most inner procedure + which isn't called directly or indirectly from any function/trigger. + */ + if (thd->is_fatal_sub_stmt_error && thd->in_sub_stmt) + { + bool ret= FALSE; + sp_rcontext *found_ctx= 0, *ctx= this; + while ((ctx= ctx->m_prev_runtime_ctx)) + { + if (!ctx->sp) + continue; + if (ctx->sp->m_type == TYPE_ENUM_PROCEDURE) + { + if (!found_ctx) + found_ctx= ctx; + } + else + found_ctx= 0; + } + if (found_ctx) + { + thd->in_sub_stmt= FALSE; + ret= found_ctx->find_handler(thd, sql_errno, level); + thd->in_sub_stmt= TRUE; + } + return ret; + } + /* Search handlers from the latest (innermost) to the oldest (outermost) */ while (i--) { @@ -252,7 +282,7 @@ sp_rcontext::find_handler(uint sql_errno */ if (m_prev_runtime_ctx && IS_EXCEPTION_CONDITION(sqlstate) && level == MYSQL_ERROR::WARN_LEVEL_ERROR) - return m_prev_runtime_ctx->find_handler(sql_errno, level); + return m_prev_runtime_ctx->find_handler(thd, sql_errno, level); return FALSE; } m_hfound= found; @@ -298,7 +328,7 @@ sp_rcontext::handle_error(uint sql_errno elevated_level= MYSQL_ERROR::WARN_LEVEL_ERROR; } - if (find_handler(sql_errno, elevated_level)) + if (find_handler(thd, sql_errno, elevated_level)) { if (elevated_level == MYSQL_ERROR::WARN_LEVEL_ERROR) { --- 1.34/sql/sp_rcontext.h 2006-12-23 22:04:26 +03:00 +++ 1.35/sql/sp_rcontext.h 2007-07-18 17:27:17 +04:00 @@ -75,13 +75,11 @@ class sp_rcontext : public Sql_alloc */ Query_arena *callers_arena; -#ifndef DBUG_OFF /* The routine for which this runtime context is created. Used for checking if correct runtime context is used for variable handling. */ sp_head *sp; -#endif sp_rcontext(sp_pcontext *root_parsing_ctx, Field *return_value_fld, sp_rcontext *prev_runtime_ctx); @@ -125,7 +123,7 @@ class sp_rcontext : public Sql_alloc // Returns 1 if a handler was found, 0 otherwise. bool - find_handler(uint sql_errno,MYSQL_ERROR::enum_warning_level level); + find_handler(THD *thd, uint sql_errno,MYSQL_ERROR::enum_warning_level level); // If there is an error handler for this error, handle it and return TRUE. bool