List:Internals« Previous MessageNext Message »
From:antony Date:August 18 2005 3:02pm
Subject:bk commit into 5.0 tree (acurtis:1.1993) BUG#10100
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of antony. When antony 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
  1.1993 05/08/18 16:02:04 acurtis@stripped +8 -0
  Bug#10100
    "function (and stored procedure?) recursivity problem"
    Add system variable to control recursion

  sql/sql_class.h
    1.260 05/08/18 16:01:31 acurtis@stripped +1 -0
    new system variable sp_recursion_enabled

  sql/sp_head.h
    1.65 05/08/18 16:01:31 acurtis@stripped +5 -3
    changes for recursion

  sql/sp_head.cc
    1.167 05/08/18 16:01:31 acurtis@stripped +3 -1
    changes for recursion

  sql/sp.cc
    1.91 05/08/18 16:01:31 acurtis@stripped +139 -81
    refactor and enable recursion

  sql/set_var.cc
    1.133 05/08/18 16:01:30 acurtis@stripped +9 -0
    new system var - sp_recursion_enabled

  sql/item_func.cc
    1.241 05/08/18 16:01:30 acurtis@stripped +4 -1
    enable recursion and respect sp_recursion_enabled flag

  mysql-test/t/sp.test
    1.138 05/08/18 16:01:30 acurtis@stripped +41 -0
    Test for bug 10100

  mysql-test/r/sp.result
    1.143 05/08/18 16:01:30 acurtis@stripped +39 -0
    Test for bug 10100

# 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:	acurtis
# Host:	ltantony.xiphis.org
# Root:	/usr/home/antony/work2/p3-bug10100

--- 1.240/sql/item_func.cc	2005-08-15 16:15:03 +01:00
+++ 1.241/sql/item_func.cc	2005-08-18 16:01:30 +01:00
@@ -4675,7 +4675,10 @@
   st_sp_security_context save_ctx;
 #endif
 
-  if (! m_sp && ! (m_sp= sp_find_function(thd, m_name, TRUE)))
+  if ((!m_sp || 
+      (thd->variables.sp_recursion_enabled ? m_sp->m_is_invoked : 
+                                             m_sp->m_is_child)) && 
+      !(m_sp= sp_find_function(thd, m_name, TRUE)))
   {
     my_error(ER_SP_DOES_NOT_EXIST, MYF(0), "FUNCTION", m_name->m_qname.str);
     goto error;

--- 1.259/sql/sql_class.h	2005-08-15 16:31:00 +01:00
+++ 1.260/sql/sql_class.h	2005-08-18 16:01:31 +01:00
@@ -546,6 +546,7 @@
   my_bool new_mode;
   my_bool query_cache_wlock_invalidate;
   my_bool engine_condition_pushdown;
+  my_bool sp_recursion_enabled;
 #ifdef HAVE_REPLICATION
   ulong sync_replication;
   ulong sync_replication_slave_id;

--- 1.132/sql/set_var.cc	2005-08-11 17:01:36 +01:00
+++ 1.133/sql/set_var.cc	2005-08-18 16:01:30 +01:00
@@ -99,6 +99,7 @@
 static int  check_pseudo_thread_id(THD *thd, set_var *var);
 static bool set_log_bin(THD *thd, set_var *var);
 static void fix_low_priority_updates(THD *thd, enum_var_type type);
+static void fix_sp_recursion_enabled(THD *thd, enum_var_type type);
 static void fix_tx_isolation(THD *thd, enum_var_type type);
 static int check_completion_type(THD *thd, set_var *var);
 static void fix_completion_type(THD *thd, enum_var_type type);
@@ -361,6 +362,9 @@
 				       &SV::table_type);
 sys_var_thd_storage_engine sys_storage_engine("storage_engine",
 				       &SV::table_type);
+sys_var_thd_bool	sys_sp_recursion_enabled("sp_recursion_enabled",
+						 &SV::sp_recursion_enabled,
+						 fix_sp_recursion_enabled);
 #ifdef HAVE_REPLICATION
 sys_var_sync_binlog_period sys_sync_binlog_period("sync_binlog", &sync_binlog_period);
 sys_var_thd_ulong	sys_sync_replication("sync_replication",
@@ -678,6 +682,7 @@
   &sys_sql_warnings,
   &sys_sql_notes,
   &sys_storage_engine,
+  &sys_sp_recursion_enabled,
 #ifdef HAVE_REPLICATION
   &sys_sync_binlog_period,
   &sys_sync_replication,
@@ -966,6 +971,7 @@
   {sys_sort_buffer.name,      (char*) &sys_sort_buffer, 	    SHOW_SYS},
   {sys_sql_mode.name,         (char*) &sys_sql_mode,                SHOW_SYS},
   {sys_storage_engine.name,   (char*) &sys_storage_engine,          SHOW_SYS},
+  {sys_sp_recursion_enabled.name, (char*) &sys_sp_recursion_enabled, SHOW_SYS},
   {"sql_notes",               (char*) &sys_sql_notes,               SHOW_BOOL},
   {"sql_warnings",            (char*) &sys_sql_warnings,            SHOW_BOOL},
 #ifdef HAVE_REPLICATION
@@ -1180,6 +1186,9 @@
     thd->session_tx_isolation= ((enum_tx_isolation)
 				thd->variables.tx_isolation);
 }
+
+static void fix_sp_recursion_enabled(THD *thd __attribute__(unused), 
+				enum_var_type type __attribute__(unused)) {}
 
 static void fix_completion_type(THD *thd __attribute__(unused), 
 				enum_var_type type __attribute__(unused)) {}

--- 1.142/mysql-test/r/sp.result	2005-08-09 08:43:42 +01:00
+++ 1.143/mysql-test/r/sp.result	2005-08-18 16:01:30 +01:00
@@ -3085,4 +3085,43 @@
 id	id
 data	data
 drop function bug10055|
+drop function if exists bug10100f|
+drop procedure if exists bug10100p|
+drop procedure if exists bug10100t|
+create function bug10100f(prm int) returns int
+begin
+if prm > 1 then
+return prm * bug10100f(prm - 1);
+end if;
+return 1;
+end|
+create procedure bug10100p(prm int, inout res int)
+begin
+set res = res * prm;
+if prm > 1 then
+call bug10100p(prm - 1, res);  
+end if;
+end|
+create procedure bug10100t(prm int)
+begin
+declare res int;
+set res = 1;
+call bug10100p(prm, res);
+select res;
+end|
+set @@sp_recursion_enabled=1|
+select bug10100f(5)|
+bug10100f(5)
+120
+call bug10100t(5)|
+res
+120
+set @@sp_recursion_enabled=0|
+select bug10100f(5)|
+ERROR HY000: Recursive stored routines are not allowed.
+call bug10100t(5)|
+ERROR HY000: Recursive stored routines are not allowed.
+drop function bug10100f|
+drop procedure bug10100p|
+drop procedure bug10100t|
 drop table t1,t2;

--- 1.137/mysql-test/t/sp.test	2005-08-11 13:59:20 +01:00
+++ 1.138/mysql-test/t/sp.test	2005-08-18 16:01:30 +01:00
@@ -3871,6 +3871,47 @@
 drop function bug10055|
 
 #
+# BUG#10100: function (and stored procedure?) recursivity problem
+#
+--disable_warnings
+drop function if exists bug10100f|
+drop procedure if exists bug10100p|
+drop procedure if exists bug10100t|
+--enable_warnings
+create function bug10100f(prm int) returns int
+begin
+  if prm > 1 then
+    return prm * bug10100f(prm - 1);
+  end if;
+  return 1;
+end|
+create procedure bug10100p(prm int, inout res int)
+begin
+  set res = res * prm;
+  if prm > 1 then
+    call bug10100p(prm - 1, res);  
+  end if;
+end|
+create procedure bug10100t(prm int)
+begin
+  declare res int;
+  set res = 1;
+  call bug10100p(prm, res);
+  select res;
+end|
+set @@sp_recursion_enabled=1|
+select bug10100f(5)|
+call bug10100t(5)|
+set @@sp_recursion_enabled=0|
+--error 1424
+select bug10100f(5)|
+--error 1424
+call bug10100t(5)|
+drop function bug10100f|
+drop procedure bug10100p|
+drop procedure bug10100t|
+
+#
 # BUG#NNNN: New bug synopsis
 #
 #--disable_warnings

--- 1.90/sql/sp.cc	2005-08-15 20:39:32 +01:00
+++ 1.91/sql/sp.cc	2005-08-18 16:01:31 +01:00
@@ -29,6 +29,14 @@
 	      const char *returns, ulong returnslen,
 	      const char *body, ulong bodylen,
 	      st_sp_chistics *chistics);
+static int
+db_load_routine(THD *thd, int type, sp_name *name, sp_head **sphp,
+                ulong sql_mode, const char *params, const char *returns,
+                const char *body, st_sp_chistics &chistics,
+                const char *definer, longlong created, longlong modified);
+static sp_head *
+sp_find_routine(THD *thd, int type, sp_name *name, sp_cache **cp,
+                bool cache_only);
 
 /*
  *
@@ -377,83 +385,92 @@
   close_proc_table(thd, &open_tables_state_backup);
   table= 0;
 
-  {
-    String defstr;
-    LEX *oldlex= thd->lex;
-    char olddb[128];
-    bool dbchanged;
-    enum enum_sql_command oldcmd= thd->lex->sql_command;
-    ulong old_sql_mode= thd->variables.sql_mode;
-    ha_rows select_limit= thd->variables.select_limit;
-
-    thd->variables.sql_mode= sql_mode;
-    thd->variables.select_limit= HA_POS_ERROR;
-
-    defstr.set_charset(system_charset_info);
-    if (!create_string(thd, &defstr,
-		       type,
-		       name,
-		       params, strlen(params),
-		       returns, strlen(returns),
-		       body, strlen(body),
-		       &chistics))
-    {
-      ret= SP_INTERNAL_ERROR;
-      goto done;
-    }
+  ret= db_load_routine(thd, type, name, sphp,
+                       sql_mode, params, returns, body, chistics,
+                       definer, created, modified);
+                       
+ done:
+  if (table)
+    close_proc_table(thd, &open_tables_state_backup);
+  DBUG_RETURN(ret);
+}
 
-    dbchanged= FALSE;
-    if ((ret= sp_use_new_db(thd, name->m_db.str, olddb, sizeof(olddb),
-			    1, &dbchanged)))
-      goto done;
 
-    {
-      /* This is something of a kludge. We need to initialize some fields
-       * in thd->lex (the unit and master stuff), and the easiest way to
-       * do it is, is to call mysql_init_query(), but this unfortunately
-       * resets teh value_list where we keep the CALL parameters. So we
-       * copy the list and then restore it. (... and found_semicolon too).
-       */
-      List<Item> tmpvals= thd->lex->value_list;
-      char *tmpfsc= thd->lex->found_semicolon;
-
-      lex_start(thd, (uchar*)defstr.c_ptr(), defstr.length());
-      thd->lex->value_list= tmpvals;
-      thd->lex->found_semicolon= tmpfsc;
-    }
+static int
+db_load_routine(THD *thd, int type, sp_name *name, sp_head **sphp,
+                ulong sql_mode, const char *params, const char *returns,
+                const char *body, st_sp_chistics &chistics,
+                const char *definer, longlong created, longlong modified)
+{
+  String defstr;
+  LEX *oldlex= thd->lex;
+  char olddb[128];
+  bool dbchanged;
+  enum enum_sql_command oldcmd= thd->lex->sql_command;
+  ulong old_sql_mode= thd->variables.sql_mode;
+  ha_rows select_limit= thd->variables.select_limit;
+  int ret;
 
-    if (yyparse(thd) || thd->is_fatal_error || thd->lex->sphead == NULL)
-    {
-      LEX *newlex= thd->lex;
-      sp_head *sp= newlex->sphead;
+  thd->variables.sql_mode= sql_mode;
+  thd->variables.select_limit= HA_POS_ERROR;
 
-      if (dbchanged && (ret= mysql_change_db(thd, olddb, 1)))
-	goto done;
-      if (sp)
-      {
-	delete sp;
-	newlex->sphead= NULL;
-      }
-      ret= SP_PARSE_ERROR;
-    }
-    else
+  defstr.set_charset(system_charset_info);
+  if (!create_string(thd, &defstr,
+		     type,
+		     name,
+		     params, strlen(params),
+		     returns, strlen(returns),
+		     body, strlen(body),
+		     &chistics))
+    return SP_INTERNAL_ERROR;
+
+  dbchanged= FALSE;
+  if ((ret= sp_use_new_db(thd, name->m_db.str, olddb, sizeof(olddb),
+			  1, &dbchanged)))
+    return ret;
+
+  {
+    /* This is something of a kludge. We need to initialize some fields
+     * in thd->lex (the unit and master stuff), and the easiest way to
+     * do it is, is to call mysql_init_query(), but this unfortunately
+     * resets teh value_list where we keep the CALL parameters. So we
+     * copy the list and then restore it. (... and found_semicolon too).
+     */
+    List<Item> tmpvals= thd->lex->value_list;
+    char *tmpfsc= thd->lex->found_semicolon;
+
+    lex_start(thd, (uchar*)defstr.c_ptr(), defstr.length());
+    thd->lex->value_list= tmpvals;
+    thd->lex->found_semicolon= tmpfsc;
+  }
+
+  if (yyparse(thd) || thd->is_fatal_error || thd->lex->sphead == NULL)
+  {
+    LEX *newlex= thd->lex;
+    sp_head *sp= newlex->sphead;
+
+    if (dbchanged && (ret= mysql_change_db(thd, olddb, 1)))
+      return ret;
+    if (sp)
     {
-      if (dbchanged && (ret= mysql_change_db(thd, olddb, 1)))
-	goto done;
-      *sphp= thd->lex->sphead;
-      (*sphp)->set_info((char *)definer, (uint)strlen(definer),
-			created, modified, &chistics, sql_mode);
-      (*sphp)->optimize();
+      delete sp;
+      newlex->sphead= NULL;
     }
-    thd->lex->sql_command= oldcmd;
-    thd->variables.sql_mode= old_sql_mode;
-    thd->variables.select_limit= select_limit;
+    ret= SP_PARSE_ERROR;
   }
-
- done:
-  if (table)
-    close_proc_table(thd, &open_tables_state_backup);
-  DBUG_RETURN(ret);
+  else
+  {
+    if (dbchanged && (ret= mysql_change_db(thd, olddb, 1)))
+      return ret;
+    *sphp= thd->lex->sphead;
+    (*sphp)->set_info((char *)definer, (uint)strlen(definer),
+		      created, modified, &chistics, sql_mode);
+    (*sphp)->optimize();
+  }
+  thd->lex->sql_command= oldcmd;
+  thd->variables.sql_mode= old_sql_mode;
+  thd->variables.select_limit= select_limit;
+  return ret;
 }
 
 
@@ -924,13 +941,8 @@
   DBUG_PRINT("enter", ("name: %*s.%*s",
 		       name->m_db.length, name->m_db.str,
 		       name->m_name.length, name->m_name.str));
-
-  if (!(sp= sp_cache_lookup(&thd->sp_proc_cache, name)) && !cache_only)
-  {
-    if (db_find_routine(thd, TYPE_ENUM_PROCEDURE, name, &sp) == SP_OK)
-      sp_cache_insert(&thd->sp_proc_cache, sp);
-  }
-
+  sp= sp_find_routine(thd, TYPE_ENUM_PROCEDURE, name, &thd->sp_proc_cache,
+                      cache_only);
   DBUG_RETURN(sp);
 }
 
@@ -1072,12 +1084,58 @@
   sp_head *sp;
   DBUG_ENTER("sp_find_function");
   DBUG_PRINT("enter", ("name: %*s", name->m_name.length, name->m_name.str));
+  sp= sp_find_routine(thd, TYPE_ENUM_FUNCTION, name, &thd->sp_func_cache, 
+                      cache_only);
+  DBUG_RETURN(sp);
+}
+
+
+static sp_head *
+sp_find_routine(THD *thd, int type, sp_name *name, sp_cache **cp,
+                bool cache_only)
+{
+  sp_head *sp;
+  DBUG_ENTER("sp_find_function");
+  DBUG_PRINT("enter", ("name: %*s", name->m_name.length, name->m_name.str));
 
-  if (!(sp= sp_cache_lookup(&thd->sp_func_cache, name)) &&
-      !cache_only)
+  if (!(sp= sp_cache_lookup(cp, name)) && !cache_only)
   {
-    if (db_find_routine(thd, TYPE_ENUM_FUNCTION, name, &sp) == SP_OK)
-      sp_cache_insert(&thd->sp_func_cache, sp);
+    if (db_find_routine(thd, type, name, &sp) == SP_OK)
+      sp_cache_insert(cp, sp);
+  }
+  else
+  if (sp && sp->m_is_invoked && thd->variables.sp_recursion_enabled)
+  {
+    sp_head *sp2= sp;
+
+    while (sp2 && sp2->m_is_invoked)
+      sp2= sp2->m_next_cached_sp;
+      
+    if (sp2 == NULL)
+    {
+      const char *returns= "";
+      char definer[HOSTNAME_LENGTH+USERNAME_LENGTH+2];
+      String retstr(64);
+      strxmov(definer, sp->m_definer_user.str, "@",
+                       sp->m_definer_host.str, NullS);
+      if (type == TYPE_ENUM_FUNCTION)
+      {
+        sp_returns_type(thd, retstr, sp);
+        returns= retstr.ptr();
+      }
+
+      if (db_load_routine(thd, type, name, &sp2,
+                          sp->m_sql_mode, sp->m_params.str, returns,
+                          sp->m_body.str, *sp->m_chistics, definer,
+                          sp->m_created, sp->m_modified) == SP_OK)
+      {
+        sp2->m_is_child= TRUE;
+        while (sp->m_next_cached_sp)
+          sp= sp->m_next_cached_sp;
+        sp->m_next_cached_sp= sp2;
+      }
+    }
+    sp= sp2;
   }
   DBUG_RETURN(sp);
 }

--- 1.166/sql/sp_head.cc	2005-08-15 22:19:53 +01:00
+++ 1.167/sql/sp_head.cc	2005-08-18 16:01:31 +01:00
@@ -316,7 +316,7 @@
   :Query_arena(&main_mem_root, INITIALIZED_FOR_SP),
    m_returns_cs(NULL), m_has_return(FALSE),
    m_simple_case(FALSE), m_multi_results(FALSE), m_in_handler(FALSE),
-   m_is_invoked(FALSE)
+   m_is_invoked(FALSE), m_is_child(FALSE), m_next_cached_sp(NULL)
 {
   extern byte *
     sp_table_key(const byte *ptr, uint *plen, my_bool first);
@@ -494,6 +494,8 @@
 sp_head::~sp_head()
 {
   destroy();
+  if (m_next_cached_sp)
+    delete m_next_cached_sp;
   if (m_thd)
     restore_thd_mem_root(m_thd);
 }

--- 1.64/sql/sp_head.h	2005-08-03 04:44:29 +01:00
+++ 1.65/sql/sp_head.h	2005-08-18 16:01:31 +01:00
@@ -132,6 +132,11 @@
   LEX_STRING m_definer_host;
   longlong m_created;
   longlong m_modified;
+
+  /* Used for tracking of routine invocations and preventing recursion. */
+  bool m_is_invoked, m_is_child;
+  sp_head *m_next_cached_sp;
+  
   /*
     Set containing names of stored routines used by this routine.
     Note that unlike elements of similar set for statement elements of this
@@ -288,9 +293,6 @@
     in prelocked mode and in non-prelocked mode.
   */
   HASH m_sptabs;
-
-  /* Used for tracking of routine invocations and preventing recursion. */
-  bool m_is_invoked;
 
   int
   execute(THD *thd);
Thread
bk commit into 5.0 tree (acurtis:1.1993) BUG#10100antony18 Aug