bk commit into 5.0 tree (dlenev:1.1954) BUG#13825

[dlenev]

Below is the list of changes that have just been committed into a local
5.0 repository of dlenev. When dlenev 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
  1.1954 05/11/04 12:38:09 dlenev@stripped +7 -0
  Fix for bug #13825 "Triggers: crash if release savepoint" and for general
  handling of savepoints in stored routines.
  
  Fixed ha_rollback_to_savepoint()/ha_savepoint()/ha_release_savepoint()
  functions to properly handle savepoints inside of stored functions and
  triggers.
  Also now when we invoke stored function or trigger we create new savepoint
  level. We destroy it at the end of function/trigger execution and return back
  to old savepoint level.

  sql/sql_parse.cc
    1.512 05/11/04 12:38:02 dlenev@stripped +2 -2
    mysql_execute_command():
      Changed processing of SQLCOM_SAVEPOINT so now it is not ignored when
      we are in autocommit mode and savepoint is set inside of stored
      function or trigger.

  sql/sql_class.h
    1.272 05/11/04 12:38:02 dlenev@stripped +1 -0
    Sub_statement_state:
      When we invoke stored function or trigger we should create new savepoint
      level. We should destroy it at the end of function/trigger execution and
      return back to old savepoint level. To support this behavior added "savepoint"
      member which is used to save/restore list of current savepoints on
      entering/leaving function.

  sql/sql_class.cc
    1.218 05/11/04 12:38:01 dlenev@stripped +3 -0
    THD::reset_sub_statement_state()/restore_sub_statement_state():
      When we invoke stored function or trigger we should create new savepoint
      level. We should destroy it at the end of function/trigger execution and
      return back to old savepoint level. To support this behavior we should
      save and reset list of current savepoints on entering function and restore
      old list when we leave it.

  sql/handler.cc
    1.201 05/11/04 12:38:01 dlenev@stripped +8 -7
    ha_rollback_to_savepoint()/ha_savepoint()/ha_release_savepoint():
      Changed functions to properly support handling of savepoints
      inside of stored functions and triggers.

  sql/ha_innodb.cc
    1.276 05/11/04 12:38:01 dlenev@stripped +7 -5
    innobase_savepoint():
      Replaced check which always failed due to similar check in caller
      with assertion.

  mysql-test/t/sp_trans.test
    1.5 05/11/04 12:38:01 dlenev@stripped +180 -0
    Added test for bug #13825 "Triggers: crash if release savepoint" and for
    general handling of savepoints in stored routines.

  mysql-test/r/sp_trans.result
    1.5 05/11/04 12:38:01 dlenev@stripped +195 -0
    Added test for bug #13825 "Triggers: crash if release savepoint" and for
    general handling of savepoints in stored routines.

# 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:	dlenev
# Host:	brandersnatch.site
# Root:	/home/dlenev/src/mysql-5.0-bg13825

--- 1.200/sql/handler.cc	2005-10-13 20:40:43 +04:00
+++ 1.201/sql/handler.cc	2005-11-04 12:38:01 +03:00
@@ -1145,10 +1145,10 @@
 int ha_rollback_to_savepoint(THD *thd, SAVEPOINT *sv)
 {
   int error=0;
-  THD_TRANS *trans=&thd->transaction.all;
+  THD_TRANS *trans= (thd->in_sub_stmt ? &thd->transaction.stmt :
+                                        &thd->transaction.all);
   handlerton **ht=trans->ht, **end_ht;
   DBUG_ENTER("ha_rollback_to_savepoint");
-  DBUG_ASSERT(thd->transaction.stmt.ht[0] == 0);
 
   trans->nht=sv->nht;
   trans->no_2pc=0;
@@ -1176,7 +1176,7 @@
   for (; *ht ; ht++)
   {
     int err;
-    if ((err= (*(*ht)->rollback)(thd, 1)))
+    if ((err= (*(*ht)->rollback)(thd, !thd->in_sub_stmt)))
     { // cannot happen
       my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err);
       error=1;
@@ -1196,10 +1196,10 @@
 int ha_savepoint(THD *thd, SAVEPOINT *sv)
 {
   int error=0;
-  THD_TRANS *trans=&thd->transaction.all;
+  THD_TRANS *trans= (thd->in_sub_stmt ? &thd->transaction.stmt :
+                                        &thd->transaction.all);
   handlerton **ht=trans->ht;
   DBUG_ENTER("ha_savepoint");
-  DBUG_ASSERT(thd->transaction.stmt.ht[0] == 0);
 #ifdef USING_TRANSACTIONS
   for (; *ht; ht++)
   {
@@ -1225,9 +1225,10 @@
 int ha_release_savepoint(THD *thd, SAVEPOINT *sv)
 {
   int error=0;
-  handlerton **ht=thd->transaction.all.ht, **end_ht;
+  THD_TRANS *trans= (thd->in_sub_stmt ? &thd->transaction.stmt :
+                                        &thd->transaction.all);
+  handlerton **ht=trans->ht, **end_ht;
   DBUG_ENTER("ha_release_savepoint");
-  DBUG_ASSERT(thd->transaction.stmt.ht[0] == 0);
 
   end_ht=ht+sv->nht;
   for (; ht < end_ht; ht++)

--- 1.217/sql/sql_class.cc	2005-10-25 13:02:41 +04:00
+++ 1.218/sql/sql_class.cc	2005-11-04 12:38:01 +03:00
@@ -1939,6 +1939,7 @@
   backup->sent_row_count=   sent_row_count;
   backup->cuted_fields=     cuted_fields;
   backup->client_capabilities= client_capabilities;
+  backup->savepoints= transaction.savepoints;
 
   if (!lex->requires_prelocking() || is_update_query(lex->sql_command))
     options&= ~OPTION_BIN_LOG;
@@ -1951,6 +1952,7 @@
   examined_row_count= 0;
   sent_row_count= 0;
   cuted_fields= 0;
+  transaction.savepoints= 0;
 
 #ifndef EMBEDDED_LIBRARY
   /* Surpress OK packets in case if we will execute statements */
@@ -1971,6 +1973,7 @@
   limit_found_rows= backup->limit_found_rows;
   sent_row_count=   backup->sent_row_count;
   client_capabilities= backup->client_capabilities;
+  transaction.savepoints= backup->savepoints;
 
   /*
     The following is added to the old values as we are interested in the

--- 1.271/sql/sql_class.h	2005-10-25 13:02:41 +04:00
+++ 1.272/sql/sql_class.h	2005-11-04 12:38:02 +03:00
@@ -1097,6 +1097,7 @@
   uint in_sub_stmt;
   bool enable_slow_log, insert_id_used;
   my_bool no_send_ok;
+  SAVEPOINT *savepoints;
 };
 
 

--- 1.511/sql/sql_parse.cc	2005-10-28 01:56:37 +04:00
+++ 1.512/sql/sql_parse.cc	2005-11-04 12:38:02 +03:00
@@ -4016,8 +4016,8 @@
     break;
   }
   case SQLCOM_SAVEPOINT:
-    if (!(thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) ||
-        !opt_using_transactions)
+    if (!(thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN) ||
+          thd->in_sub_stmt) || !opt_using_transactions)
       send_ok(thd);
     else
     {

--- 1.275/sql/ha_innodb.cc	2005-10-25 10:57:09 +04:00
+++ 1.276/sql/ha_innodb.cc	2005-11-04 12:38:01 +03:00
@@ -2178,11 +2178,13 @@
 
 	DBUG_ENTER("innobase_savepoint");
 
-	if (!(thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN))) {
-		/* In the autocommit state there is no sense to set a
-		savepoint: we return immediate success */
-	        DBUG_RETURN(0);
-	}
+        /*
+          In the autocommit mode there is no sense to set a savepoint
+          (unless we are in sub-statement), so SQL layer ensures that
+          this method is never called in such situation.
+        */
+        DBUG_ASSERT(thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN) ||
+                    thd->in_sub_stmt);
 
 	trx = check_trx_exists(thd);
 

--- 1.4/mysql-test/r/sp_trans.result	2005-06-07 18:48:25 +04:00
+++ 1.5/mysql-test/r/sp_trans.result	2005-11-04 12:38:01 +03:00
@@ -174,3 +174,198 @@
 drop procedure bug10015_8|
 drop function bug10015_7|
 drop table t1, t2|
+drop function if exists bug13825_0|
+drop function if exists bug13825_1|
+drop function if exists bug13825_2|
+drop function if exists bug13825_3|
+drop function if exists bug13825_4|
+drop function if exists bug13825_5|
+drop procedure if exists bug13825_0|
+drop procedure if exists bug13825_1|
+drop procedure if exists bug13825_2|
+drop table if exists t1|
+create table t1 (i int) engine=innodb|
+create table t2 (i int) engine=innodb|
+create function bug13825_0() returns int
+begin
+rollback to savepoint x;
+return 1;
+end|
+create function bug13825_1() returns int
+begin
+release savepoint x;
+return 1;
+end|
+create function bug13825_2() returns int
+begin
+insert into t1 values (2);
+savepoint x;
+insert into t1 values (3);
+rollback to savepoint x;
+insert into t1 values (4);
+return 1;
+end|
+create procedure bug13825_0()
+begin
+rollback to savepoint x;
+end|
+create procedure bug13825_1()
+begin
+release savepoint x;
+end|
+create procedure bug13825_2()
+begin
+savepoint x;
+end|
+insert into t2 values (1)|
+create trigger t2_bi before insert on t2 for each row
+rollback to savepoint x|
+create trigger t2_bu before update on t2 for each row
+release savepoint x|
+create trigger t2_bd before delete on t2 for each row
+begin
+insert into t1 values (2);
+savepoint x;
+insert into t1 values (3);
+rollback to savepoint x;
+insert into t1 values (4);
+end|
+create function bug13825_3(rb int) returns int
+begin
+insert into t1 values(1);
+savepoint x;
+insert into t1 values(2);
+if rb then
+rollback to savepoint x;
+end if;
+insert into t1 values(3);
+return rb;
+end|
+create function bug13825_4() returns int
+begin
+savepoint x;
+insert into t1 values(2);
+rollback to savepoint x;
+return 0;
+end|
+create function bug13825_5(p int) returns int
+begin
+savepoint x;
+insert into t2 values(p);
+rollback to savepoint x;
+insert into t2 values(p+1);
+return p;
+end|
+set autocommit= 0|
+begin |
+insert into t1 values (1)|
+savepoint x|
+set @a:= bug13825_0()|
+ERROR 42000: SAVEPOINT x does not exist
+insert into t2 values (2)|
+ERROR 42000: SAVEPOINT x does not exist
+set @a:= bug13825_1()|
+ERROR 42000: SAVEPOINT x does not exist
+update t2 set i = 2|
+ERROR 42000: SAVEPOINT x does not exist
+set @a:= bug13825_2()|
+select * from t1|
+i
+1
+2
+4
+rollback to savepoint x|
+select * from t1|
+i
+1
+delete from t2|
+select * from t1|
+i
+1
+2
+4
+rollback to savepoint x|
+select * from t1|
+i
+1
+release savepoint x|
+set @a:= bug13825_2()|
+select * from t1|
+i
+1
+2
+4
+rollback to savepoint x|
+ERROR 42000: SAVEPOINT x does not exist
+delete from t1|
+commit|
+begin|
+insert into t1 values (5)|
+savepoint x|
+insert into t1 values (6)|
+call bug13825_0()|
+select * from t1|
+i
+5
+call bug13825_1()|
+rollback to savepoint x|
+ERROR 42000: SAVEPOINT x does not exist
+savepoint x|
+insert into t1 values (7)|
+call bug13825_2()|
+rollback to savepoint x|
+select * from t1|
+i
+5
+7
+delete from t1|
+commit|
+set autocommit= 1|
+select bug13825_3(0)|
+bug13825_3(0)
+0
+select * from t1|
+i
+1
+2
+3
+delete from t1|
+select bug13825_3(1)|
+bug13825_3(1)
+1
+select * from t1|
+i
+1
+3
+delete from t1|
+set autocommit= 0|
+begin|
+insert into t1 values (1)|
+set @a:= bug13825_4()|
+select * from t1|
+i
+1
+delete from t1|
+commit|
+set autocommit= 1|
+drop table t2|
+create table t2 (i int) engine=innodb|
+insert into t1 values (1), (bug13825_5(2)), (3)|
+select * from t1|
+i
+1
+2
+3
+select * from t2|
+i
+3
+drop function bug13825_0|
+drop function bug13825_1|
+drop function bug13825_2|
+drop function bug13825_3|
+drop function bug13825_4|
+drop function bug13825_5|
+drop procedure bug13825_0|
+drop procedure bug13825_1|
+drop procedure bug13825_2|
+drop table t1, t2|

--- 1.4/mysql-test/t/sp_trans.test	2005-06-07 18:48:25 +04:00
+++ 1.5/mysql-test/t/sp_trans.test	2005-11-04 12:38:01 +03:00
@@ -176,6 +176,186 @@
 
 
 #
+# BUG#13825 "Triggers: crash if release savepoint".
+# Also general test for handling of savepoints in stored routines.
+#
+# According to SQL standard we should establish new savepoint
+# level before executing stored function/trigger and destroy 
+# this savepoint level after execution. Stored procedures by
+# default should be executed using the same savepoint level
+# as their caller (to execute stored procedure using new 
+# savepoint level one should explicitly specify NEW SAVEPOINT
+# LEVEL clause in procedure creation statement which MySQL
+# does not support yet).
+--disable_warnings
+drop function if exists bug13825_0|
+drop function if exists bug13825_1|
+drop function if exists bug13825_2|
+drop function if exists bug13825_3|
+drop function if exists bug13825_4|
+drop function if exists bug13825_5|
+drop procedure if exists bug13825_0|
+drop procedure if exists bug13825_1|
+drop procedure if exists bug13825_2|
+drop table if exists t1|
+--enable_warnings
+create table t1 (i int) engine=innodb|
+create table t2 (i int) engine=innodb|
+create function bug13825_0() returns int
+begin
+  rollback to savepoint x;
+  return 1;
+end|
+create function bug13825_1() returns int
+begin
+  release savepoint x;
+  return 1;
+end|
+create function bug13825_2() returns int
+begin
+  insert into t1 values (2);
+  savepoint x;
+  insert into t1 values (3);
+  rollback to savepoint x;
+  insert into t1 values (4);
+  return 1;
+end|
+create procedure bug13825_0()
+begin
+  rollback to savepoint x;
+end|
+create procedure bug13825_1()
+begin
+  release savepoint x;
+end|
+create procedure bug13825_2()
+begin
+  savepoint x;
+end|
+insert into t2 values (1)|
+create trigger t2_bi before insert on t2 for each row
+  rollback to savepoint x|
+create trigger t2_bu before update on t2 for each row
+  release savepoint x|
+create trigger t2_bd before delete on t2 for each row
+begin
+  insert into t1 values (2);
+  savepoint x;
+  insert into t1 values (3);
+  rollback to savepoint x;
+  insert into t1 values (4);
+end|
+create function bug13825_3(rb int) returns int
+begin
+  insert into t1 values(1);
+  savepoint x;
+  insert into t1 values(2);
+  if rb then
+    rollback to savepoint x;
+  end if;
+  insert into t1 values(3);
+  return rb;
+end|
+create function bug13825_4() returns int
+begin
+  savepoint x;
+  insert into t1 values(2);
+  rollback to savepoint x;
+  return 0;
+end|
+create function bug13825_5(p int) returns int
+begin
+  savepoint x;
+  insert into t2 values(p);
+  rollback to savepoint x;
+  insert into t2 values(p+1);
+  return p;
+end|
+set autocommit= 0|
+# Test of savepoint level handling for stored functions and triggers
+begin |
+insert into t1 values (1)|
+savepoint x|
+--error ER_SP_DOES_NOT_EXIST
+set @a:= bug13825_0()|
+--error ER_SP_DOES_NOT_EXIST
+insert into t2 values (2)|
+--error ER_SP_DOES_NOT_EXIST
+set @a:= bug13825_1()|
+--error ER_SP_DOES_NOT_EXIST
+update t2 set i = 2|
+set @a:= bug13825_2()|
+select * from t1|
+rollback to savepoint x|
+select * from t1|
+delete from t2|
+select * from t1|
+rollback to savepoint x|
+select * from t1|
+# Of course savepoints set in function should not be visible from its caller
+release savepoint x|
+set @a:= bug13825_2()|
+select * from t1|
+--error ER_SP_DOES_NOT_EXIST
+rollback to savepoint x|
+delete from t1|
+commit|
+# Test of savepoint level handling for stored procedures
+begin|
+insert into t1 values (5)|
+savepoint x|
+insert into t1 values (6)|
+call bug13825_0()|
+select * from t1|
+call bug13825_1()|
+--error ER_SP_DOES_NOT_EXIST
+rollback to savepoint x|
+savepoint x|
+insert into t1 values (7)|
+call bug13825_2()|
+rollback to savepoint x|
+select * from t1|
+delete from t1|
+commit|
+set autocommit= 1|
+# Let us test that savepoints work inside of functions
+# even in auto-commit mode
+select bug13825_3(0)|
+select * from t1|
+delete from t1|
+select bug13825_3(1)|
+select * from t1|
+delete from t1|
+# Curious case: rolling back to savepoint which is set by first
+# statement in function should not rollback whole transaction.
+set autocommit= 0|
+begin|
+insert into t1 values (1)|
+set @a:= bug13825_4()|
+select * from t1|
+delete from t1|
+commit|
+set autocommit= 1|
+# Other curious case: savepoint in the middle of statement
+drop table t2|
+create table t2 (i int) engine=innodb|
+insert into t1 values (1), (bug13825_5(2)), (3)|
+select * from t1|
+select * from t2|
+# Cleanup
+drop function bug13825_0|
+drop function bug13825_1|
+drop function bug13825_2|
+drop function bug13825_3|
+drop function bug13825_4|
+drop function bug13825_5|
+drop procedure bug13825_0|
+drop procedure bug13825_1|
+drop procedure bug13825_2|
+drop table t1, t2|
+
+
+#
 # BUG#NNNN: New bug synopsis
 #
 #--disable_warnings