MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:marc.alff Date:August 3 2006 5:18am
Subject:bk commit into 5.0 tree (malff:1.2246) BUG#8153
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of marcsql. When marcsql does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2006-08-02 22:18:49-07:00, malff@weblab.(none) +7 -0
  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.

  mysql-test/r/sp.result@stripped, 2006-08-02 21:45:42-07:00, malff@weblab.(none) +60 -2
    Bug#8153, added test cases, fixed expected result of bug18787.

  mysql-test/t/sp.test@stripped, 2006-08-02 21:44:48-07:00, malff@weblab.(none) +62 -0
    Bug#8153, added test cases.

  sql/mysqld.cc@stripped, 2006-08-02 21:36:06-07:00, malff@weblab.(none) +1 -3
    Bug#8153, use sp_rcontext::handle_error() to handle errors.

  sql/protocol.cc@stripped, 2006-08-02 21:39:59-07:00, malff@weblab.(none) +7 -8
    Bug#8153, use sp_rcontext::handle_error() to handle errors.

  sql/sp_rcontext.cc@stripped, 2006-08-02 21:43:00-07:00, malff@weblab.(none) +59 -0
    Bug#8153, created helper sp_rcontext::handle_error() to handle errors.

  sql/sp_rcontext.h@stripped, 2006-08-02 21:41:51-07:00, malff@weblab.(none) +6 -0
    Bug#8153, created helper sp_rcontext::handle_error() to handle errors.

  sql/sql_error.cc@stripped, 2006-08-02 21:38:45-07:00, malff@weblab.(none) +1 -7
    Bug#8153, use sp_rcontext::handle_error() to handle errors.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	malff
# Host:	weblab.(none)
# Root:	/home/marcsql/TREE/mysql-5.0-8153

--- 1.560/sql/mysqld.cc	2006-08-02 22:18:54 -07:00
+++ 1.561/sql/mysqld.cc	2006-08-02 22:18:54 -07:00
@@ -2392,10 +2392,8 @@ static int my_message_sql(uint error, co
   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);
     }
 

--- 1.114/sql/protocol.cc	2006-08-02 22:18:54 -07:00
+++ 1.115/sql/protocol.cc	2006-08-02 22:18:54 -07:00
@@ -70,13 +70,13 @@ void net_send_error(THD *thd, uint sql_e
     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

--- 1.204/mysql-test/r/sp.result	2006-08-02 22:18:54 -07:00
+++ 1.205/mysql-test/r/sp.result	2006-08-02 22:18:54 -07:00
@@ -4872,8 +4872,6 @@ declare continue handler for sqlexceptio
 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;

--- 1.192/mysql-test/t/sp.test	2006-08-02 22:18:54 -07:00
+++ 1.193/mysql-test/t/sp.test	2006-08-02 22:18:54 -07:00
@@ -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|

--- 1.42/sql/sp_rcontext.cc	2006-08-02 22:18:54 -07:00
+++ 1.43/sql/sp_rcontext.cc	2006-08-02 22:18:54 -07:00
@@ -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)

--- 1.32/sql/sp_rcontext.h	2006-08-02 22:18:54 -07:00
+++ 1.33/sql/sp_rcontext.h	2006-08-02 22:18:54 -07:00
@@ -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)

--- 1.37/sql/sql_error.cc	2006-08-02 22:18:54 -07:00
+++ 1.38/sql/sql_error.cc	2006-08-02 22:18:54 -07:00
@@ -139,14 +139,8 @@ MYSQL_ERROR *push_warning(THD *thd, MYSQ
   }
 
   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);
Thread
bk commit into 5.0 tree (malff:1.2246) BUG#8153marc.alff3 Aug