Adjust to the changed Query_arena constructor:

main_mem_root is moved out of class Query_arena.


sql/sp_head.cc:
  Adjust to the changed Query_arena constructor. 
  main_mem_root is moved out of class Query_arena.
sql/sp_head.h:
  main_mem_root is moved out of class Query_arena: add it to class sp_head.
sql/sql_class.cc:
  main_mem_root is moved out of class Query_arena: remove
  constructors no longer relevant, remove dead code.
sql/sql_class.h:
  main_mem_root is moved out of class Query_arena.
sql/sql_prepare.cc:
  It's better to not use main_mem_root anywhere: logically, it's not
  a public member (can't fix sp_head::make_field and Item_subselect::exec
  to make it protected)
sql/sql_select.cc:
  New Cursor constructor, which avoids unneeded memory allocation
  when initializign main_mem_root.
sql/sql_select.h:
  main_mem_root is moved out of class Query_arena.

7 files changed, 42 insertions, 81 deletions

diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index fae657a8caf..2b1ab415aec 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -310,7 +310,8 @@ sp_head::operator delete(void *ptr, size_t size)
 
 
 sp_head::sp_head()
-  :Query_arena((bool)FALSE), m_returns_cs(NULL), m_has_return(FALSE),
+  :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)
 {
   extern byte *
@@ -319,7 +320,6 @@ sp_head::sp_head()
     *sp_lex_sp_key(const byte *ptr, uint *plen, my_bool first);
   DBUG_ENTER("sp_head::sp_head");
 
-  state= INITIALIZED_FOR_SP;
   m_backpatch.empty();
   m_lex.empty();
   hash_init(&m_sptabs, system_charset_info, 0, 0, 0, sp_table_key, 0, 0);
diff --git a/sql/sp_head.h b/sql/sp_head.h
index d22515672f9..2c75a320f30 100644
--- a/sql/sp_head.h
+++ b/sql/sp_head.h
@@ -79,6 +79,7 @@ class sp_head :private Query_arena
   sp_head(const sp_head &);	/* Prevent use of these */
   void operator=(sp_head &);
 
+  MEM_ROOT main_mem_root;
 public:
 
   int m_type;			// TYPE_ENUM_FUNCTION or TYPE_ENUM_PROCEDURE
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 8b3fb87e662..e5fdeb8cc69 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -420,8 +420,6 @@ THD::~THD()
 #ifndef DBUG_OFF
   dbug_sentry= THD_SENTRY_GONE;
 #endif  
-  /* Reset stmt_backup.mem_root to not double-free memory from thd.mem_root */
-  clear_alloc_root(&stmt_backup.main_mem_root);
   DBUG_VOID_RETURN;
 }
 
@@ -1474,52 +1472,6 @@ void select_dumpvar::cleanup()
 }
 
 
-/*
-  Create arena for already constructed THD.
-
-  SYNOPSYS
-    Query_arena()
-      thd - thread for which arena is created
-
-  DESCRIPTION
-    Create arena for already existing THD using its variables as parameters
-    for memory root initialization.
-*/
-Query_arena::Query_arena(THD* thd)
-  :free_list(0), mem_root(&main_mem_root),
-   state(INITIALIZED)
-{
-  init_sql_alloc(&main_mem_root,
-                 thd->variables.query_alloc_block_size,
-                 thd->variables.query_prealloc_size);
-}
-
-
-/*
-  Create arena and optionally initialize memory root.
-
-  SYNOPSYS
-    Query_arena()
-      init_mem_root - whenever we need to initialize memory root
-
-  DESCRIPTION
-    Create arena and optionally initialize memory root with minimal
-    possible parameters.
-
-  NOTE
-    We use this constructor when arena is part of THD, but reinitialize
-    its memory root in THD::init_for_queries() before execution of real
-    statements.
-*/
-Query_arena::Query_arena(bool init_mem_root)
-  :free_list(0), mem_root(&main_mem_root),
-  state(CONVENTIONAL_EXECUTION)
-{
-  if (init_mem_root)
-    init_sql_alloc(&main_mem_root, ALLOC_ROOT_MIN_BLOCK_SIZE, 0);
-}
-
-
 Query_arena::Type Query_arena::type() const
 {
   DBUG_ASSERT(0); /* Should never be called */
@@ -1532,7 +1484,7 @@ Query_arena::Type Query_arena::type() const
 */
 
 Statement::Statement(THD *thd)
-  :Query_arena(thd),
+  :Query_arena(&main_mem_root, INITIALIZED),
   id(++thd->statement_id_counter),
   set_query_id(1),
   allow_sum_func(0),
@@ -1542,16 +1494,19 @@ Statement::Statement(THD *thd)
   cursor(0)
 {
   name.str= NULL;
+  init_sql_alloc(&main_mem_root,
+                 thd->variables.query_alloc_block_size,
+                 thd->variables.query_prealloc_size);
 }
 
 /*
-  This constructor is called when statement is a subobject of THD:
-  Some variables are initialized in THD::init due to locking problems
-  This statement object will be used to 
+  This constructor is called when Statement is a parent of THD and
+  for the backup statement. Some variables are initialized in
+  THD::init due to locking problems.
 */
 
 Statement::Statement()
-  :Query_arena((bool)TRUE),
+  :Query_arena(&main_mem_root, CONVENTIONAL_EXECUTION),
   id(0),
   set_query_id(1),
   allow_sum_func(0),                            /* initialized later */
@@ -1560,6 +1515,12 @@ Statement::Statement()
   query_length(0),                              /* in alloc_query() */
   cursor(0)
 {
+  /*
+    This is just to ensure that the destructor works correctly in
+    case of an error and the backup statement. The memory root will
+    be re-initialized in THD::init.
+  */
+  init_sql_alloc(&main_mem_root, ALLOC_ROOT_MIN_BLOCK_SIZE, 0);
 }
 
 
@@ -1632,16 +1593,6 @@ void Query_arena::restore_backup_item_arena(Query_arena *set, Query_arena *backu
 #ifndef DBUG_OFF
   backup_arena= 0;
 #endif
-#ifdef NOT_NEEDED_NOW
-  /*
-    Reset backup mem_root to avoid its freeing.
-    Since Query_arena's mem_root is freed only when it is part of Statement
-    we need this only if we use some Statement's arena as backup storage.
-    But we do this only with THD::stmt_backup and this Statement is specially
-    handled in this respect. So this code is not really needed now.
-  */
-  clear_alloc_root(&backup->mem_root);
-#endif
   DBUG_VOID_RETURN;
 }
 
@@ -1654,6 +1605,11 @@ void Query_arena::set_item_arena(Query_arena *set)
 
 Statement::~Statement()
 {
+  /*
+    We must free `main_mem_root', not `mem_root' (pointer), to work
+    correctly if this statement is used as a backup statement,
+    for which `mem_root' may point to some other statement.
+  */
   free_root(&main_mem_root, MYF(0));
 }
 
diff --git a/sql/sql_class.h b/sql/sql_class.h
index dd4b8310e51..fff789d3066 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -661,7 +661,6 @@ public:
     itself to the list on creation (see Item::Item() for details))
   */
   Item *free_list;
-  MEM_ROOT main_mem_root;
   MEM_ROOT *mem_root;                   // Pointer to current memroot
 #ifndef DBUG_OFF
   bool backup_arena;
@@ -680,21 +679,14 @@ public:
     STATEMENT, PREPARED_STATEMENT, STORED_PROCEDURE
   };
 
+  Query_arena(MEM_ROOT *mem_root_arg, enum enum_state state_arg) :
+    free_list(0), mem_root(mem_root_arg), state(state_arg)
+  {}
   /*
     This constructor is used only when Query_arena is created as
     backup storage for another instance of Query_arena.
   */
   Query_arena() {};
-  /*
-    Create arena for already constructed THD using its variables as
-    parameters for memory root initialization.
-  */
-  Query_arena(THD *thd);
-  /*
-    Create arena and optionally init memory root with minimal values.
-    Particularly used if Query_arena is part of Statement.
-  */
-  Query_arena(bool init_mem_root);
   virtual Type type() const;
   virtual ~Query_arena() {};
 
@@ -708,6 +700,7 @@ public:
   { return state == PREPARED || state == EXECUTED; }
   inline bool is_conventional() const
   { return state == CONVENTIONAL_EXECUTION; }
+
   inline gptr alloc(unsigned int size) { return alloc_root(mem_root,size); }
   inline gptr calloc(unsigned int size)
   {
@@ -757,7 +750,8 @@ class Statement: public Query_arena
   Statement(const Statement &rhs);              /* not implemented: */
   Statement &operator=(const Statement &rhs);   /* non-copyable */
 public:
-  /* FIXME: must be private */
+  /* FIXME: these must be protected */
+  MEM_ROOT main_mem_root;
   LEX     main_lex;
 
   /*
@@ -1388,7 +1382,7 @@ public:
       already changed to use this arena.
     */
     if (!current_arena->is_conventional() &&
-        mem_root != &current_arena->main_mem_root)
+        mem_root != current_arena->mem_root)
     {
       set_n_backup_item_arena(current_arena, backup);
       return current_arena;
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
index 8ff77f5a404..73bbea79760 100644
--- a/sql/sql_prepare.cc
+++ b/sql/sql_prepare.cc
@@ -2002,7 +2002,7 @@ void mysql_stmt_execute(THD *thd, char *packet, uint packet_length)
     {
       DBUG_PRINT("info",("Using READ_ONLY cursor"));
       if (!cursor &&
-          !(cursor= stmt->cursor= new (&stmt->main_mem_root) Cursor()))
+          !(cursor= stmt->cursor= new (stmt->mem_root) Cursor(thd)))
         DBUG_VOID_RETURN;
       /* If lex->result is set, mysql_execute_command will use it */
       stmt->lex->result= &cursor->result;
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index fcd0c7b4786..8f638a0b094 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -1708,7 +1708,16 @@ JOIN::cleanup()
 
 
 /************************* Cursor ******************************************/
-  
+
+Cursor::Cursor(THD *thd)
+  :Query_arena(&main_mem_root, INITIALIZED),
+   join(0), unit(0)
+{
+  /* We will overwrite it at open anyway. */
+  init_sql_alloc(&main_mem_root, ALLOC_ROOT_MIN_BLOCK_SIZE, 0);
+}
+
+
 void
 Cursor::init_from_thd(THD *thd)
 {
diff --git a/sql/sql_select.h b/sql/sql_select.h
index 5bec06fa36c..e5266944251 100644
--- a/sql/sql_select.h
+++ b/sql/sql_select.h
@@ -372,6 +372,7 @@ class JOIN :public Sql_alloc
 
 class Cursor: public Sql_alloc, public Query_arena
 {
+  MEM_ROOT main_mem_root;
   JOIN *join;
   SELECT_LEX_UNIT *unit;
 
@@ -396,7 +397,7 @@ public:
   void close();
 
   void set_unit(SELECT_LEX_UNIT *unit_arg) { unit= unit_arg; }
-  Cursor() :Query_arena(TRUE), join(0), unit(0) {}
+  Cursor(THD *thd);
   ~Cursor();
 };