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

Before this fix,
- a runtime error in a statement in a stored procedure with no error handlers
was properly detected (as expected)
- a runtime error in a statement with an error handler inherited from a non
local runtime context (i.e., proc a with a handler, calling proc b) was
properly detected (as expected)
- a runtime error in a statement with a *local* error handler was executed
as follows :
a) the statement would succeed, regardless of the error condition, (bug)
b) the error handler would be called (as expected).

The root cause is that functions like my_messqge_sql would "forget" to set
the thread flag thd->net.report_error to 1, because of the check involving
sp_rcontext::found_handler_here().
Failure to set this flag would cause, later in the call stack,
in Item_func::fix_fields() at line 190, the code to return FALSE and consider
that executing the statement was successful.

With this fix :
- error handling code, that was duplicated in different places in the code,
is now implemented in sp_rcontext::handle_error(),
- handle_error() correctly sets thd->net.report_error when a handler is
present, regardless of the handler location (local, or in the call stack).

A test case, bug8153_subselect, has been written to demonstrate the change
of behavior before and after the fix.

Another test case, bug8153_function_a, as also been writen.
This test has the same behavior before and after the fix.
This test has been written to demonstrate that the previous expected
result of procedure bug18787, was incorrect, since select no_such_function()
should fail and therefore not produce a result.

The incorrect result for bug18787 has the same root cause as Bug#8153,
and the expected result has been adjusted.

7 files changed, 196 insertions, 20 deletions

diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result
index 70d8a99c7c2..7e74cfffa94 100644
--- a/mysql-test/r/sp.result
+++ b/mysql-test/r/sp.result
@@ -4872,8 +4872,6 @@ declare continue handler for sqlexception begin end;
 select no_such_function();
 end|
 call bug18787()|
-no_such_function()
-NULL
 drop procedure bug18787|
 create database bug18344_012345678901|
 use bug18344_012345678901|
@@ -5220,6 +5218,66 @@ CHARSET(p2)	COLLATION(p2)
 cp1251	cp1251_general_ci
 CHARSET(p3)	COLLATION(p3)
 greek	greek_general_ci
+drop table if exists t3, t4, t5|
+drop procedure if exists bug8153_subselect|
+drop procedure if exists bug8153_function|
+create table t3 (a int)|
+create table t4 (a int)|
+insert into t3 values (1)|
+insert into t4 values (1), (1)|
+create procedure bug8153_subselect()
+begin
+declare continue handler for sqlexception
+begin
+select 'statement failed';
+end;
+update t3 set a=a+1 where (select a from t4 where a=1) is null;
+select 'statement after update';
+end|
+call bug8153_subselect()|
+statement failed
+statement failed
+statement after update
+statement after update
+select * from t3|
+a
+1
+call bug8153_subselect()|
+statement failed
+statement failed
+statement after update
+statement after update
+select * from t3|
+a
+1
+drop procedure bug8153_subselect|
+create procedure bug8153_function_a()
+begin
+declare continue handler for sqlexception
+begin
+select 'in continue handler';
+end;
+select 'reachable code a1';
+call bug8153_function_b();
+select 'reachable code a2';
+end|
+create procedure bug8153_function_b()
+begin
+select 'reachable code b1';
+select no_such_function();
+select 'unreachable code b2';
+end|
+call bug8153_function_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
+drop procedure bug8153_function_a|
+drop procedure bug8153_function_b|
 use test|
 DROP DATABASE mysqltest1|
 drop table t1,t2;
diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test
index 3f051f77954..6eadf06461b 100644
--- a/mysql-test/t/sp.test
+++ b/mysql-test/t/sp.test
@@ -6141,6 +6141,68 @@ SET @v3 = 'c'|
 CALL bug16676_p1('a', @v2, @v3)|
 CALL bug16676_p2('a', @v2, @v3)|
 
+#
+# BUG#8153: Stored procedure with subquery and continue handler, wrong result
+#
+
+--disable_warnings
+drop table if exists t3, t4, t5|
+drop procedure if exists bug8153_subselect|
+drop procedure if exists bug8153_function|
+--enable_warnings
+
+create table t3 (a int)|
+create table t4 (a int)|
+insert into t3 values (1)|
+insert into t4 values (1), (1)|
+
+## Testing the use case reported in Bug#8153
+
+create procedure bug8153_subselect()
+begin
+  declare continue handler for sqlexception
+  begin
+    select 'statement failed';
+  end;
+  update t3 set a=a+1 where (select a from t4 where a=1) is null;
+  select 'statement after update';
+end|
+
+call bug8153_subselect()|
+select * from t3|
+
+call bug8153_subselect()|
+select * from t3|
+
+drop procedure bug8153_subselect|
+
+## Testing extra use cases, found while investigating
+## This is related to BUG#18787, with a non local handler
+
+create procedure bug8153_function_a()
+begin
+  declare continue handler for sqlexception
+  begin
+    select 'in continue handler';
+  end;
+
+  select 'reachable code a1';
+  call bug8153_function_b();
+  select 'reachable code a2';
+end|
+
+create procedure bug8153_function_b()
+begin
+  select 'reachable code b1';
+  select no_such_function();
+  select 'unreachable code b2';
+end|
+
+call bug8153_function_a()|
+
+drop procedure bug8153_function_a|
+drop procedure bug8153_function_b|
+
 # Cleanup.
 
 use test|
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index 429bdee17d6..426edfed52f 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -2392,10 +2392,8 @@ static int my_message_sql(uint error, const char *str, myf MyFlags)
   if ((thd= current_thd))
   {
     if (thd->spcont &&
-        thd->spcont->find_handler(error, MYSQL_ERROR::WARN_LEVEL_ERROR))
+        thd->spcont->handle_error(error, MYSQL_ERROR::WARN_LEVEL_ERROR, thd))
     {
-      if (! thd->spcont->found_handler_here())
-        thd->net.report_error= 1; /* Make "select" abort correctly */
       DBUG_RETURN(0);
     }
 
diff --git a/sql/protocol.cc b/sql/protocol.cc
index f4efb8004ee..c6cacf978ea 100644
--- a/sql/protocol.cc
+++ b/sql/protocol.cc
@@ -70,13 +70,13 @@ void net_send_error(THD *thd, uint sql_errno, const char *err)
     DBUG_PRINT("info", ("sending error messages prohibited"));
     DBUG_VOID_RETURN;
   }
-  if (thd->spcont && thd->spcont->find_handler(sql_errno,
-                                               MYSQL_ERROR::WARN_LEVEL_ERROR))
+
+  if (thd->spcont &&
+      thd->spcont->handle_error(sql_errno, MYSQL_ERROR::WARN_LEVEL_ERROR, thd))
   {
-    if (! thd->spcont->found_handler_here())
-      thd->net.report_error= 1; /* Make "select" abort correctly */ 
     DBUG_VOID_RETURN;
   }
+
   thd->query_error=  1; // needed to catch query errors during replication
   if (!err)
   {
@@ -143,13 +143,12 @@ net_printf_error(THD *thd, uint errcode, ...)
     DBUG_VOID_RETURN;
   }
 
-  if (thd->spcont && thd->spcont->find_handler(errcode,
-                                               MYSQL_ERROR::WARN_LEVEL_ERROR))
+  if (thd->spcont &&
+      thd->spcont->handle_error(errcode, MYSQL_ERROR::WARN_LEVEL_ERROR, thd))
   {
-    if (! thd->spcont->found_handler_here())
-      thd->net.report_error= 1; /* Make "select" abort correctly */ 
     DBUG_VOID_RETURN;
   }
+
   thd->query_error=  1; // needed to catch query errors during replication
 #ifndef EMBEDDED_LIBRARY
   query_cache_abort(net);	// Safety
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc
index 3bc27a029d0..67ee5459bb4 100644
--- a/sql/sp_rcontext.cc
+++ b/sql/sp_rcontext.cc
@@ -260,6 +260,65 @@ sp_rcontext::find_handler(uint sql_errno,
   return TRUE;
 }
 
+/*
+   Handle the error for a given errno.
+   The severity of the error is adjusted depending of the current sql_mode.
+   If an handler is present for the error (see find_handler()),
+   this function will return true.
+   If a handler is found and if the severity of the error indicate
+   that the current instruction executed should abort,
+   the flag thd->net.report_error is also set.
+   This will cause the execution of the current instruction in a
+   sp_instr* to fail, and give control to the handler code itself
+   in the sp_head::execute() loop.
+
+  SYNOPSIS
+    sql_errno     The error code
+    level         Warning level
+    thd           The current thread
+                  - thd->net.report_error is an optional output.
+
+  RETURN
+    TRUE       if a handler was found.
+    FALSE      if no handler was found.
+*/
+bool
+sp_rcontext::handle_error(uint sql_errno,
+                          MYSQL_ERROR::enum_warning_level level,
+                          THD *thd)
+{
+  bool handled= FALSE;
+  MYSQL_ERROR::enum_warning_level elevated_level= level;
+
+
+  /* Depending on the sql_mode of execution,
+     warnings may be considered errors */
+  if ((level == MYSQL_ERROR::WARN_LEVEL_WARN) &&
+      thd->really_abort_on_warning())
+  {
+    elevated_level= MYSQL_ERROR::WARN_LEVEL_ERROR;
+  }
+
+  if (find_handler(sql_errno, elevated_level))
+  {
+    if (elevated_level == MYSQL_ERROR::WARN_LEVEL_ERROR)
+    {
+      /*
+         Forces to abort the current instruction execution.
+         NOTE: This code is altering the original meaning of
+         the net.report_error flag (send an error to the client).
+         In the context of stored procedures with error handlers,
+         the flag is reused to cause error propagation,
+         until the error handler is reached.
+         No messages will be sent to the client in that context.
+      */
+      thd->net.report_error= 1;
+    }
+    handled= TRUE;
+  }
+
+  return handled;
+}
 
 void
 sp_rcontext::push_cursor(sp_lex_keeper *lex_keeper, sp_instr_cpush *i)
diff --git a/sql/sp_rcontext.h b/sql/sp_rcontext.h
index 30521f6da84..5e03aa60d23 100644
--- a/sql/sp_rcontext.h
+++ b/sql/sp_rcontext.h
@@ -128,6 +128,12 @@ class sp_rcontext : public Sql_alloc
   bool
   find_handler(uint sql_errno,MYSQL_ERROR::enum_warning_level level);
 
+  // If there is an error handler for this error, handle it and return TRUE.
+  bool
+  handle_error(uint sql_errno,
+               MYSQL_ERROR::enum_warning_level level,
+               THD *thd);
+
   // Returns handler type and sets *ip to location if one was found
   inline int
   found_handler(uint *ip, uint *fp)
diff --git a/sql/sql_error.cc b/sql/sql_error.cc
index 19811efbb12..ebd515bd209 100644
--- a/sql/sql_error.cc
+++ b/sql/sql_error.cc
@@ -139,14 +139,8 @@ MYSQL_ERROR *push_warning(THD *thd, MYSQL_ERROR::enum_warning_level level,
   }
 
   if (thd->spcont &&
-      thd->spcont->find_handler(code,
-                                ((int) level >=
-                                 (int) MYSQL_ERROR::WARN_LEVEL_WARN &&
-                                 thd->really_abort_on_warning()) ?
-                                MYSQL_ERROR::WARN_LEVEL_ERROR : level))
+      thd->spcont->handle_error(code, level, thd))
   {
-    if (! thd->spcont->found_handler_here())
-      thd->net.report_error= 1; /* Make "select" abort correctly */ 
     DBUG_RETURN(NULL);
   }
   query_cache_abort(&thd->net);