MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:eugene Date:July 4 2007 7:24pm
Subject:bk commit into 5.0 tree (evgen:1.2500) BUG#24989
View as plain text  
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-04 23:23:58+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. Procedures are still allowed to catch such errors.
  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 mysql_execute_command function resets the THD::is_fatal_sub_stmt_error
  flag.
  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 sp_head::execute function now allows execution of a continue handler after
  fatal error but aborts right after it.

  mysql-test/r/innodb_mysql.result@stripped, 2007-07-04 22:50:58+04:00, evgen@stripped +63 -2
    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-04 22:50:54+04:00, evgen@stripped +81 -0
    Added a test case for the bug#24989: The DEADLOCK error is improperly handled by InnoDB.

  sql/ha_innodb.cc@stripped, 2007-07-04 23:02:38+04:00, evgen@stripped +3 -22
    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-04 23:07:12+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-04 22:58:46+04:00, evgen@stripped +21 -5
    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-04 22:56:14+04:00, evgen@stripped +8 -1
    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. Procedures are still allowed to catch such errors.

  sql/sql_class.cc@stripped, 2007-07-04 23:04:51+04:00, evgen@stripped +18 -0
    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.

  sql/sql_class.h@stripped, 2007-07-04 22:54:49+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.

  sql/sql_parse.cc@stripped, 2007-07-04 22:52:54+04:00, evgen@stripped +3 -0
    Bug#24989: The DEADLOCK error is improperly handled by InnoDB.
    Now the mysql_execute_command function resets the THD::is_fatal_sub_stmt_error
    flag.

# 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-04 23:07:12 +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-04 23:04:51 +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),
@@ -2163,6 +2164,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-04 22:54:49 +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.621/sql/sql_parse.cc	2007-05-15 13:56:04 +04:00
+++ 1.622/sql/sql_parse.cc	2007-07-04 22:52:54 +04:00
@@ -5107,6 +5107,8 @@ create_sp_error:
   if (thd->one_shot_set && lex->sql_command != SQLCOM_SET_OPTION)
     reset_one_shot_variables(thd);
 
+  if (!thd->spcont)
+    thd->is_fatal_sub_stmt_error= FALSE;
   /*
     The return value for ROW_COUNT() is "implementation dependent" if the
     statement is not DELETE, INSERT or UPDATE, but -1 is what JDBC and ODBC
@@ -5812,6 +5814,7 @@ void mysql_reset_thd_for_next_command(TH
   thd->query_start_used= thd->insert_id_used=0;
   thd->last_insert_id_used_bin_log= FALSE;
   thd->is_fatal_error= thd->time_zone_used= 0;
+  thd->transaction_rollback_request= FALSE;
   thd->server_status&= ~ (SERVER_MORE_RESULTS_EXISTS | 
                           SERVER_QUERY_NO_INDEX_USED |
                           SERVER_QUERY_NO_GOOD_INDEX_USED);

--- 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-04 22:50:58 +04:00
@@ -487,7 +487,6 @@ ERROR HY000: Lock wait timeout exceeded;
 select * from t1;
 a
 1
-5
 commit;
 select * from t1;
 a
@@ -498,7 +497,6 @@ select * from t1;
 a
 1
 2
-5
 drop table t1;
 set @save_qcache_size=@@global.query_cache_size;
 set @save_qcache_type=@@global.query_cache_type;
@@ -617,4 +615,67 @@ 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 @a:= '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);
+ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+select @a;
+@a
+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-04 22:50:54 +04:00
@@ -597,4 +597,85 @@ 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 @a:= 'deadlock';
+  DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET @a:= 'exception';
+  INSERT INTO t2 (f2) VALUES (1);
+  DELETE FROM t2 WHERE f2 = 1;
+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;
+--error 1213
+reap;
+select @a;
+# check that the whole transaction was rolled back
+select * from t2;
+
+connection con1;
+commit;
+disconnect con1;
+disconnect con2;
+connection default;
+drop procedure proc24989;
+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-04 23:02:38 +04:00
@@ -451,25 +451,12 @@ convert_error_code_to_mysql(
     		return(-1); /* unspecified error */
 
  	} else if (error == (int) DB_DEADLOCK) {
- 		/* Since we rolled back the whole transaction, we must
- 		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);
 
  	} else if (error == (int) DB_LOCK_WAIT_TIMEOUT) {
-
-		/* Starting from 5.0.13, we let MySQL just roll back the
-		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, TRUE);
 
    		return(HA_ERR_LOCK_WAIT_TIMEOUT);
 
@@ -517,13 +504,7 @@ convert_error_code_to_mysql(
 
     		return(HA_ERR_NO_SAVEPOINT);
   	} else if (error == (int) DB_LOCK_TABLE_FULL) {
- 		/* Since we rolled back the whole transaction, we must
- 		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-04 22:58:46 +04:00
@@ -1070,6 +1070,13 @@ sp_head::execute(THD *thd)
     thd->cleanup_after_query();
     free_root(&execute_mem_root, MYF(0));    
 
+    if (!err_status && thd->is_fatal_sub_stmt_error)
+    {
+      /* Restore error flags after continue handler. */
+      err_status= TRUE;
+      thd->net.report_error= TRUE;
+      break;
+    }
     /*
       Check if an exception has occurred and a handler has been found
       Note: We have to check even if err_status == FALSE, since warnings (and
@@ -1090,13 +1097,22 @@ sp_head::execute(THD *thd)
         ctx->push_hstack(i->get_cont_dest());
         // Fall through
       default:
-	ip= hip;
-	err_status= FALSE;
+        /*
+          To allow continue handler to catch fatal errors reset error flag
+          but don't clear the error itself, we will need it to report to
+          a user.
+        */
+        ip= hip;
 	ctx->clear_handler();
 	ctx->enter_handler(hip);
-        thd->clear_error();
-	thd->killed= THD::NOT_KILLED;
-        thd->mysys_var->abort= 0;
+        err_status= FALSE;
+        thd->net.report_error= 0;
+        if (!thd->is_fatal_sub_stmt_error)
+        {
+          thd->clear_error();
+          thd->killed= THD::NOT_KILLED;
+          thd->mysys_var->abort= 0;
+        }
 	continue;
       }
     }

--- 1.44/sql/sp_rcontext.cc	2006-12-23 22:04:26 +03:00
+++ 1.45/sql/sp_rcontext.cc	2007-07-04 22:56:14 +04:00
@@ -194,8 +194,14 @@ bool
 sp_rcontext::find_handler(uint sql_errno,
                           MYSQL_ERROR::enum_warning_level level)
 {
+  THD *thd;
   if (m_hfound >= 0)
     return 1;			// Already got one
+  thd= current_thd;
+  /* Don't allow fatal sub statement errors to be caught in a sub statement */
+  if (thd->is_fatal_sub_stmt_error &&
+      thd->spcont->sp->m_type != TYPE_ENUM_PROCEDURE)
+    return FALSE;
 
   const char *sqlstate= mysql_errno_to_sqlstate(sql_errno);
   int i= m_hcount, found= -1;
@@ -313,7 +319,8 @@ sp_rcontext::handle_error(uint sql_errno
       */
       thd->net.report_error= 1;
     }
-    handled= TRUE;
+    if (!thd->is_fatal_sub_stmt_error)
+      handled= TRUE;
   }
 
   return handled;
Thread
bk commit into 5.0 tree (evgen:1.2500) BUG#24989eugene4 Jul
  • Re: bk commit into 5.0 tree (evgen:1.2500) BUG#24989Sergei Golubchik5 Jul