Re: bk commit into 5.0 tree (evgen:1.2500) BUG#24989

[Sergei Golubchik]

Hi!

On Jul 10, eugene@stripped wrote:
> ChangeSet@stripped, 2007-07-10 00:48:08+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 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 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.

> --- 1.621/sql/sql_parse.cc	2007-05-15 13:56:04 +04:00
> +++ 1.622/sql/sql_parse.cc	2007-07-09 23:54:20 +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->in_sub_stmt)
> +    thd->is_fatal_sub_stmt_error= FALSE;

No, this is wrong.
Why to check thd->spcont after every statement ?
It can never change here. It changes inside the statement and is
restored immediately after executing the routine - in
sp_head::execute_function and sp_head::execute_trigger.

Also, note that it's also set and reset in sp_head::execute_procedure,
but in the precedure we're *not* in the sub-statement, even if
thd->spcont is set.

You should not need to check thd->spcont. The flag you want to reset is
about *sub_stmt*, so you need to check thd->in_sub_stmt. And do it where
it's changed (THD::restore_sub_statement_state), not after every
statement.

> --- 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-09 23:52:27 +04:00
.....
> +select @a,@b;
> +@a	@b
> +exception	deadlock

Why both handlers were triggered ?
As far as I understand, one event should never trigger two handlers.

Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Radlkoferstr. 2, D-81373 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140