Bug#8153 (Stored procedure with subquery and continue handler, wrong result)

Implemented code review comments
Test cleanup


sql/protocol.cc:
  Bug#8153 (Stored procedure with subquery and continue handler, wrong result)
  
  Implemented code review comments

mysql-test/r/sp.result

@@ -5218,12 +5218,18 @@ CHARSET(p2)	COLLATION(p2)
 cp1251	cp1251_general_ci
 CHARSET(p3)	COLLATION(p3)
 greek	greek_general_ci
-drop table if exists t3, t4, t5|
+use test|
+DROP DATABASE mysqltest1|
+drop table if exists t3|
+drop table if exists t4|
 drop procedure if exists bug8153_subselect|
-drop procedure if exists bug8153_function|
+drop procedure if exists bug8153_subselect_a|
+drop procedure if exists bug8153_subselect_b|
+drop procedure if exists bug8153_proc_a|
+drop procedure if exists bug8153_proc_b|
 create table t3 (a int)|
 create table t4 (a int)|
-insert into t3 values (1)|
+insert into t3 values (1), (1), (2), (3)|
 insert into t4 values (1), (1)|
 create procedure bug8153_subselect()
 begin
@@ -5242,6 +5248,9 @@ statement after update
 select * from t3|
 a
 1
+1
+2
+3
 call bug8153_subselect()|
 statement failed
 statement failed
@@ -5250,24 +5259,75 @@ statement after update
 select * from t3|
 a
 1
+1
+2
+3
 drop procedure bug8153_subselect|
-create procedure bug8153_function_a()
+create procedure bug8153_subselect_a()
 begin
 declare continue handler for sqlexception
 begin
 select 'in continue handler';
 end;
 select 'reachable code a1';
-call bug8153_function_b();
+call bug8153_subselect_b();
 select 'reachable code a2';
 end|
-create procedure bug8153_function_b()
+create procedure bug8153_subselect_b()
+begin
+select 'reachable code b1';
+update t3 set a=a+1 where (select a from t4 where a=1) is null;
+select 'unreachable code b2';
+end|
+call bug8153_subselect_a()|
+reachable code a1
+reachable code a1
+reachable code b1
+reachable code b1
+in continue handler
+in continue handler
+reachable code a2
+reachable code a2
+select * from t3|
+a
+1
+1
+2
+3
+call bug8153_subselect_a()|
+reachable code a1
+reachable code a1
+reachable code b1
+reachable code b1
+in continue handler
+in continue handler
+reachable code a2
+reachable code a2
+select * from t3|
+a
+1
+1
+2
+3
+drop procedure bug8153_subselect_a|
+drop procedure bug8153_subselect_b|
+create procedure bug8153_proc_a()
+begin
+declare continue handler for sqlexception
+begin
+select 'in continue handler';
+end;
+select 'reachable code a1';
+call bug8153_proc_b();
+select 'reachable code a2';
+end|
+create procedure bug8153_proc_b()
 begin
 select 'reachable code b1';
 select no_such_function();
 select 'unreachable code b2';
 end|
-call bug8153_function_a()|
+call bug8153_proc_a()|
 reachable code a1
 reachable code a1
 reachable code b1
@@ -5276,10 +5336,10 @@ in continue handler
 in continue handler
 reachable code a2
 reachable code a2
-drop procedure bug8153_function_a|
+drop procedure bug8153_proc_a|
-drop procedure bug8153_function_b|
+drop procedure bug8153_proc_b|
-use test|
+drop table t3|
-DROP DATABASE mysqltest1|
+drop table t4|
 drop procedure if exists bug19862|
 CREATE TABLE t11 (a INT)|
 CREATE TABLE t12 (a INT)|

mysql-test/t/sp.test

@@ -6141,19 +6141,28 @@ SET @v3 = 'c'|
 CALL bug16676_p1('a', @v2, @v3)|
 CALL bug16676_p2('a', @v2, @v3)|
 
+# Cleanup.
+
+use test|
+
+DROP DATABASE mysqltest1|
 #
 # BUG#8153: Stored procedure with subquery and continue handler, wrong result
 #
 
 --disable_warnings
-drop table if exists t3, t4, t5|
+drop table if exists t3|
+drop table if exists t4|
 drop procedure if exists bug8153_subselect|
-drop procedure if exists bug8153_function|
+drop procedure if exists bug8153_subselect_a|
+drop procedure if exists bug8153_subselect_b|
+drop procedure if exists bug8153_proc_a|
+drop procedure if exists bug8153_proc_b|
 --enable_warnings
 
 create table t3 (a int)|
 create table t4 (a int)|
-insert into t3 values (1)|
+insert into t3 values (1), (1), (2), (3)|
 insert into t4 values (1), (1)|
 
 ## Testing the use case reported in Bug#8153
@@ -6176,10 +6185,9 @@ select * from t3|
 
 drop procedure bug8153_subselect|
 
-## Testing extra use cases, found while investigating
+## Testing a subselect with a non local handler
-## This is related to BUG#18787, with a non local handler
 
-create procedure bug8153_function_a()
+create procedure bug8153_subselect_a()
 begin
   declare continue handler for sqlexception
   begin
@@ -6187,27 +6195,55 @@ begin
   end;
 
   select 'reachable code a1';
-  call bug8153_function_b();
+  call bug8153_subselect_b();
   select 'reachable code a2';
 end|
 
-create procedure bug8153_function_b()
+create procedure bug8153_subselect_b()
+begin
+  select 'reachable code b1';
+  update t3 set a=a+1 where (select a from t4 where a=1) is null;
+  select 'unreachable code b2';
+end|
+
+call bug8153_subselect_a()|
+select * from t3|
+
+call bug8153_subselect_a()|
+select * from t3|
+
+drop procedure bug8153_subselect_a|
+drop procedure bug8153_subselect_b|
+
+## Testing extra use cases, found while investigating
+## This is related to BUG#18787, with a non local handler
+
+create procedure bug8153_proc_a()
+begin
+  declare continue handler for sqlexception
+  begin
+    select 'in continue handler';
+  end;
+
+  select 'reachable code a1';
+  call bug8153_proc_b();
+  select 'reachable code a2';
+end|
+
+create procedure bug8153_proc_b()
 begin
   select 'reachable code b1';
   select no_such_function();
   select 'unreachable code b2';
 end|
 
-call bug8153_function_a()|
+call bug8153_proc_a()|
 
-drop procedure bug8153_function_a|
+drop procedure bug8153_proc_a|
-drop procedure bug8153_function_b|
+drop procedure bug8153_proc_b|
+drop table t3|
+drop table t4|
 
-# Cleanup.
-
-use test|
-
-DROP DATABASE mysqltest1|
 #
 # BUG#19862: Sort with filesort by function evaluates function twice
 #

sql/protocol.cc

@@ -53,8 +53,18 @@ bool Protocol_prep::net_store_data(const char *from, uint length)
 }
 
 
-	/* Send a error string to client */
+/*
+   Send a error string to client
 
+   Design note:
+
+   net_printf_error and net_send_error are low-level functions
+   that shall be used only when a new connection is being
+   established or at server startup.
+   For SIGNAL/RESIGNAL and GET DIAGNOSTICS functionality it's
+   critical that every error that can be intercepted is issued in one
+   place only, my_message_sql.
+*/
 void net_send_error(THD *thd, uint sql_errno, const char *err)
 {
   NET *net= &thd->net;
@@ -64,6 +74,8 @@ void net_send_error(THD *thd, uint sql_errno, const char *err)
 		      err ? err : net->last_error[0] ?
 		      net->last_error : "NULL"));
 
+  DBUG_ASSERT(!thd->spcont);
+
   if (net && net->no_send_error)
   {
     thd->clear_error();
@@ -71,12 +83,6 @@ void net_send_error(THD *thd, uint sql_errno, const char *err)
     DBUG_VOID_RETURN;
   }
 
-  if (thd->spcont &&
-      thd->spcont->handle_error(sql_errno, MYSQL_ERROR::WARN_LEVEL_ERROR, thd))
-  {
-    DBUG_VOID_RETURN;
-  }
-
   thd->query_error=  1; // needed to catch query errors during replication
   if (!err)
   {
@@ -117,6 +123,15 @@ void net_send_error(THD *thd, uint sql_errno, const char *err)
    Write error package and flush to client
    It's a little too low level, but I don't want to use another buffer for
    this
+
+   Design note:
+
+   net_printf_error and net_send_error are low-level functions
+   that shall be used only when a new connection is being
+   established or at server startup.
+   For SIGNAL/RESIGNAL and GET DIAGNOSTICS functionality it's
+   critical that every error that can be intercepted is issued in one
+   place only, my_message_sql.
 */
 
 void
@@ -136,6 +151,8 @@ net_printf_error(THD *thd, uint errcode, ...)
   DBUG_ENTER("net_printf_error");
   DBUG_PRINT("enter",("message: %u",errcode));
 
+  DBUG_ASSERT(!thd->spcont);
+
   if (net && net->no_send_error)
   {
     thd->clear_error();
@@ -143,12 +160,6 @@ net_printf_error(THD *thd, uint errcode, ...)
     DBUG_VOID_RETURN;
   }
 
-  if (thd->spcont &&
-      thd->spcont->handle_error(errcode, MYSQL_ERROR::WARN_LEVEL_ERROR, thd))
-  {
-    DBUG_VOID_RETURN;
-  }
-
   thd->query_error=  1; // needed to catch query errors during replication
 #ifndef EMBEDDED_LIBRARY
   query_cache_abort(net);	// Safety