- implement inheritance of sp_instr: public Query_arena.

  We need every instruction to have its own arena, because we want to
  track instruction's state (INITIALIZED_FOR_SP -> EXECUTED). Because of 
  `if' statements and other conditional instructions used in stored 
  procedures, not every instruction of a stored procedure gets executed 
  during the first (or even subsequent) execution of the procedure. 
  So it's better if we track the execution state of every instruction 
  independently.
  All instructions of a given procedure now also share sp_head's 
  mem_root, but keep their own free_list.
  This simplifies juggling with free Item lists in sp_head::execute.
- free_items() moved to be a member of Query_arena. 
- logic of 'backup_arena' debug member of Query_arena has been
  changed to support
  multi-backups. Until now, TRUE 'backup_arena' meant that there is
  exactly one active backup of the THD arena. Now it means simply that
  the arena is used for backup, so that we can't accidentally overwrite an 
  existing backup. This allows doing multiple backups, e.g. in
  sp_head::execute and Cursor::fetch, when THD arena is already backed up
  but we want to set yet another arena (usually the 'permanent' arena,
  to save permanent transformations/optimizations of a parsed tree).

sql/sp_head.cc

@@ -509,7 +509,7 @@ sp_head::destroy()
     delete i;
   delete_dynamic(&m_instr);
   m_pcont->destroy();
-  free_items(free_list);
+  free_items();
 
   /*
     If we have non-empty LEX stack then we just came out of parser with
@@ -596,7 +596,6 @@ sp_head::execute(THD *thd)
     ctx->clear_handler();
   thd->query_error= 0;
   old_arena= thd->current_arena;
-  thd->current_arena= this;
 
   /*
     We have to save/restore this info when we are changing call level to
@@ -636,23 +635,18 @@ sp_head::execute(THD *thd)
       break;
     DBUG_PRINT("execute", ("Instruction %u", ip));
     thd->set_time();		// Make current_time() et al work
-    {
     /*
-        We have to substitute free_list of executing statement to
-        current_arena to store there all new items created during execution
-        (for example '*' expanding, or items made during permanent subquery
-        transformation)
-        Note: Every statement have to have all its items listed in free_list
-        for correct cleaning them up
+      We have to set thd->current_arena before executing the instruction
+      to store in the instruction free_list all new items, created
+      during the first execution (for example expanding of '*' or the
+      items made during other permanent subquery transformations).
     */
-      Item *save_free_list= thd->current_arena->free_list;
-      thd->current_arena->free_list= i->free_list;
+    thd->current_arena= i;
     ret= i->execute(thd, &ip);
-      i->free_list= thd->current_arena->free_list;
-      thd->current_arena->free_list= save_free_list;
-    }
     if (i->free_list)
       cleanup_items(i->free_list);
+    i->state= Query_arena::EXECUTED;
+
     // 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
     //       errors don't return a non-zero value.
@@ -694,7 +688,6 @@ sp_head::execute(THD *thd)
   DBUG_ASSERT(!thd->derived_tables);
   thd->derived_tables= old_derived_tables;
 
-  cleanup_items(thd->current_arena->free_list);
   thd->current_arena= old_arena;
   state= EXECUTED;
 
@@ -728,8 +721,8 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
   sp_rcontext *nctx = NULL;
   uint i;
   int ret;
-  MEM_ROOT *old_mem_root, call_mem_root;
-  Item *old_free_list, *call_free_list;
+  MEM_ROOT call_mem_root;
+  Query_arena call_arena(&call_mem_root, INITIALIZED_FOR_SP), backup_arena;
 
   if (argcount != params)
   {
@@ -741,14 +734,12 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
   }
 
   init_alloc_root(&call_mem_root, MEM_ROOT_BLOCK_SIZE, 0);
-  old_mem_root= thd->mem_root;
-  thd->mem_root= &call_mem_root;
-  old_free_list= thd->free_list; // Keep the old list
-  thd->free_list= NULL;	// Start a new one
+
+  thd->set_n_backup_item_arena(&call_arena, &backup_arena);
 
   // QQ Should have some error checking here? (types, etc...)
   nctx= new sp_rcontext(csize, hmax, cmax);
-  nctx->callers_mem_root= old_mem_root;
+  nctx->callers_mem_root= backup_arena.mem_root;
   for (i= 0 ; i < argcount ; i++)
   {
     sp_pvar_t *pvar = m_pcont->find_pvar(i);
@@ -780,9 +771,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
   // Partially restore context now.
   // We still need the call mem root and free list for processing
   // of the result.
-  call_free_list= thd->free_list;
-  thd->free_list= old_free_list;
-  thd->mem_root= old_mem_root;
+  thd->restore_backup_item_arena(&call_arena, &backup_arena);
 
   if (m_type == TYPE_ENUM_FUNCTION && ret == 0)
   {
@@ -802,8 +791,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
   thd->spcont= octx;
 
   // Now get rid of the rest of the callee context
-  cleanup_items(call_free_list);
-  free_items(call_free_list);
+  call_arena.free_items();
   free_root(&call_mem_root, MYF(0));
 
   DBUG_RETURN(ret);
@@ -835,8 +823,8 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
   sp_rcontext *octx = thd->spcont;
   sp_rcontext *nctx = NULL;
   my_bool tmp_octx = FALSE;	// True if we have allocated a temporary octx
-  MEM_ROOT *old_mem_root, call_mem_root;
-  Item *old_free_list, *call_free_list;
+  MEM_ROOT call_mem_root;
+  Query_arena call_arena(&call_mem_root, INITIALIZED_FOR_SP), backup_arena;
 
   if (args->elements != params)
   {
@@ -846,10 +834,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
   }
 
   init_alloc_root(&call_mem_root, MEM_ROOT_BLOCK_SIZE, 0);
-  old_mem_root= thd->mem_root;
-  thd->mem_root= &call_mem_root;
-  old_free_list= thd->free_list; // Keep the old list
-  thd->free_list= NULL;	// Start a new one
+  thd->set_n_backup_item_arena(&call_arena, &backup_arena);
 
   if (csize > 0 || hmax > 0 || cmax > 0)
   {
@@ -919,9 +904,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
   // Partially restore context now.
   // We still need the call mem root and free list for processing
   // of out parameters.
-  call_free_list= thd->free_list;
-  thd->free_list= old_free_list;
-  thd->mem_root= old_mem_root;
+  thd->restore_backup_item_arena(&call_arena, &backup_arena);
 
   if (!ret && csize > 0)
   {
@@ -996,8 +979,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
   thd->spcont= octx;
 
   // Now get rid of the rest of the callee context
-  cleanup_items(call_free_list);
-  free_items(call_free_list);
+  call_arena.free_items();
   thd->lex->unit.cleanup();
   free_root(&call_mem_root, MYF(0));
 
@@ -1291,6 +1273,13 @@ void sp_head::add_instr(sp_instr *instr)
 {
   instr->free_list= m_thd->free_list;
   m_thd->free_list= 0;
+  /*
+    Memory root of every instruction is designated for permanent
+    transformations (optimizations) made on the parsed tree during
+    the first execution. It points to the memory root of the
+    entire stored procedure, as their life span is equal.
+  */
+  instr->mem_root= &main_mem_root;
   insert_dynamic(&m_instr, (gptr)&instr);
 }
 

sql/sp_head.h

@@ -274,7 +274,7 @@ class sp_head :private Query_arena
 // "Instructions"...
 //
 
-class sp_instr : public Sql_alloc
+class sp_instr :public Query_arena, public Sql_alloc
 {
   sp_instr(const sp_instr &);	/* Prevent use of these */
   void operator=(sp_instr &);
@@ -282,17 +282,16 @@ class sp_instr : public Sql_alloc
 public:
 
   uint marked;
-  Item *free_list;              // My Items
   uint m_ip;			// My index
   sp_pcontext *m_ctx;		// My parse context
 
   // Should give each a name or type code for debugging purposes?
   sp_instr(uint ip, sp_pcontext *ctx)
-    :Sql_alloc(), marked(0), free_list(0), m_ip(ip), m_ctx(ctx)
+    :Query_arena(0, INITIALIZED_FOR_SP), marked(0), m_ip(ip), m_ctx(ctx)
   {}
 
   virtual ~sp_instr()
-  { free_items(free_list); }
+  { free_items(); }
 
   // Execute this instrution. '*nextp' will be set to the index of the next
   // instruction to execute. (For most instruction this will be the

sql/sql_class.cc

@@ -171,9 +171,6 @@ THD::THD()
    spcont(NULL)
 {
   current_arena= this;
-#ifndef DBUG_OFF
-  backup_arena= 0;
-#endif
   host= user= priv_user= db= ip= 0;
   catalog= (char*)"std"; // the only catalog we have for now
   host_or_ip= "connecting host";
@@ -528,7 +525,7 @@ void THD::cleanup_after_query()
     next_insert_id= 0;
   }
   /* Free Items that were created during this execution */
-  free_items(free_list);
+  free_items();
   /*
     In the rest of code we assume that free_list never points to garbage:
     Keep this predicate true.
@@ -1485,6 +1482,21 @@ Query_arena::Type Query_arena::type() const
 }
 
 
+void Query_arena::free_items()
+{
+  Item *next;
+  DBUG_ENTER("Query_arena::free_items");
+  /* This works because items are allocated with sql_alloc() */
+  for (; free_list; free_list= next)
+  {
+    next= free_list->next;
+    free_list->delete_self();
+  }
+  /* Postcondition: free_list is 0 */
+  DBUG_VOID_RETURN;
+}
+
+
 /*
   Statement functions 
 */
@@ -1556,11 +1568,11 @@ void THD::end_statement()
 void Query_arena::set_n_backup_item_arena(Query_arena *set, Query_arena *backup)
 {
   DBUG_ENTER("Query_arena::set_n_backup_item_arena");
-  DBUG_ASSERT(backup_arena == 0);
+  DBUG_ASSERT(backup->is_backup_arena == FALSE);
   backup->set_item_arena(this);
   set_item_arena(set);
 #ifndef DBUG_OFF
-  backup_arena= 1;
+  backup->is_backup_arena= TRUE;
 #endif
   DBUG_VOID_RETURN;
 }
@@ -1569,10 +1581,11 @@ void Query_arena::set_n_backup_item_arena(Query_arena *set, Query_arena *backup)
 void Query_arena::restore_backup_item_arena(Query_arena *set, Query_arena *backup)
 {
   DBUG_ENTER("Query_arena::restore_backup_item_arena");
+  DBUG_ASSERT(backup->is_backup_arena);
   set->set_item_arena(this);
   set_item_arena(backup);
 #ifndef DBUG_OFF
-  backup_arena= 0;
+  backup->is_backup_arena= FALSE;
 #endif
   DBUG_VOID_RETURN;
 }

sql/sql_class.h

@@ -663,7 +663,10 @@ class Query_arena
   Item *free_list;
   MEM_ROOT *mem_root;                   // Pointer to current memroot
 #ifndef DBUG_OFF
-  bool backup_arena;
+  bool is_backup_arena; /* True if this arena is used for backup. */
+#define INIT_ARENA_DBUG_INFO is_backup_arena= 0
+#else 
+#define INIT_ARENA_DBUG_INFO  
 #endif
   enum enum_state
   {
@@ -681,12 +684,14 @@ class Query_arena
 
   Query_arena(MEM_ROOT *mem_root_arg, enum enum_state state_arg) :
     free_list(0), mem_root(mem_root_arg), state(state_arg)
-  {}
+  { INIT_ARENA_DBUG_INFO; }
   /*
     This constructor is used only when Query_arena is created as
     backup storage for another instance of Query_arena.
   */
-  Query_arena() {};
+  Query_arena() { INIT_ARENA_DBUG_INFO; }
+
+#undef INIT_ARENA_DBUG_INFO
   virtual Type type() const;
   virtual ~Query_arena() {};
 
@@ -726,6 +731,8 @@ class Query_arena
   void set_n_backup_item_arena(Query_arena *set, Query_arena *backup);
   void restore_backup_item_arena(Query_arena *set, Query_arena *backup);
   void set_item_arena(Query_arena *set);
+
+  void free_items();
 };
 
 

sql/sql_prepare.cc

@@ -2429,7 +2429,7 @@ Prepared_statement::~Prepared_statement()
 {
   if (cursor)
     cursor->Cursor::~Cursor();
-  free_items(free_list);
+  free_items();
   delete lex->result;
 }
 

sql/sql_select.cc

@@ -1914,8 +1914,7 @@ Cursor::close()
   }
   join= 0;
   unit= 0;
-  free_items(free_list);
-  free_list= 0;
+  free_items();
   /*
     Must be last, as some memory might be allocated for free purposes,
     like in free_tmp_table() (TODO: fix this issue)