Fix for bug #10015 "Crash in InnoDB if stored routines are used".

We should not allow explicit or implicit transaction commits inside
of stored functions or triggers (so in autocommit mode we should not
do commits after execution of sub-statement).
Also since we don't support nested statement transactions in 5.0,
we shouldn't commit or rollback stmt transactions while we are inside
stored functions or triggers. This should be fixed in later (>=5.1)
releases.


mysql-test/r/sp_trans.result:
  Added test for bug #10015 "Crash in InnoDB if stored routines are used"
  and for general transaction handling inside of functions.
mysql-test/t/sp_trans.test:
  Added test for bug #10015 "Crash in InnoDB if stored routines are used"
  and for general transaction handling inside of functions.
sql/handler.cc:
  ha_trans_commit()/ha_trans_rollback():
    Since we don't support nested statement transactions in 5.0,
    we can't commit or rollback stmt transactions while we are inside
    stored functions or triggers. So we simply do nothing now.
    This should be fixed in later ( >= 5.1) releases.
sql/item_func.cc:
  Item_func_sp::execute():
    Set THD::transaction.in_sub_stmt flag to TRUE during stored function
    execution to prevent commits and rollbacks for statement level
    transactions, since doing them will ruin such transaction for
    stateemtn which calls this function.
sql/share/errmsg.txt:
  Added error message which says that statements doing explicit or implicit
  commits are disallowed in triggers and stored functions.
sql/sql_base.cc:
  close_thread_tables():
    Clarified comment about committing of statement transactions in
    prelocked mode.
sql/sql_class.h:
  THD::transaction:
    Added in_sub_stmt method which indicates that we are executing
    statements from trigger or stored function now, and thus
    statement transaction belongs to statement which invoked this
    routine and we should not commit or rollback it while executing
    these sub-statements.
sql/sql_parse.cc:
  end_active_trans()/begin_trans()/end_trans():
    We should not commit or rollback global (non-stmt) transaction
    if we are executing stored function or trigger. These checks will
    catch situation when we are trying to do commit or rollback in stored
    procedure which is called from function or trigger.
sql/sql_trigger.h:
  Table_triggers_list::process_triggers():
    Set THD::transaction.in_sub_stmt flag to TRUE during trigger
    execution to prevent commits and rollbacks for statement level
    transactions, since doing them will ruin such transaction for
    stateemtn which invokes this trigger.
sql/sql_yacc.yy:
  Prohibited usage of statements which do explicit or implicit commit or
  rollback inside of stored functions and triggers.

mysql-test/r/sp_trans.result

+drop table if exists t1, t2;
 drop procedure if exists bug8850|
 create table t1 (a int) engine=innodb|
 create procedure bug8850()
@@ -20,3 +21,160 @@ a
 2
 drop table t1|
 drop procedure bug8850|
+drop function if exists bug10015_1|
+drop function if exists bug10015_2|
+drop function if exists bug10015_3|
+drop function if exists bug10015_4|
+drop function if exists bug10015_5|
+drop function if exists bug10015_6|
+drop function if exists bug10015_7|
+drop procedure if exists bug10015_8|
+create table t1 (id int) engine=innodb|
+create table t2 (id int primary key, j int) engine=innodb|
+insert into t1 values (1),(2),(3)|
+create function bug10015_1() returns int return (select count(*) from t1)|
+select *, bug10015_1() from t1|
+id	bug10015_1()
+1	3
+2	3
+3	3
+drop function bug10015_1|
+create function bug10015_2() returns int 
+begin
+declare i, s int;
+set i:= (select min(id) from t1);
+set s:= (select max(id) from t1);
+return (s - i);
+end|
+select *, bug10015_2() from t1|
+id	bug10015_2()
+1	2
+2	2
+3	2
+drop function bug10015_2|
+create function bug10015_3() returns int 
+return (select max(a.id - b.id) from t1 as a, t1 as b where a.id >= b.id)|
+select *, bug10015_3() from t1|
+id	bug10015_3()
+1	2
+2	2
+3	2
+drop function bug10015_3|
+create function bug10015_4(i int) returns int 
+begin
+declare m int;
+set m:= (select max(id) from t2);
+insert into t2 values (i, m);
+return m;
+end|
+select *, bug10015_4(id) from t1|
+id	bug10015_4(id)
+1	NULL
+2	1
+3	2
+select * from t2|
+id	j
+1	NULL
+2	1
+3	2
+drop function bug10015_4|
+create function bug10015_5(i int) returns int
+begin
+if (i = 5) then
+insert into t2 values (1, 0);
+end if;
+return i;
+end|
+insert into t1 values (bug10015_5(4)), (bug10015_5(5))|
+ERROR 23000: Duplicate entry '1' for key 1
+select * from t1|
+id
+1
+2
+3
+drop function bug10015_5|
+create function bug10015_6(i int) returns int
+begin
+declare continue handler for sqlexception set @error_in_func:= 1;
+if (i = 5) then
+insert into t2 values (4, 0), (1, 0);
+end if;
+return i;
+end|
+set @error_in_func:= 0|
+insert into t1 values (bug10015_6(5)), (bug10015_6(6))|
+select @error_in_func|
+@error_in_func
+1
+select * from t1|
+id
+1
+2
+3
+5
+6
+select * from t2|
+id	j
+1	NULL
+2	1
+3	2
+4	0
+drop function bug10015_6|
+create function bug10015_7() returns int
+begin
+alter table t1 add k int;
+return 1;
+end|
+ERROR HY000: Explicit or implicit commit is not allowed in stored function or trigger.
+create function bug10015_7() returns int
+begin
+start transaction;
+return 1;
+end|
+ERROR HY000: Explicit or implicit commit is not allowed in stored function or trigger.
+create function bug10015_7() returns int
+begin
+drop table t1;
+return 1;
+end|
+ERROR HY000: Explicit or implicit commit is not allowed in stored function or trigger.
+create function bug10015_7() returns int
+begin
+drop temporary table t1;
+return 1;
+end|
+drop function bug10015_7|
+create function bug10015_7() returns int
+begin
+commit;
+return 1;
+end|
+ERROR HY000: Explicit or implicit commit is not allowed in stored function or trigger.
+create function bug10015_7() returns int
+begin
+call bug10015_8();
+return 1;
+end|
+create procedure bug10015_8() alter table t1 add k int|
+select *, bug10015_7() from t1|
+ERROR HY000: Explicit or implicit commit is not allowed in stored function or trigger.
+drop procedure bug10015_8|
+create procedure bug10015_8() start transaction|
+select *, bug10015_7() from t1|
+ERROR HY000: Explicit or implicit commit is not allowed in stored function or trigger.
+drop procedure bug10015_8|
+create procedure bug10015_8() drop temporary table if exists t1_temp|
+select *, bug10015_7() from t1|
+id	bug10015_7()
+1	1
+2	1
+3	1
+5	1
+6	1
+drop procedure bug10015_8|
+create procedure bug10015_8() commit|
+select *, bug10015_7() from t1|
+ERROR HY000: Explicit or implicit commit is not allowed in stored function or trigger.
+drop procedure bug10015_8|
+drop function bug10015_7|
+drop table t1, t2|

mysql-test/t/sp_trans.test

@@ -4,6 +4,10 @@
 
 -- source include/have_innodb.inc
 
+--disable_warnings
+drop table if exists t1, t2;
+--enable_warnings
+
 delimiter |;
 
 #
@@ -35,6 +39,146 @@ drop table t1|
 drop procedure bug8850|
 
 
+#
+# BUG#10015: Crash in InnoDB if stored routines are used
+# (crash happens in auto-commit mode)
+#
+--disable_warnings
+drop function if exists bug10015_1|
+drop function if exists bug10015_2|
+drop function if exists bug10015_3|
+drop function if exists bug10015_4|
+drop function if exists bug10015_5|
+drop function if exists bug10015_6|
+drop function if exists bug10015_7|
+drop procedure if exists bug10015_8|
+--enable_warnings
+create table t1 (id int) engine=innodb|
+create table t2 (id int primary key, j int) engine=innodb|
+insert into t1 values (1),(2),(3)|
+create function bug10015_1() returns int return (select count(*) from t1)|
+select *, bug10015_1() from t1|
+drop function bug10015_1|
+# Test couple of a bit more complex cases
+create function bug10015_2() returns int 
+  begin
+    declare i, s int;
+    set i:= (select min(id) from t1);
+    set s:= (select max(id) from t1);
+    return (s - i);
+  end|
+select *, bug10015_2() from t1|
+drop function bug10015_2|
+create function bug10015_3() returns int 
+  return (select max(a.id - b.id) from t1 as a, t1 as b where a.id >= b.id)|
+select *, bug10015_3() from t1|
+drop function bug10015_3|
+create function bug10015_4(i int) returns int 
+  begin
+    declare m int;
+    set m:= (select max(id) from t2);
+    insert into t2 values (i, m);
+    return m;
+  end|
+select *, bug10015_4(id) from t1|
+select * from t2|
+drop function bug10015_4|
+# Now let us test how statement rollback works
+# This function will cause the whole stmt to be rolled back,
+# there should not be any traces left.
+create function bug10015_5(i int) returns int
+  begin
+    if (i = 5) then
+      insert into t2 values (1, 0);
+    end if;
+    return i;
+  end|
+--error 1062
+insert into t1 values (bug10015_5(4)), (bug10015_5(5))|
+select * from t1|
+drop function bug10015_5|
+# Thanks to error-handler this function should not cause rollback
+# of statement calling it. But insert statement in it should be 
+# rolled back completely and don't leave any traces in t2.
+# Unfortunately we can't implement such behavior in 5.0, so it
+# is something to be fixed in later 5.* releases (TODO).
+create function bug10015_6(i int) returns int
+  begin
+    declare continue handler for sqlexception set @error_in_func:= 1;
+    if (i = 5) then
+      insert into t2 values (4, 0), (1, 0);
+    end if;
+    return i;
+  end|
+set @error_in_func:= 0|
+insert into t1 values (bug10015_6(5)), (bug10015_6(6))|
+select @error_in_func|
+select * from t1|
+select * from t2|
+drop function bug10015_6|
+# Let us test that we don't allow any statements causing transaction
+# commit in stored functions (we test only most interesting cases here).
+# Cases which can be caught at creation time:
+--error 1422
+create function bug10015_7() returns int
+  begin
+    alter table t1 add k int;
+    return 1;
+  end|
+--error 1422
+create function bug10015_7() returns int
+  begin
+    start transaction;
+    return 1;
+  end|
+--error 1422
+create function bug10015_7() returns int
+  begin
+    drop table t1;
+    return 1;
+  end|
+# It should be OK to drop temporary table.
+create function bug10015_7() returns int
+  begin
+    drop temporary table t1;
+    return 1;
+  end|
+drop function bug10015_7|
+--error 1422
+create function bug10015_7() returns int
+  begin
+    commit;
+    return 1;
+  end|
+# Now let us test cases which we can catch only at run-time:
+create function bug10015_7() returns int
+  begin
+    call bug10015_8();
+    return 1;
+  end|
+create procedure bug10015_8() alter table t1 add k int|
+--error 1422
+select *, bug10015_7() from t1|
+drop procedure bug10015_8|
+create procedure bug10015_8() start transaction|
+--error 1422
+select *, bug10015_7() from t1|
+drop procedure bug10015_8|
+# Again it is OK to drop temporary table
+# We are surpressing warnings since they are not essential
+create procedure bug10015_8() drop temporary table if exists t1_temp|
+--disable_warnings
+select *, bug10015_7() from t1|
+--enable_warnings
+drop procedure bug10015_8|
+create procedure bug10015_8() commit|
+--error 1422
+select *, bug10015_7() from t1|
+drop procedure bug10015_8|
+drop function bug10015_7|
+drop table t1, t2|
+
+
 #
 # BUG#NNNN: New bug synopsis
 #

sql/handler.cc

@@ -595,6 +595,27 @@ int ha_commit_trans(THD *thd, bool all)
   handlerton **ht= trans->ht;
   my_xid xid= thd->transaction.xid.get_my_xid();
   DBUG_ENTER("ha_commit_trans");
+
+  if (thd->transaction.in_sub_stmt)
+  {
+    /*
+      Since we don't support nested statement transactions in 5.0,
+      we can't commit or rollback stmt transactions while we are inside
+      stored functions or triggers. So we simply do nothing now.
+      TODO: This should be fixed in later ( >= 5.1) releases.
+    */
+    if (!all)
+      DBUG_RETURN(0);
+    /*
+      We assume that all statements which commit or rollback main transaction
+      are prohibited inside of stored functions or triggers. So they should
+      bail out with error even before ha_commit_trans() call. To be 100% safe
+      let us throw error in non-debug builds.
+    */
+    DBUG_ASSERT(0);
+    my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+    DBUG_RETURN(2);
+  }
 #ifdef USING_TRANSACTIONS
   if (trans->nht)
   {
@@ -689,6 +710,19 @@ int ha_rollback_trans(THD *thd, bool all)
   THD_TRANS *trans=all ? &thd->transaction.all : &thd->transaction.stmt;
   bool is_real_trans=all || thd->transaction.all.nht == 0;
   DBUG_ENTER("ha_rollback_trans");
+  if (thd->transaction.in_sub_stmt)
+  {
+    /*
+      If we are inside stored function or trigger we should not commit or
+      rollback current statement transaction. See comment in ha_commit_trans()
+      call for more information.
+    */
+    if (!all)
+      DBUG_RETURN(0);
+    DBUG_ASSERT(0);
+    my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+    DBUG_RETURN(1);
+  }
 #ifdef USING_TRANSACTIONS
   if (trans->nht)
   {

sql/item_func.cc

@@ -4757,6 +4757,7 @@ Item_func_sp::execute(Item **itp)
   THD *thd= current_thd;
   ulong old_client_capabilites;
   int res;
+  bool save_in_sub_stmt= thd->transaction.in_sub_stmt;
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
   st_sp_security_context save_ctx;
 #endif
@@ -4799,9 +4800,11 @@ Item_func_sp::execute(Item **itp)
     problem).
   */
   tmp_disable_binlog(thd); /* don't binlog the substatements */
+  thd->transaction.in_sub_stmt= TRUE;
 
   res= m_sp->execute_function(thd, args, arg_count, itp);
 
+  thd->transaction.in_sub_stmt= save_in_sub_stmt;
   reenable_binlog(thd);
   if (res && mysql_bin_log.is_open() &&
       (m_sp->m_chistics->daccess == SP_CONTAINS_SQL ||

sql/share/errmsg.txt

@@ -5356,3 +5356,5 @@ ER_EXEC_STMT_WITH_OPEN_CURSOR
 	eng "You can't execute a prepared statement which has an open cursor associated with it. Reset the statement to re-execute it."
 ER_STMT_HAS_NO_OPEN_CURSOR
 	eng "The statement (%d) has no open cursor."
+ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG
+        eng "Explicit or implicit commit is not allowed in stored function or trigger."

sql/sql_base.cc

@@ -446,8 +446,12 @@ void close_thread_tables(THD *thd, bool lock_in_use, bool skip_derived,
   if (thd->locked_tables || prelocked_mode)
   {
     /*
-      TODO: It is not 100% clear whenever we should do ha_commit_stmt() for
-            sub-statements. This issue needs additional investigation.
+      Let us commit transaction for statement. Since in 5.0 we only have
+      one statement transaction and don't allow several nested statement
+      transactions this call will do nothing if we are inside of stored
+      function or trigger (i.e. statement transaction is already active and
+      does not belong to statement for which we do close_thread_tables()).
+      TODO: This should be fixed in later releases.
     */
     ha_commit_stmt(thd);
 

sql/sql_class.h

@@ -1065,6 +1065,8 @@ class THD :public ilink,
     THD_TRANS all;			// Trans since BEGIN WORK
     THD_TRANS stmt;			// Trans for current statement
     bool on;                            // see ha_enable_transaction()
+    /* TRUE if we are inside of trigger or stored function. */
+    bool in_sub_stmt;
     XID  xid;                           // transaction identifier
     enum xa_states xa_state;            // used by external XA only
     /*

sql/sql_parse.cc

@@ -125,6 +125,11 @@ static bool end_active_trans(THD *thd)
 {
   int error=0;
   DBUG_ENTER("end_active_trans");
+  if (unlikely(thd->transaction.in_sub_stmt))
+  {
+    my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+    DBUG_RETURN(1);
+  }
   if (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN |
 		      OPTION_TABLE_LOCK))
   {
@@ -143,6 +148,15 @@ static bool end_active_trans(THD *thd)
 static bool begin_trans(THD *thd)
 {
   int error=0;
+  /*
+    QQ: May be it is better to simply prohibit COMMIT and ROLLBACK in
+        stored routines as SQL2003 suggests?
+  */
+  if (unlikely(thd->transaction.in_sub_stmt))
+  {
+    my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+    return 1;
+  }
   if (thd->locked_tables)
   {
     thd->lock=thd->locked_tables;
@@ -1338,6 +1352,15 @@ int end_trans(THD *thd, enum enum_mysql_completiontype completion)
   int res= 0;
   DBUG_ENTER("end_trans");
 
+  /*
+    QQ: May be it is better to simply prohibit COMMIT and ROLLBACK in
+        stored routines as SQL2003 suggests?
+  */
+  if (unlikely(thd->transaction.in_sub_stmt))
+  {
+    my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+    DBUG_RETURN(1);
+  }
   switch (completion) {
   case COMMIT:
     /*

sql/sql_trigger.h

@@ -53,6 +53,7 @@ class Table_triggers_list: public Sql_alloc
 
     if (bodies[event][time_type])
     {
+      bool save_in_sub_stmt= thd->transaction.in_sub_stmt;
 #ifndef EMBEDDED_LIBRARY
       /* Surpress OK packets in case if we will execute statements */
       my_bool nsok= thd->net.no_send_ok;
@@ -81,7 +82,11 @@ class Table_triggers_list: public Sql_alloc
         does NOT go into binlog.
       */
       tmp_disable_binlog(thd);
+      thd->transaction.in_sub_stmt= TRUE;
+
       res= bodies[event][time_type]->execute_function(thd, 0, 0, 0);
+
+      thd->transaction.in_sub_stmt= save_in_sub_stmt;
       reenable_binlog(thd);
 
 #ifndef EMBEDDED_LIBRARY

sql/sql_yacc.yy

@@ -1138,6 +1138,11 @@ create:
 	| CREATE opt_unique_or_fulltext INDEX_SYM ident key_alg ON table_ident
 	  {
 	    LEX *lex=Lex;
+            if (lex->sphead && lex->sphead->m_type != TYPE_ENUM_PROCEDURE)
+            {
+              my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+              YYABORT;
+            }
 	    lex->sql_command= SQLCOM_CREATE_INDEX;
 	    if (!lex->current_select->add_table_to_list(lex->thd, $7, NULL,
 							TL_OPTION_UPDATING))
@@ -3285,6 +3290,11 @@ alter:
 	{
 	  THD *thd= YYTHD;
 	  LEX *lex= thd->lex;
+          if (lex->sphead && lex->sphead->m_type != TYPE_ENUM_PROCEDURE)
+          {
+            my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+            YYABORT;
+          }
 	  lex->sql_command= SQLCOM_ALTER_TABLE;
 	  lex->name= 0;
 	  lex->duplicates= DUP_ERROR; 
@@ -3593,8 +3603,14 @@ slave:
 start:
 	START_SYM TRANSACTION_SYM start_transaction_opts
         {
-           Lex->sql_command = SQLCOM_BEGIN;
-           Lex->start_transaction_opt= $3;
+          LEX *lex= Lex;
+          if (lex->sphead && lex->sphead->m_type != TYPE_ENUM_PROCEDURE)
+          {
+            my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+            YYABORT;
+          }
+          lex->sql_command= SQLCOM_BEGIN;
+          lex->start_transaction_opt= $3;
         }
 	;
 
@@ -3772,7 +3788,13 @@ opt_no_write_to_binlog:
 rename:
 	RENAME table_or_tables
 	{
-	   Lex->sql_command=SQLCOM_RENAME_TABLE;
+          LEX *lex= Lex;
+          if (lex->sphead && lex->sphead->m_type != TYPE_ENUM_PROCEDURE)
+          {
+            my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+            YYABORT;
+          }
+          lex->sql_command=SQLCOM_RENAME_TABLE;
 	}
 	table_to_table_list
 	{}
@@ -5740,10 +5762,21 @@ drop:
 	  lex->sql_command = SQLCOM_DROP_TABLE;
 	  lex->drop_temporary= $2;
 	  lex->drop_if_exists= $4;
+          if (!lex->drop_temporary && lex->sphead &&
+              lex->sphead->m_type != TYPE_ENUM_PROCEDURE)
+          {
+            my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+            YYABORT;
+          }
 	}
 	| DROP INDEX_SYM ident ON table_ident {}
 	  {
 	     LEX *lex=Lex;
+             if (lex->sphead && lex->sphead->m_type != TYPE_ENUM_PROCEDURE)
+             {
+               my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+               YYABORT;
+             }
 	     lex->sql_command= SQLCOM_DROP_INDEX;
 	     lex->alter_info.drop_list.empty();
 	     lex->alter_info.drop_list.push_back(new Alter_drop(Alter_drop::KEY,
@@ -5791,6 +5824,11 @@ drop:
 	  {
 	    THD *thd= YYTHD;
 	    LEX *lex= thd->lex;
+            if (lex->sphead && lex->sphead->m_type != TYPE_ENUM_PROCEDURE)
+            {
+              my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+              YYABORT;
+            }
 	    lex->sql_command= SQLCOM_DROP_VIEW;
 	    lex->drop_if_exists= $3;
 	  }
@@ -8323,6 +8361,11 @@ begin:
 	BEGIN_SYM  
         {
 	  LEX *lex=Lex;
+          if (lex->sphead && lex->sphead->m_type != TYPE_ENUM_PROCEDURE)
+          {
+            my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+            YYABORT;
+          }
           lex->sql_command = SQLCOM_BEGIN;
           lex->start_transaction_opt= 0;
         }
@@ -8355,6 +8398,11 @@ commit:
 	COMMIT_SYM opt_work opt_chain opt_release
 	{
 	  LEX *lex=Lex;
+          if (lex->sphead && lex->sphead->m_type != TYPE_ENUM_PROCEDURE)
+          {
+            my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+            YYABORT;
+          }
 	  lex->sql_command= SQLCOM_COMMIT;
 	  lex->tx_chain= $3; 
 	  lex->tx_release= $4;
@@ -8365,6 +8413,11 @@ rollback:
 	ROLLBACK_SYM opt_work opt_chain opt_release
 	{
 	  LEX *lex=Lex;
+          if (lex->sphead && lex->sphead->m_type != TYPE_ENUM_PROCEDURE)
+          {
+            my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
+            YYABORT;
+          }
 	  lex->sql_command= SQLCOM_ROLLBACK;
 	  lex->tx_chain= $3; 
 	  lex->tx_release= $4;