bk commit into 5.0 tree (petr:1.1993) BUG#12297

[Petr Chardin]

Below is the list of changes that have just been committed into a local
5.0 repository of pchardin. When pchardin 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 11:23:54 petr@stripped +6 -0
  Fix for Bug#11247 Stored procedures: Function calls in long loops leak memory
      and Bug#12297 SP crashes the server if data inserted inside a lon loop
  Third commit attempt. With fixes to the issues, showed up after full rebuild and
  tests on other hosts.

  sql/sp_rcontext.h
    1.22 05/08/18 11:23:47 petr@stripped +7 -1
    Add new member to sp_rcontext class, in order to handle instructions with assignment
    and/or with nested SP processing properly.

  sql/sp_rcontext.cc
    1.32 05/08/18 11:23:47 petr@stripped +5 -5
     Now we should deal with callers_arena->free_list when we employ reuse mechanism
with callers_arena
     switched during sp_eval_func_item

  sql/sp_head.cc
    1.167 05/08/18 11:23:47 petr@stripped +211 -132
    Per-instruction arena is implemented

  mysql-test/t/sp.test
    1.138 05/08/18 11:23:47 petr@stripped +59 -0
    Added tests for bugs. Though one of them is disabled, as it fails because of the other
bug.
    It should be enabled, when bug 12297 is fixed.

  mysql-test/r/sp.result
    1.143 05/08/18 11:23:47 petr@stripped +15 -0
    fixed result file to reflect new tests

  mysql-test/r/rpl_sp.result
    1.6 05/08/18 11:23:47 petr@stripped +2 -0
    New warnings appeared in result file, as now we always create spcont in a stored
routine.
    This is correct behaviour. We swallowed some warnings, as we used thd->spcont to
check whether
    we are in the SP though we didn't set spcont in certain cases. This is fixed now.

# 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:	petr
# Host:	production.mysql.com
# Root:	/usersnfs/pchardin/mysql-5.0

--- 1.5/mysql-test/r/rpl_sp.result	2005-07-19 18:06:42 +02:00
+++ 1.6/mysql-test/r/rpl_sp.result	2005-08-18 11:23:47 +02:00
@@ -109,6 +109,7 @@
 Got one of the listed errors
 show warnings;
 Level	Code	Message
+Error	1142	INSERT command denied to user 'zedjzlcsjhd'@'localhost' for table 't1'
 Warning	1417	A routine failed and has neither NO SQL nor READS SQL DATA in its
declaration and binary logging is enabled; if non-transactional tables were updated, the
binary log will miss their changes
 call foo3();
 show warnings;
@@ -117,6 +118,7 @@
 Got one of the listed errors
 show warnings;
 Level	Code	Message
+Error	1142	INSERT command denied to user 'zedjzlcsjhd'@'localhost' for table 't1'
 Warning	1417	A routine failed and has neither NO SQL nor READS SQL DATA in its
declaration and binary logging is enabled; if non-transactional tables were updated, the
binary log will miss their changes
 alter procedure foo4 sql security invoker;
 call foo4();

--- 1.142/mysql-test/r/sp.result	2005-08-09 09:43:42 +02:00
+++ 1.143/mysql-test/r/sp.result	2005-08-18 11:23:47 +02:00
@@ -3085,4 +3085,19 @@
 id	id
 data	data
 drop function bug10055|
+drop function if exists f_bug11247|
+drop procedure if exists p_bug11247|
+create function f_bug11247(param int)
+returns int
+return param + 1|
+create procedure p_bug11247(lim int)
+begin
+declare v int default 0;
+while v < lim do
+set v= f_bug11247(v);
+end while;
+end|
+call p_bug11247(10)|
+drop function f_bug11247|
+drop procedure p_bug11247|
 drop table t1,t2;

--- 1.137/mysql-test/t/sp.test	2005-08-11 14:59:20 +02:00
+++ 1.138/mysql-test/t/sp.test	2005-08-18 11:23:47 +02:00
@@ -3871,6 +3871,65 @@
 drop function bug10055|
 
 #
+# Bug #12297 "SP crashes the server if data inserted inside a lon loop"
+# The test for memleak bug, so actually there is no way to test it
+# from the suite. The test below could be used to check SP memory
+# consumption by passing large input parameter.
+#
+
+#
+#  Note: the test is currenly disabled because of the
+#  Bug #12637: SP crashes the server if it has update query with user var
+#  & binlog is enabled.
+#
+
+--disable_warnings
+#drop procedure if exists bug12297|
+--enable_warnings
+
+#create procedure bug12297(lim int)
+#begin
+#  set @x = 0;
+#  repeat
+#   insert into t1(id,data)
+#   values('aa', @x);
+#   set @x = @x + 1;
+#  until @x >= lim
+#  end repeat;
+#end|
+
+#call bug12297(10)|
+#drop procedure bug12297|
+
+#
+# Bug #11247 "Stored procedures: Function calls in long loops leak memory"
+# One more memleak bug test. One could use this test to check that the memory
+# isn't leaking by increasing the input value for p_bug11247.
+#
+
+--disable_warnings
+drop function if exists f_bug11247|
+drop procedure if exists p_bug11247|
+--enable_warnings
+
+create function f_bug11247(param int)
+  returns int
+return param + 1|
+
+create procedure p_bug11247(lim int)
+begin
+  declare v int default 0;
+
+  while v < lim do
+    set v= f_bug11247(v);
+  end while;
+end|
+
+call p_bug11247(10)|
+drop function f_bug11247|
+drop procedure p_bug11247|
+
+#
 # BUG#NNNN: New bug synopsis
 #
 #--disable_warnings

--- 1.166/sql/sp_head.cc	2005-08-15 23:19:53 +02:00
+++ 1.167/sql/sp_head.cc	2005-08-18 11:23:47 +02:00
@@ -126,16 +126,47 @@
 }
 
 
-/* Evaluate a (presumed) func item. Always returns an item, the parameter
-** if nothing else.
+/* Macro to switch arena in sp_eval_func_item */
+#define CREATE_ON_CALLERS_ARENA(new_command, condition, backup_arena) do\
+        {\
+          if (condition) \
+            thd->set_n_backup_item_arena(thd->spcont->callers_arena,\
+                                         backup_arena);\
+          new_command;\
+          if (condition)\
+            thd->restore_backup_item_arena(thd->spcont->callers_arena,\
+                                           &backup_current_arena);\
+        } while(0)
+
+/*
+  Evaluate an item and store it in the returned item
+
+  SYNOPSIS
+    sp_eval_func_item()
+      name                  - current thread object
+      it_addr               - pointer to the item to evaluate
+      type                  - type of the item we evaluating
+      reuse                 - used if we would like to reuse existing item
+                              instead of allocation of the new one
+      use_callers_arena     - TRUE if we want to use caller's arena
+                              rather then current one.
+  DESCRIPTION
+   We use this function to evaluate result for stored functions
+   and stored procedure parameters. It is also used to evaluate and
+   (re) allocate variables.
+
+  RETURN VALUES
+    Evaluated item is returned
 */
+
 Item *
 sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type,
-		  Item *reuse)
+		  Item *reuse, bool use_callers_arena)
 {
   DBUG_ENTER("sp_eval_func_item");
   Item *it= sp_prepare_func_item(thd, it_addr);
   uint rsize;
+  Query_arena backup_current_arena;
   DBUG_PRINT("info", ("type: %d", type));
 
   if (!it)
@@ -145,91 +176,100 @@
 
   /* QQ How do we do this? Is there some better way? */
   if (type == MYSQL_TYPE_NULL)
-    it= new(reuse, &rsize) Item_null();
-  else
-  {
-    switch (sp_map_result_type(type)) {
-    case INT_RESULT:
-      {
-	longlong i= it->val_int();
+    goto return_null_item;
 
-	if (it->null_value)
-	{
-	  DBUG_PRINT("info", ("INT_RESULT: null"));
-	  it= new(reuse, &rsize) Item_null();
-	}
-	else
-	{
-	  DBUG_PRINT("info", ("INT_RESULT: %d", i));
-          it= new(reuse, &rsize) Item_int(i);
-	}
-	break;
+  switch (sp_map_result_type(type)) {
+  case INT_RESULT:
+    {
+      longlong i= it->val_int();
+
+      if (it->null_value)
+      {
+        DBUG_PRINT("info", ("INT_RESULT: null"));
+        goto return_null_item;
       }
-    case REAL_RESULT:
+      else
       {
-	double d= it->val_real();
+        DBUG_PRINT("info", ("INT_RESULT: %d", i));
+        CREATE_ON_CALLERS_ARENA(it= new(reuse, &rsize) Item_int(i),
+                          use_callers_arena, &backup_current_arena);
+      }
+      break;
+    }
+  case REAL_RESULT:
+    {
+      double d= it->val_real();
 
-	if (it->null_value)
-	{
-	  DBUG_PRINT("info", ("REAL_RESULT: null"));
-	  it= new(reuse, &rsize) Item_null();
-	}
-	else
-	{
-	  /* There's some difference between Item::new_item() and the
-	   * constructor; the former crashes, the latter works... weird. */
-	  uint8 decimals= it->decimals;
-	  uint32 max_length= it->max_length;
-	  DBUG_PRINT("info", ("REAL_RESULT: %g", d));
-          it= new(reuse, &rsize) Item_float(d);
-	  it->decimals= decimals;
-	  it->max_length= max_length;
-	}
-	break;
+      if (it->null_value)
+      {
+        DBUG_PRINT("info", ("REAL_RESULT: null"));
+        goto return_null_item;
       }
-    case DECIMAL_RESULT:
+      else
       {
-        my_decimal value, *val= it->val_decimal(&value);
-        if (it->null_value)
-          it= new(reuse, &rsize) Item_null();
-        else
-          it= new(reuse, &rsize) Item_decimal(val);
+        /* There's some difference between Item::new_item() and the
+         * constructor; the former crashes, the latter works... weird. */
+        uint8 decimals= it->decimals;
+        uint32 max_length= it->max_length;
+        DBUG_PRINT("info", ("REAL_RESULT: %g", d));
+        CREATE_ON_CALLERS_ARENA(it= new(reuse, &rsize) Item_float(d),
+                          use_callers_arena, &backup_current_arena);
+        it->decimals= decimals;
+        it->max_length= max_length;
+      }
+      break;
+    }
+  case DECIMAL_RESULT:
+    {
+      my_decimal value, *val= it->val_decimal(&value);
+      if (it->null_value)
+        goto return_null_item;
+      else
+        CREATE_ON_CALLERS_ARENA(it= new(reuse, &rsize) Item_decimal(val),
+                          use_callers_arena, &backup_current_arena);
 #ifndef DBUG_OFF
-        char dbug_buff[DECIMAL_MAX_STR_LENGTH+1];
-        DBUG_PRINT("info", ("DECIMAL_RESULT: %s", dbug_decimal_as_string(dbug_buff,
val)));
+      char dbug_buff[DECIMAL_MAX_STR_LENGTH+1];
+      DBUG_PRINT("info", ("DECIMAL_RESULT: %s", dbug_decimal_as_string(dbug_buff, val)));
 #endif
-        break;
+      break;
+    }
+  case STRING_RESULT:
+    {
+      char buffer[MAX_FIELD_WIDTH];
+      String tmp(buffer, sizeof(buffer), it->collation.collation);
+      String *s= it->val_str(&tmp);
+
+      if (it->null_value)
+      {
+        DBUG_PRINT("info", ("default result: null"));
+        goto return_null_item;
       }
-    case STRING_RESULT:
+      else
       {
-	char buffer[MAX_FIELD_WIDTH];
-	String tmp(buffer, sizeof(buffer), it->collation.collation);
-	String *s= it->val_str(&tmp);
-
-	if (it->null_value)
-	{
-	  DBUG_PRINT("info", ("default result: null"));
-	  it= new(reuse, &rsize) Item_null();
-	}
-	else
-	{
-	  DBUG_PRINT("info",("default result: %*s",
-                             s->length(), s->c_ptr_quick()));
-	  it= new(reuse, &rsize) Item_string(thd->strmake(s->ptr(),
-							  s->length()),
-					     s->length(),
-					     it->collation.collation);
-	}
-	break;
+        DBUG_PRINT("info",("default result: %*s",
+                           s->length(), s->c_ptr_quick()));
+        CREATE_ON_CALLERS_ARENA(it= new(reuse, &rsize)
+                              Item_string(thd->strmake(s->ptr(),
+                                          s->length()), s->length(),
+                                          it->collation.collation),
+                          use_callers_arena, &backup_current_arena);
       }
-    case ROW_RESULT:
-    default:
-      DBUG_ASSERT(0);
+      break;
     }
+  case ROW_RESULT:
+  default:
+    DBUG_ASSERT(0);
   }
   it->rsize= rsize;
 
   DBUG_RETURN(it);
+
+return_null_item:
+  CREATE_ON_CALLERS_ARENA(it= new(reuse, &rsize) Item_null(),
+                    use_callers_arena, &backup_current_arena);
+  it->rsize= rsize;
+
+  DBUG_RETURN(it);
 }
 
 
@@ -545,23 +585,14 @@
 sp_head::make_field(uint max_length, const char *name, TABLE *dummy)
 {
   Field *field;
-  MEM_ROOT *tmp_mem_root;
-  THD *thd;
   DBUG_ENTER("sp_head::make_field");
 
-  thd= current_thd;
-  tmp_mem_root= thd->mem_root;
-  if (thd->spcont && thd->spcont->callers_mem_root)
-    thd->mem_root= thd->spcont->callers_mem_root;
-  else
-    thd->mem_root= &thd->main_mem_root;
   field= ::make_field((char *)0,
 		!m_returns_len ? max_length : m_returns_len, 
 		(uchar *)"", 0, m_returns_pack, m_returns, m_returns_cs,
 		(enum Field::geometry_type)0, Field::NONE, 
 		m_returns_typelib,
 		name ? name : (const char *)m_name.str, dummy);
-  thd->mem_root= tmp_mem_root;
   DBUG_RETURN(field);
 }
 
@@ -576,12 +607,20 @@
   uint ip= 0;
   ulong save_sql_mode;
   Query_arena *old_arena;
+  /* per-instruction arena */
+  MEM_ROOT execute_mem_root;
+  Query_arena execute_arena(&execute_mem_root, INITIALIZED_FOR_SP),
+              execute_backup_arena;
   query_id_t old_query_id;
   TABLE *old_derived_tables;
   LEX *old_lex;
   Item_change_list old_change_list;
   String old_packet;
 
+  /* init per-instruction memroot */
+  init_alloc_root(&execute_mem_root, MEM_ROOT_BLOCK_SIZE, 0);
+
+
   /* Use some extra margin for possible SP recursion and functions */
   if (check_stack_overrun(thd, 4*STACK_MIN_SIZE, olddb))
   {
@@ -650,6 +689,18 @@
   */
   old_packet.swap(thd->packet);
 
+  /*
+    Switch to per-instruction arena here. We can do it since we cleanup
+    arena after every instruction.
+  */
+  thd->set_n_backup_item_arena(&execute_arena, &execute_backup_arena);
+
+  /*
+    Save callers arena in order to store instruction results and out
+    parameters in it later during sp_eval_func_item()
+  */
+  thd->spcont->callers_arena= &execute_backup_arena;
+
   do
   {
     sp_instr *i;
@@ -670,6 +721,7 @@
     */
     thd->current_arena= i;
     ret= i->execute(thd, &ip);
+
     /*
       If this SP instruction have sent eof, it has caused no_send_error to be
       set. Clear it back to allow the next instruction to send error. (multi-
@@ -680,6 +732,10 @@
       cleanup_items(i->free_list);
     i->state= Query_arena::EXECUTED;
 
+    /* we should cleanup free_list and memroot, used by instruction */
+    thd->free_items();
+    free_root(&execute_mem_root, MYF(0));
+
     /*
       Check if an exception has occurred and a handler has been found
       Note: We havo to check even if ret==0, since warnings (and some
@@ -696,8 +752,10 @@
       case SP_HANDLER_NONE:
 	break;
       case SP_HANDLER_CONTINUE:
-	ctx->save_variables(hf);
-	ctx->push_hstack(ip);
+        thd->restore_backup_item_arena(&execute_arena, &execute_backup_arena);
+        ctx->save_variables(hf);
+        thd->set_n_backup_item_arena(&execute_arena, &execute_backup_arena);
+        ctx->push_hstack(ip);
         // Fall through
       default:
 	ip= hip;
@@ -711,6 +769,9 @@
     }
   } while (ret == 0 && !thd->killed);
 
+  thd->restore_backup_item_arena(&execute_arena, &execute_backup_arena);
+
+
   /* Restore all saved */
   old_packet.swap(thd->packet);
   DBUG_ASSERT(thd->change_list.is_empty());
@@ -757,8 +818,6 @@
   sp_rcontext *nctx = NULL;
   uint i;
   int ret;
-  MEM_ROOT call_mem_root;
-  Query_arena call_arena(&call_mem_root, INITIALIZED_FOR_SP), backup_arena;
 
   if (argcount != params)
   {
@@ -771,16 +830,13 @@
     DBUG_RETURN(-1);
   }
 
-  init_alloc_root(&call_mem_root, MEM_ROOT_BLOCK_SIZE, 0);
-
 
   // QQ Should have some error checking here? (types, etc...)
   nctx= new sp_rcontext(csize, hmax, cmax);
-  nctx->callers_mem_root= thd->mem_root;
   for (i= 0 ; i < argcount ; i++)
   {
     sp_pvar_t *pvar = m_pcont->find_pvar(i);
-    Item *it= sp_eval_func_item(thd, argp++, pvar->type, NULL);
+    Item *it= sp_eval_func_item(thd, argp++, pvar->type, NULL, FALSE);
 
     if (it)
       nctx->push_item(it);
@@ -804,26 +860,16 @@
     }
   }
   thd->spcont= nctx;
-  thd->set_n_backup_item_arena(&call_arena, &backup_arena);
-  /* mem_root was moved to backup_arena */
-  DBUG_ASSERT(nctx->callers_mem_root == backup_arena.mem_root);
 
   ret= execute(thd);
 
-  /*
-    Partially restore context now.
-    We still need the call mem root and free list for processing
-    of the result.
-  */
-  thd->restore_backup_item_arena(&call_arena, &backup_arena);
-
   if (m_type == TYPE_ENUM_FUNCTION && ret == 0)
   {
     /* We need result only in function but not in trigger */
     Item *it= nctx->get_result();
 
     if (it)
-      *resp= sp_eval_func_item(thd, &it, m_returns, NULL);
+      *resp= sp_eval_func_item(thd, &it, m_returns, NULL, FALSE);
     else
     {
       my_error(ER_SP_NORETURNEND, MYF(0), m_name.str);
@@ -832,12 +878,9 @@
   }
 
   nctx->pop_all_cursors();	// To avoid memory leaks after an error
+  delete nctx;
   thd->spcont= octx;
 
-  // Now get rid of the rest of the callee context
-  call_arena.free_items();
-  free_root(&call_mem_root, MYF(0));
-
   DBUG_RETURN(ret);
 }
 
@@ -866,9 +909,7 @@
   uint cmax = m_pcont->max_cursors();
   sp_rcontext *octx = thd->spcont;
   sp_rcontext *nctx = NULL;
-  my_bool tmp_octx = FALSE;	// True if we have allocated a temporary octx
-  MEM_ROOT call_mem_root;
-  Query_arena call_arena(&call_mem_root, INITIALIZED_FOR_SP), backup_arena;
+  my_bool is_tmp_octx = FALSE;  // True if we have allocated a temporary octx
 
   if (args->elements != params)
   {
@@ -877,7 +918,17 @@
     DBUG_RETURN(-1);
   }
 
-  init_alloc_root(&call_mem_root, MEM_ROOT_BLOCK_SIZE, 0);
+  if (! octx)
+  {				// Create a temporary old context
+    octx= new sp_rcontext(csize, hmax, cmax);
+    is_tmp_octx= TRUE;
+    thd->spcont= octx;
+
+    /* set callers_arena to thd, for upper-level function to work */
+    thd->spcont->callers_arena= thd;
+  }
+
+  nctx= new sp_rcontext(csize, hmax, cmax);
 
   if (csize > 0 || hmax > 0 || cmax > 0)
   {
@@ -886,12 +937,6 @@
     List_iterator<Item> li(*args);
     Item *it;
 
-    nctx= new sp_rcontext(csize, hmax, cmax);
-    if (! octx)
-    {				// Create a temporary old context
-      octx= new sp_rcontext(csize, hmax, cmax);
-      tmp_octx= TRUE;
-    }
 
     /* Evaluate SP arguments (i.e. get the values passed as parameters) */
     // QQ: Should do type checking?
@@ -919,7 +964,7 @@
 	}
 	else
 	{
-	  Item *it2= sp_eval_func_item(thd, li.ref(), pvar->type, NULL);
+	  Item *it2= sp_eval_func_item(thd, li.ref(), pvar->type, NULL, FALSE);
 
 	  if (it2)
 	    nctx->push_item(it2); // IN or INOUT
@@ -952,15 +997,20 @@
 	nit= new Item_null();
       nctx->push_item(nit);
     }
-    thd->spcont= nctx;
   }
 
+  thd->spcont= nctx;
+
   if (! ret)
-  {
-    thd->set_n_backup_item_arena(&call_arena, &backup_arena);
     ret= execute(thd);
-    thd->restore_backup_item_arena(&call_arena, &backup_arena);
-  }
+
+  /*
+    In the case when we weren't able to employ reuse mechanism for
+    OUT/INOUT paranmeters, we should reallocate memory. This
+    allocation should be done on the arena which will live through
+    all execution of calling routine.
+  */
+  thd->spcont->callers_arena= octx->callers_arena;
 
   if (!ret && csize > 0)
   {
@@ -985,12 +1035,20 @@
 	  Item *val= nctx->get_item(i);
 	  Item *orig= octx->get_item(offset);
 	  Item *o_item_next;
-	  Item *o_free_list= thd->free_list;
+          /* we'll use callers_arena in sp_eval_func_item */
+	  Item *o_free_list= thd->spcont->callers_arena->free_list;
+
 	  LINT_INIT(o_item_next);
 
 	  if (orig)
 	    o_item_next= orig->next;
-	  copy= sp_eval_func_item(thd, &val, pvar->type, orig); // Copy
+
+          /*
+            We might need to allocate new item if we weren't able to
+            employ reuse mechanism. Then we should do it on the callers arena.
+          */
+	  copy= sp_eval_func_item(thd, &val, pvar->type, orig, TRUE); // Copy
+
 	  if (!copy)
 	  {
 	    ret= -1;
@@ -1004,7 +1062,7 @@
               A reused item slot, where the constructor put it in the
               free_list, so we have to restore the list.
             */
-	    thd->free_list= o_free_list;
+	    thd->spcont->callers_arena->free_list= o_free_list;
 	    copy->next= o_item_next;
 	  }
 	}
@@ -1032,16 +1090,15 @@
     }
   }
 
-  if (tmp_octx)
+  if (is_tmp_octx)
+  {
+    delete octx;                                /* call destructor */
     octx= NULL;
-  if (nctx)
-    nctx->pop_all_cursors();	// To avoid memory leaks after an error
-  thd->spcont= octx;
+  }
 
-  // Now get rid of the rest of the callee context
-  call_arena.free_items();
-  thd->lex->unit.cleanup();
-  free_root(&call_mem_root, MYF(0));
+  nctx->pop_all_cursors();	// To avoid memory leaks after an error
+  delete nctx;
+  thd->spcont= octx;
 
   DBUG_RETURN(ret);
 }
@@ -1894,7 +1951,7 @@
   Item *it;
   int res;
 
-  it= sp_eval_func_item(thd, &m_value, m_type, NULL);
+  it= sp_eval_func_item(thd, &m_value, m_type, NULL, TRUE);
   if (! it)
     res= -1;
   else
@@ -2044,9 +2101,23 @@
 int
 sp_instr_cpush::execute(THD *thd, uint *nextp)
 {
+  Query_arena backup_current_arena;
   DBUG_ENTER("sp_instr_cpush::execute");
+
+  /*
+    We should create cursors in the callers arena, as
+    it could be (and usually is) used in several instructions.
+  */
+  thd->set_n_backup_item_arena(thd->spcont->callers_arena,
+                               &backup_current_arena);
+
   thd->spcont->push_cursor(&m_lex_keeper, this);
+
+  thd->restore_backup_item_arena(thd->spcont->callers_arena,
+                                 &backup_current_arena);
+
   *nextp= m_ip+1;
+
   DBUG_RETURN(0);
 }
 
@@ -2197,12 +2268,20 @@
 {
   sp_cursor *c= thd->spcont->get_cursor(m_cursor);
   int res;
+  Query_arena backup_current_arena;
   DBUG_ENTER("sp_instr_cfetch::execute");
 
   if (! c)
     res= -1;
   else
+  {
+    thd->set_n_backup_item_arena(thd->spcont->callers_arena,
+                                 &backup_current_arena);
     res= c->fetch(thd, &m_varlist);
+    thd->restore_backup_item_arena(thd->spcont->callers_arena,
+                                   &backup_current_arena);
+  }
+
   *nextp= m_ip+1;
   DBUG_RETURN(res);
 }

--- 1.31/sql/sp_rcontext.cc	2005-06-30 18:07:01 +02:00
+++ 1.32/sql/sp_rcontext.cc	2005-08-18 11:23:47 +02:00
@@ -32,7 +32,6 @@
   : m_count(0), m_fsize(fsize), m_result(NULL), m_hcount(0), m_hsp(0),
     m_hfound(-1), m_ccount(0)
 {
-  callers_mem_root= NULL;
   in_handler= FALSE;
   m_frame= (Item **)sql_alloc(fsize * sizeof(Item*));
   m_handler= (sp_handler_t *)sql_alloc(hmax * sizeof(sp_handler_t));
@@ -47,17 +46,18 @@
 			   enum_field_types type)
 {
   extern Item *sp_eval_func_item(THD *thd, Item **it, enum_field_types type,
-				 Item *reuse);
+				 Item *reuse, bool use_callers_arena);
   Item *it;
   Item *reuse_it;
   Item *old_item_next;
-  Item *old_free_list= thd->free_list;
+  /* sp_eval_func_item will use callers_arena */
+  Item *old_free_list= thd->spcont->callers_arena->free_list;
   int res;
   LINT_INIT(old_item_next);
 
   if ((reuse_it= get_item(idx)))
     old_item_next= reuse_it->next;
-  it= sp_eval_func_item(thd, item_addr, type, reuse_it);
+  it= sp_eval_func_item(thd, item_addr, type, reuse_it, TRUE);
   if (! it)
     res= -1;
   else
@@ -67,7 +67,7 @@
     {
       // A reused item slot, where the constructor put it in the free_list,
       // so we have to restore the list.
-      thd->free_list= old_free_list;
+      thd->spcont->callers_arena->free_list= old_free_list;
       it->next= old_item_next;
     }
     set_item(idx, it);

--- 1.21/sql/sp_rcontext.h	2005-06-30 18:07:01 +02:00
+++ 1.22/sql/sp_rcontext.h	2005-08-18 11:23:47 +02:00
@@ -48,8 +48,14 @@
 
  public:
 
-  MEM_ROOT *callers_mem_root;	// Used to store result fields
   bool in_handler;
+  /*
+    Arena used to (re) allocate items on . E.g. reallocate INOUT/OUT
+    SP parameters when they don't fit into prealloced items. This
+    is common situation with String items. It is used mainly in
+    sp_eval_func_item().
+  */
+  Query_arena *callers_arena;
 
   sp_rcontext(uint fsize, uint hmax, uint cmax);