List:Commits« Previous MessageNext Message »
From:Konstantin Osipov Date:May 8 2008 10:09am
Subject:Re: bk commit into 6.0 tree (davi:1.2628) BUG#23032
View as plain text  
* Davi Arnaut <davi@stripped> [08/04/24 03:20]:
> 
> ChangeSet@stripped, 2008-04-23 19:52:42-03:00, davi@stripped +22 -0
>   Bug#23032: Handlers declared in a SP do not handle warnings generated in sub-SP
>   
>   The problem is that warnings generated by a statement executed
>   inside a stored procedure should only invoke handlers in the
>   calling context, unless the warning is generated by the last
>   statement of the stored procedure.

The problem was that a warning generated by a statement executed
inside a stored procedure could not be handled by continue/exit
handlers declared in the outer stored procedure.

According to the standard this should be possible, but only 
when the statement that generated the warning was the last statement
of the procedure.


>   The solution is to prepare the environment before the execution
>   of procedure statements and tightly control if warning were generated
>   during the execution of the statement. Also handlers now are only
>   called for handling warnings generated in the calling context of the
>   executed statement, unless it's the last statement of the stored
>   procedure.
>   
>   This is a incompatible change, now statements executed inside stored
>   procedure fallow the same rules as non-sp statements with respect to

follow 
>   effects on the message list (refer to SHOW WARNINGS manual entry for
>   more details).


Please explain in a greater detail the nature of the incompatible
change:

Before this fix, an attempt to find a continue/exit handler for a
warning or error was performed at the time when the warning
occurred.

According to the standard, and thus after this fix, this is done
after the statement has completed.

The practical consequence of the change is reported in Bug#36185:
if a statement generates at first a warning, and then an error, 
the server used to invoke a continue handler for the warning, but
not for the error. Whereas according to the standard, the error
"trumps" warnings when searching for a continue handler.

Before this fix, the rules for MySQL waring/error list life cycle
(http://dev.mysql.com/doc/refman/5.1/en/show-warnings.html) did
not apply to statements inside stored programs: 
 - the manual says that the list of warnings is cleared by a
   statement that uses a table (any table). However, such statement,
   if run inside a stored program did not clear warnings.
 - the manual says that the list of warnings is cleared by a
   statement that generates a new error or a warning - this 
   was not the case with stored program statements either, and
   is changed to be the case as well.

In other words, after this fix, a statement has the same effect on
the warning list regardless of whether it's executed inside a
stored program/sub-statement or not.

This introduces an incompatible change and a bug (reported in
XXXXX -- please report a bug add bug id here please):
 - before this fix, a, e.g. statement inside a trigger could never
   clear the global warning list 
 - after this fix, a trigger that generates a warning or uses a
   table, clears the global warning list

This change is not backward compatible, and in scope of Bug#XXXXX
will be fixed to make MySQL behaviour fully follow the standard:

A stored function or trigger will get its own "warning area"
(or, in standard terminology, diagnostics area).
At the beginning of the stored function or trigger, all
warnings from the caller area will be copied to the warning area
of the trigger. During execution, the warnings will be cleared
according to the MySQL rules described at
http://dev.mysql.com/doc/refman/5.1/en/show-warnings.html. At the
end of the function/trigger, the "warning area" (in standard
terminology, the diagnostics area) will be destroyed, along with
all warnings it contains. 

Consequently, statements that use a table or generate a warning
*will* clear warnings inside the trigger, but that will have no
effect to the warning list of the calling (outer) statement.

In fact, this remaining change is so serious that I don't think
you should merge the patch into the main tree without having it
done.


>  CALL p1('alpha', 'abcdef');
>  x	y
>  alpha	abc
> -Warnings:
> -Warning	1265	Data truncated for column 'y' at row 1
>  DROP PROCEDURE p1;

Was there a reason to not modify this test to continue printing
out the warning as before (e.g. by adding SHOW WARNINGS)?
Please update the test to output the warning as before (applies to
the rest of sp-vars.test as well).

How do continue handlers work now for DECLARE ... DEFAULT
constructs? Are they invoked? Please add a test case, and if they
are not invoked, report a bug about it, as suggested by PeterG.

>  ---------------------------------------------------------------
> @@ -697,8 +700,6 @@ Table	Create Table
>  t1	CREATE TABLE "t1" (
>    "x" datetime DEFAULT NULL
>  )
> -Warnings:
> -Warning	1264	Out of range value for column 'x' at row 1
>  DROP PROCEDURE p1;
>  
>  ---------------------------------------------------------------
> @@ -887,8 +888,6 @@ var
>  CALL p1(1929.003);
>  var
>  1929.00
> -Warnings:
> -Note	1265	Data truncated for column 'arg' at row 1
>  DROP PROCEDURE p1;
>  
>  ---------------------------------------------------------------
> @@ -903,8 +902,6 @@ END|
>  SELECT f1(-2500);
>  f1(-2500)
>  0
> -Warnings:
> -Warning	1264	Out of range value for column 'arg' at row 1
>  SET @@sql_mode = 'traditional';
>  SELECT f1(-2500);
>  ERROR 22003: Out of range value for column 'arg' at row 1
> @@ -930,8 +927,6 @@ END|
>  SELECT f1(8388699);
>  f1(8388699)
>  8388607
> -Warnings:
> -Warning	1264	Out of range value for column 'arg' at row 1
>  SET @@sql_mode = 'traditional';
>  SELECT f1(8388699);
>  ERROR 22003: Out of range value for column 'arg' at row 1
> @@ -966,8 +961,6 @@ sp_var
>  0
>  @user_var
>  0
> -Warnings:
> -Warning	1366	Incorrect integer value: 'Hello, world!' for column 'sp_var' at row 1
>  DROP PROCEDURE p1;
>  DROP TABLE t1;
>  
> @@ -1020,13 +1013,9 @@ END|
>  CALL p1('c');
>  arg
>  
> -Warnings:
> -Warning	1265	Data truncated for column 'arg' at row 1
>  CALL p2('a');
>  arg	var
>  a	
> -Warnings:
> -Warning	1265	Data truncated for column 'var' at row 1
>  SELECT f1('a');
>  f1('a')

>  
> diff -Nrup a/mysql-test/r/sp.result b/mysql-test/r/sp.result
> --- a/mysql-test/r/sp.result	2008-03-27 06:56:01 -03:00
> +++ b/mysql-test/r/sp.result	2008-04-23 19:52:36 -03:00
> @@ -526,8 +526,6 @@ end|
>  delete from t1|
>  create table t3 ( s char(16), d int)|
>  call into_test4()|
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed

OK, the reason of the change is understood.
Perhaps SHOW WARNINGS between SELECT and INSERT
could be added.

>  select * from t3|
>  s	d
>  into4	NULL
> @@ -1120,8 +1118,6 @@ end|
>  select f9()|
>  f9()
>  6
> -Warnings:
> -Note	1051	Unknown table 't3'

In the test case there is a comment:

# This will emit warning as t3 was not existing before.

This comment is no longer true. Please remove it.


>  select f9() from t1 limit 1|
>  f9()
>  6
> @@ -1162,8 +1158,6 @@ drop temporary table t3|
>  select f12_1()|
>  f12_1()
>  3
> -Warnings:
> -Note	1051	Unknown table 't3'

OK.

>  select f12_1() from t1 limit 1|
>  f12_1()
>  3
> @@ -2115,12 +2109,7 @@ end if;
>  insert into t4 values (2, rc, t3);
>  end|
>  call bug1863(10)|
> -Warnings:
> -Note	1051	Unknown table 'temp_t1'
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  call bug1863(10)|
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  select * from t4|
>  f1	rc	t3
>  2	0	NULL
> @@ -2385,11 +2374,7 @@ begin
>  end|
>  call bug4579_1()|
>  call bug4579_1()|
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  call bug4579_1()|
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  drop procedure bug4579_1|
>  drop procedure bug4579_2|
>  drop table t3|
> @@ -2808,16 +2793,12 @@ var
>  call bug7743("OneWord")|
>  var
>  NULL
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  call bug7743("anotherword")|
>  var
>  2
>  call bug7743("AnotherWord")|
>  var
>  NULL
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed

OK.

>  drop procedure bug7743|
>  drop table t4|
>  delete from t3|
> @@ -3234,11 +3215,7 @@ end|
>  create procedure bug9004_2(x char(16))
>  call bug9004_1(x)|
>  call bug9004_1('12345678901234567')|
> -Warnings:
> -Warning	1265	Data truncated for column 'x' at row 1
>  call bug9004_2('12345678901234567890')|
> -Warnings:
> -Warning	1265	Data truncated for column 'x' at row 1

This test case tests that there will be a warning.
Now the warning is lost.  Please rewrite the test case to test the
intended effects of truncation.


>  delete from t1|
>  drop procedure bug9004_1|
>  drop procedure bug9004_2|
> @@ -3841,9 +3818,6 @@ Table	Create Table
>  tm1	CREATE TEMPORARY TABLE `tm1` (
>    `spv1` decimal(3,3) DEFAULT NULL
>  ) ENGINE=MyISAM DEFAULT CHARSET=latin1
> -Warnings:
> -Warning	1264	Out of range value for column 'spv1' at row 1
> -Warning	1366	Incorrect decimal value: 'test' for column 'spv1' at row 1

OK.

>  call bug12589_2()|
>  Table	Create Table
>  tm1	CREATE TEMPORARY TABLE `tm1` (
> @@ -4682,13 +4656,9 @@ Before NOT FOUND condition is triggered
>  After NOT FOUND condtition is triggered
>  xid	xdone
>  1	0
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  call bug15231_3()|

OK.
>  Result
>  Missed it (correct)
> -Warnings:
> -Warning	1366	Incorrect decimal value: 'zap' for column 'x' at row 1

OK.

>  drop table if exists t3|
>  drop procedure if exists bug15231_1|
>  drop procedure if exists bug15231_2|
> @@ -5501,13 +5471,9 @@ end|
>  select func_20028_a()|
>  func_20028_a()
>  0
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  select func_20028_b()|
>  func_20028_b()
>  0
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  select func_20028_c()|
>  ERROR 22012: Division by 0
>  call proc_20028_a()|
> @@ -5560,13 +5526,9 @@ end|
>  select func_20028_a()|
>  func_20028_a()
>  0
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  select func_20028_b()|
>  func_20028_b()
>  0
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed

OK.

>  select func_20028_c()|
>  func_20028_c()
>  NULL
> @@ -6122,8 +6084,6 @@ select bug20777(9223372036854775810) as 
>  select bug20777(-9223372036854775808) as 'lower bounds signed bigint';
>  lower bounds signed bigint
>  0
> -Warnings:
> -Warning	1264	Out of range value for column 'f1' at row 1
>  select bug20777(9223372036854775807) as 'upper bounds signed bigint';
>  upper bounds signed bigint
>  9223372036854775807
> @@ -6136,13 +6096,9 @@ upper bounds unsigned bigint
>  select bug20777(18446744073709551616) as 'upper bounds unsigned bigint + 1';
>  upper bounds unsigned bigint + 1
>  18446744073709551615
> -Warnings:
> -Warning	1264	Out of range value for column 'f1' at row 1
>  select bug20777(-1) as 'lower bounds unsigned bigint - 1';
>  lower bounds unsigned bigint - 1
>  0
> -Warnings:
> -Warning	1264	Out of range value for column 'f1' at row 1
>  create table examplebug20777 as select 
>  0 as 'i',
>  bug20777(9223372036854775806) as '2**63-2',
> @@ -6154,9 +6110,6 @@ bug20777(18446744073709551615) as '2**64
>  bug20777(18446744073709551616) as '2**64',
>  bug20777(0) as '0',
>  bug20777(-1) as '-1';
> -Warnings:
> -Warning	1264	Out of range value for column 'f1' at row 1
> -Warning	1264	Out of range value for column 'f1' at row 1
>  insert into examplebug20777 values (1, 9223372036854775806, 9223372036854775807,
> 223372036854775808, 9223372036854775809, 18446744073709551614, 18446744073709551615,
> 8446744073709551616, 0, -1);
>  Warnings:
>  Warning	1264	Out of range value for column '-1' at row 1

The test case for Bug#20777: OK (sigh).

> @@ -6200,7 +6153,8 @@ END|
>  SELECT bug5274_f2()|
>  bug5274_f2()
>  x
> -Warnings:
> +SHOW WARNINGS|
> +Level	Code	Message
>  Warning	1265	Data truncated for column 'bug5274_f1' at row 1
>  Warning	1265	Data truncated for column 'bug5274_f1' at row 1
>  Warning	1265	Data truncated for column 'bug5274_f1' at row 1

OK.

> @@ -6323,22 +6277,13 @@ c1
>  SELECT f1(2);
>  f1(2)
>  0
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  PREPARE s1 FROM 'SELECT f1(2)';
>  EXECUTE s1;
>  f1(2)
>  0
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  EXECUTE s1;
>  f1(2)
>  0
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  DROP PROCEDURE p1;
>  DROP PROCEDURE p2;
>  DROP FUNCTION f1;

OK.

> @@ -6579,8 +6524,6 @@ t1	CREATE TABLE `t1` (
>  DROP TABLE t1;
>  
>  CALL p1('text');
> -Warnings:
> -Warning	1264	Out of range value for column 'v' at row 1
>  SHOW CREATE TABLE t1;
>  Table	Create Table
>  t1	CREATE TABLE `t1` (
> @@ -6599,8 +6542,6 @@ t1	CREATE TABLE `t1` (
>  DROP TABLE t1;
>  
>  CALL p2('text');
> -Warnings:
> -Warning	1366	Incorrect integer value: 'text' for column 'v' at row 1
>  SHOW CREATE TABLE t1;
>  Table	Create Table
>  t1	CREATE TABLE `t1` (

OK.

> diff -Nrup a/mysql-test/r/trigger.result b/mysql-test/r/trigger.result
> --- a/mysql-test/r/trigger.result	2008-02-12 09:44:24 -02:00
> +++ b/mysql-test/r/trigger.result	2008-04-23 19:52:36 -03:00
> @@ -1422,8 +1422,6 @@ create table t2 (j int);
>  insert into t2 values (1), (2);
>  set @a:="";
>  create table if not exists t1 select * from t2;
> -Warnings:
> -Note	1050	Table 't1' already exists

Not OK, please add a test case to trigger.test and a reference
to the bug report that you will report.

>  select * from t1;
>  i
>  7
> @@ -1436,8 +1434,6 @@ drop trigger t1_ai;
>  create table t3 (isave int);
>  create trigger t1_bi before insert on t1 for each row insert into t3 values
> (new.i);
>  create table if not exists t1 select * from t2;
> -Warnings:
> -Note	1050	Table 't1' already exists
>  select * from t1;
>  i
>  7
> @@ -1625,8 +1621,6 @@ truncate t1;
>  truncate t1_op_log;
>  create table if not exists t1
>  select NULL, "CREATE TABLE ... SELECT, inserting a new key";
> -Warnings:
> -Note	1050	Table 't1' already exists
>  set @id=last_insert_id();
>  select * from t1;
>  id	operation
> @@ -1638,8 +1632,6 @@ After INSERT, new=CREATE TABLE ... SELEC
>  truncate t1_op_log;
>  create table if not exists t1 replace
>  select @id, "CREATE TABLE ... REPLACE SELECT, deleting a duplicate key";
> -Warnings:
> -Note	1050	Table 't1' already exists

Same comment here and in other IF NOT EXISTS cases above.

>  select * from t1;
>  id	operation
>  1	CREATE TABLE ... REPLACE SELECT, deleting a duplicate key
> @@ -1820,8 +1812,6 @@ truncate t1;
>  truncate t1_op_log;
>  create table if not exists v1
>  select NULL, "CREATE TABLE ... SELECT, inserting a new key";
> -Warnings:
> -Note	1050	Table 'v1' already exists
>  set @id=last_insert_id();
>  select * from t1;
>  id	operation
> @@ -1833,8 +1823,6 @@ After INSERT, new=CREATE TABLE ... SELEC
>  truncate t1_op_log;
>  create table if not exists v1 replace
>  select @id, "CREATE TABLE ... REPLACE SELECT, deleting a duplicate key";
> -Warnings:
> -Note	1050	Table 'v1' already exists
>  select * from t1;
>  id	operation
>  1	CREATE TABLE ... REPLACE SELECT, deleting a duplicate key

Same here.

> diff -Nrup a/mysql-test/r/view_grant.result b/mysql-test/r/view_grant.result
> --- a/mysql-test/r/view_grant.result	2008-03-22 05:02:23 -03:00
> +++ b/mysql-test/r/view_grant.result	2008-04-23 19:52:37 -03:00
> @@ -341,13 +341,9 @@ use mysqltest;
>  select * from v1;
>  f2()
>  NULL
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed

OK.

>  select * from v2;
>  f2()
>  NULL
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed

OK.

>  select * from v3;
>  ERROR HY000: View 'mysqltest.v3' references invalid table(s) or column(s) or
> function(s) or definer/invoker of view lack rights to use them
>  select * from v4;
> @@ -387,13 +383,9 @@ ERROR HY000: View 'mysqltest.v2' referen
>  select * from v3;
>  f2()
>  NULL
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  select * from v4;
>  f2()
>  NULL
> -Warnings:
> -Warning	1329	No data - zero rows fetched, selected, or processed
>  select * from v5;
>  ERROR HY000: View 'mysqltest.v5' references invalid table(s) or column(s) or
> function(s) or definer/invoker of view lack rights to use them
>  drop view v1, v2, v3, v4, v5;

OK.

> diff -Nrup a/mysql-test/suite/binlog/r/binlog_unsafe.result
> b/mysql-test/suite/binlog/r/binlog_unsafe.result
> --- a/mysql-test/suite/binlog/r/binlog_unsafe.result	2008-03-27 07:13:12 -03:00
> +++ b/mysql-test/suite/binlog/r/binlog_unsafe.result	2008-04-23 19:52:37 -03:00
> @@ -43,12 +43,6 @@ END|
>  CALL proc();
>  Warnings:
>  Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.

The number of warnings is important here.
Please add a continue handler for a warning in which you increment
the warning counter, and in the end select the value of the
counter to make sure that the number of warnings is the same.

>  ---- Insert from stored function ----
>  CREATE FUNCTION func()
>  RETURNS INT
> @@ -65,13 +59,8 @@ END|
>  SELECT func();
>  func()
>  0
> -Warnings:
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> +SHOW WARNINGS;
> +Level	Code	Message
>  Warning	1592	Statement is not safe to log in statement format.
>  ---- Insert from trigger ----
>  CREATE TRIGGER trig
> @@ -89,13 +78,6 @@ END|
>  INSERT INTO trigger_table VALUES ('bye.');
>  Warnings:
>  Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
>  ---- Insert from prepared statement ----
>  PREPARE p1 FROM 'INSERT INTO t1 VALUES (@@global.sync_binlog)';
>  PREPARE p2 FROM 'INSERT INTO t1 VALUES (@@session.insert_id)';
> @@ -153,13 +135,8 @@ PREPARE prep6 FROM 'SELECT func5()'|
>  EXECUTE prep6;
>  func5()
>  0
> -Warnings:
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> +SHOW WARNINGS;
> +Level	Code	Message
>  Warning	1592	Statement is not safe to log in statement format.
>  ==== Variables that should *not* be unsafe ====
>  INSERT INTO t1 VALUES (@@session.pseudo_thread_id);
> @@ -214,9 +191,6 @@ DELETE FROM t1 LIMIT 1;
>  END|
>  CALL p1();
>  Warnings:
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
> -Warning	1592	Statement is not safe to log in statement format.
>  Warning	1592	Statement is not safe to log in statement format.
>  DROP PROCEDURE p1;

Same suggestion applies to all changes in this test file.

>  DROP TABLE t1;
> diff -Nrup a/mysql-test/suite/binlog/t/binlog_unsafe.test
> b/mysql-test/suite/binlog/t/binlog_unsafe.test
> +  /* Fatal errors are not catchable. Do nothing if killed. */
> +  if (! ctx || thd->is_fatal_error || thd->killed_errno())
> +    return FALSE;

Please add a comment why you need to use killed_errno() here, and
not simply check for thd->killed, like everywhere else.

KILL_BAD_DATA is just another hack, unfortunately, but fixing it
is outside the scope of this work.

> > +  if (thd->is_error())
> +    ctx->find_handler(thd, thd->main_da.sql_errno(),
> +                      MYSQL_ERROR::WARN_LEVEL_ERROR);
> +  else if (thd->total_warn_count)
> +  {
> +    MYSQL_ERROR *err;
> +    List_iterator_fast<MYSQL_ERROR> it(thd->warn_list);
> +    while ((err= it++))
> +    {
> +      if (err->level != MYSQL_ERROR::WARN_LEVEL_WARN)
> +        continue;

I think we agreed that continue handlers should work for NOTEs as
well. Please fix that and add a test case. 

> +      if (ctx->find_handler(thd, err->code, err->level))
> +        break;
> +    }
> +  }
> +
> +  switch (ctx->found_handler(ip, &hf)) {
> +  case SP_HANDLER_NONE:
> +    break;
> +  case SP_HANDLER_CONTINUE:
> +    thd->restore_active_arena(execute_arena, backup_arena);
> +    thd->set_n_backup_active_arena(execute_arena, backup_arena);
> +    ctx->push_hstack(instr->get_cont_dest());
> +    /* Fall through */
> +  default:
> +    if (ctx->end_partial_result_set)
> +      thd->protocol->end_partial_result_set(thd);
> +    ctx->clear_handler();
> +    ctx->enter_handler(*ip);
> +    thd->clear_error();
> +    /* Some errors set thd->killed (e.g. "bad data"). */
> +    thd->killed= THD::NOT_KILLED;
> +    found= TRUE;
> +  }
> +
> +  ctx->end_partial_result_set= FALSE;

Why do you need to clear it here? This deserves a comment.
It seems you're mixing two loosely related functions into one, and
it's quite hard to follow, especially without comments.

Would be best if you take care about the protocol somewhere
outside the search for handler. 

> +++ b/sql/sql_class.cc	2008-04-23 19:52:39 -03:00
> @@ -1539,21 +1539,18 @@ bool select_send::send_fields(List<Item>
>  void select_send::abort()
>  {
>    DBUG_ENTER("select_send::abort");
> -  if (is_result_set_started && thd->spcont &&
> -      thd->spcont->find_handler(thd, thd->main_da.sql_errno(),
> -                                MYSQL_ERROR::WARN_LEVEL_ERROR))
> +  if (is_result_set_started && thd->spcont)
>    {
>      /*
>        We're executing a stored procedure, have an open result
> -      set, an SQL exception condition and a handler for it.
> -      In this situation we must abort the current statement,
> -      silence the error and start executing the continue/exit
> -      handler.
> +      set and an SQL exception condition. In this situation we
> +      must abort the current statement, silence the error and
> +      start executing the continue/exit handler if one is found.
>        Before aborting the statement, let's end the open result set, as
>        otherwise the client will hang due to the violation of the
>        client/server protocol.
>      */
> -    thd->protocol->end_partial_result_set(thd);
> +    thd->spcont->end_partial_result_set= TRUE;
>    }
>    DBUG_VOID_RETURN;
>  }

Looks good.
> --- a/sql/sql_parse.cc	2008-04-16 04:53:13 -03:00
> +++ b/sql/sql_parse.cc	2008-04-23 19:52:39 -03:00
> @@ -1845,7 +1845,7 @@ mysql_execute_command(THD *thd)
>      variables, but for now this is probably good enough.
>      Don't reset warnings when executing a stored routine.
>    */
> -  if ((all_tables || !lex->is_single_level_stmt()) && !thd->spcont)
> +  if (all_tables || !lex->is_single_level_stmt())
>      mysql_reset_errors(thd, 0);

Good.

The patch is otherwise looking pretty good, and I think we're
converging to a solution. Thank you for working on this.


-- 
kostja
Thread
bk commit into 6.0 tree (davi:1.2628) BUG#23032Davi Arnaut24 Apr
  • Re: bk commit into 6.0 tree (davi:1.2628) BUG#23032Konstantin Osipov8 May