- don't call JOIN::join_free(1) twice for every join in JOIN::cleanup().

The reason it happened was that both, JOIN::cleanup() and JOIN::join_free(),
went over all nested joins and called cleanup/join_free for them.
For that:
- split recursive and non-recursive parts of JOIN::cleanup() and
JOIN::join_free()
- rename JOIN::cleanup to JOIN::destroy, as it actually destroys its
  argument
- move the recursive part of JOIN::cleanup to st_select_lex::cleanup
- move the non-recursive part of JOIN::join_free to the introduced
  method JOIN::cleanup().


sql/sql_lex.h:
  Add st_select_lex::cleanup, a counterpart of st_select_lex_unit::cleanup()
sql/sql_select.cc:
  - remove two unused arguments from return_zero_rows
  - split JOIN::join_free and JOIN::cleanup to recursive and non-recursive
    parts.
  - note, the assert in JOIN::join_free _does_ fail in having.test.
    We have two options: a) propagate `full' flag to the nested joins.
    We did it before, and this patch didn't change it. If so, we 
    can end up cleaning up an uncacheable JOIN (that is, the join that
    we might need again).
    b) evaluate own 'full' flag on every level. In this case, we might
    end up with tables freed in mysql_unlock_read_tables, but not
    cleaned up properly, and this may be even worse. The test suite
    passes with both approaches, but not with the assert.
sql/sql_select.h:
  - declarations for JOIN::cleanup() and JOIN::join_free()
sql/sql_union.cc:
  Add st_select_lex::cleanup, a counterpart of st_select_lex_unit::cleanup():
  move the recursive part of JOIN::cleanup to it.

4 files changed, 115 insertions, 85 deletions

diff --git a/sql/sql_lex.h b/sql/sql_lex.h
index a9bfb6da926..5cf0b66598f 100644
--- a/sql/sql_lex.h
+++ b/sql/sql_lex.h
@@ -642,6 +642,11 @@ public:
   static void print_order(String *str, ORDER *order);
   void print_limit(THD *thd, String *str);
   void fix_prepare_information(THD *thd, Item **conds);
+  /*
+    Destroy the used execution plan (JOIN) of this subtree (this
+    SELECT_LEX and all nested SELECT_LEXes and SELECT_LEX_UNITs).
+  */
+  bool cleanup();
 };
 typedef class st_select_lex SELECT_LEX;
 
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 96a25c7919b..61234125d18 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -87,10 +87,9 @@ static void update_depend_map(JOIN *join, ORDER *order);
 static ORDER *remove_const(JOIN *join,ORDER *first_order,COND *cond,
 			   bool change_list, bool *simple_order);
 static int return_zero_rows(JOIN *join, select_result *res,TABLE_LIST *tables,
-			    List<Item> &fields, bool send_row,
-			    uint select_options, const char *info,
-			    Item *having, Procedure *proc,
-			    SELECT_LEX_UNIT *unit);
+                            List<Item> &fields, bool send_row,
+                            uint select_options, const char *info,
+                            Item *having);
 static COND *build_equal_items(THD *thd, COND *cond,
                                COND_EQUAL *inherited,
                                List<TABLE_LIST> *join_list,
@@ -1227,8 +1226,7 @@ JOIN::exec()
 			    send_row_on_empty_set(),
 			    select_options,
 			    zero_result_cause,
-			    having, procedure,
-			    unit);
+			    having);
     DBUG_VOID_RETURN;
   }
 
@@ -1437,7 +1435,7 @@ JOIN::exec()
 	DBUG_VOID_RETURN;
       }
       end_read_record(&curr_join->join_tab->read_record);
-      curr_join->const_tables= curr_join->tables; // Mark free for join_free()
+      curr_join->const_tables= curr_join->tables; // Mark free for cleanup()
       curr_join->join_tab[0].table= 0;           // Table is freed
       
       // No sum funcs anymore
@@ -1667,9 +1665,9 @@ JOIN::exec()
 */
 
 int
-JOIN::cleanup()
+JOIN::destroy()
 {
-  DBUG_ENTER("JOIN::cleanup");
+  DBUG_ENTER("JOIN::destroy");
   select_lex->join= 0;
 
   if (tmp_join)
@@ -1684,12 +1682,11 @@ JOIN::cleanup()
     }
     tmp_join->tmp_join= 0;
     tmp_table_param.copy_field=0;
-    DBUG_RETURN(tmp_join->cleanup());
+    DBUG_RETURN(tmp_join->destroy());
   }
   cond_equal= 0;
 
-  lock=0;                                     // It's faster to unlock later
-  join_free(1);
+  cleanup(1);
   if (exec_tmp_table1)
     free_tmp_table(thd, exec_tmp_table1);
   if (exec_tmp_table2)
@@ -1697,12 +1694,6 @@ JOIN::cleanup()
   delete select;
   delete_dynamic(&keyuse);
   delete procedure;
-  for (SELECT_LEX_UNIT *lex_unit= select_lex->first_inner_unit();
-       lex_unit != 0;
-       lex_unit= lex_unit->next_unit())
-  {
-    error|= lex_unit->cleanup();
-  }
   DBUG_RETURN(error);
 }
 
@@ -1885,17 +1876,14 @@ Cursor::close()
   THD *thd= join->thd;
   DBUG_ENTER("Cursor::close");
 
-  join->join_free(0);
+  /*
+    In case of UNIONs JOIN is freed inside of unit->cleanup(),
+    otherwise in select_lex->cleanup().
+  */
   if (unit)
-  {
-    /* In case of UNIONs JOIN is freed inside unit->cleanup() */
-    unit->cleanup();
-  }
+    (void) unit->cleanup();
   else
-  {
-    join->cleanup();
-    delete join;
-  }
+    (void) join->select_lex->cleanup();
   {
     /* XXX: Another hack: closing tables used in the cursor */
     DBUG_ASSERT(lock || open_tables || derived_tables);
@@ -2071,8 +2059,7 @@ err:
   if (free_join)
   {
     thd->proc_info="end";
-    err= join->cleanup();
-    delete join;
+    err= select_lex->cleanup();
     DBUG_RETURN(err || thd->net.report_error);
   }
   DBUG_RETURN(join->error);
@@ -5905,29 +5892,75 @@ void JOIN_TAB::cleanup()
 }
 
 
+void JOIN::join_free(bool full)
+{
+  SELECT_LEX_UNIT *unit;
+  SELECT_LEX *sl;
+  DBUG_ENTER("JOIN::join_free");
+
+  /*
+    Optimization: if not EXPLAIN and we are done with the JOIN,
+    free all tables.
+  */
+  full= full || (!select_lex->uncacheable && !thd->lex->subqueries &&
+                 !thd->lex->describe);
+
+  cleanup(full);
+
+  for (unit= select_lex->first_inner_unit(); unit; unit= unit->next_unit())
+    for (sl= unit->first_select_in_union(); sl; sl= sl->next_select())
+    {
+      JOIN *join= sl->join;
+      if (join)
+      {
+        /* Check that we don't occasionally clean up an uncacheable JOIN */
+#if 0
+        DBUG_ASSERT(! (!select_lex->uncacheable && sl->uncacheable));
+#endif
+        join->join_free(full);
+      }
+    }
+
+  /*
+    We are not using tables anymore
+    Unlock all tables. We may be in an INSERT .... SELECT statement.
+  */
+  if (full && lock && thd->lock && !(select_options & SELECT_NO_UNLOCK) &&
+      !select_lex->subquery_in_having &&
+      (select_lex == (thd->lex->unit.fake_select_lex ?
+                      thd->lex->unit.fake_select_lex : &thd->lex->select_lex)))
+  {
+    /*
+      TODO: unlock tables even if the join isn't top level select in the
+      tree.
+    */
+    mysql_unlock_read_tables(thd, lock);           // Don't free join->lock
+    lock= 0;
+  }
+
+  DBUG_VOID_RETURN;
+}
+
+
 /*
   Free resources of given join
 
   SYNOPSIS
-    JOIN::join_free()
+    JOIN::cleanup()
     fill - true if we should free all resources, call with full==1 should be
            last, before it this function can be called with full==0
 
   NOTE: with subquery this function definitely will be called several times,
     but even for simple query it can be called several times.
 */
-void
-JOIN::join_free(bool full)
-{
-  JOIN_TAB *tab,*end;
-  DBUG_ENTER("JOIN::join_free");
 
-  full= full || (!select_lex->uncacheable &&
-                 !thd->lex->subqueries &&
-                 !thd->lex->describe); // do not cleanup too early on EXPLAIN
+void JOIN::cleanup(bool full)
+{
+  DBUG_ENTER("JOIN::cleanup");
 
   if (table)
   {
+    JOIN_TAB *tab,*end;
     /*
       Only a sorted table may be cached.  This sorted table is always the
       first non const table in join->table
@@ -5938,16 +5971,6 @@ JOIN::join_free(bool full)
       filesort_free_buffers(table[const_tables]);
     }
 
-    for (SELECT_LEX_UNIT *unit= select_lex->first_inner_unit(); unit;
-         unit= unit->next_unit())
-    {
-      JOIN *join;
-      for (SELECT_LEX *sl= unit->first_select_in_union(); sl;
-           sl= sl->next_select())
-        if ((join= sl->join))
-          join->join_free(full);
-    }
-
     if (full)
     {
       for (tab= join_tab, end= tab+tables; tab != end; tab++)
@@ -5964,23 +5987,10 @@ JOIN::join_free(bool full)
       }
     }
   }
-
   /*
     We are not using tables anymore
     Unlock all tables. We may be in an INSERT .... SELECT statement.
   */
-  if (full && lock && thd->lock && !(select_options & SELECT_NO_UNLOCK) &&
-      !select_lex->subquery_in_having)
-  {
-    // TODO: unlock tables even if the join isn't top level select in the tree
-    if (select_lex == (thd->lex->unit.fake_select_lex ?
-                       thd->lex->unit.fake_select_lex : &thd->lex->select_lex))
-    {
-      mysql_unlock_read_tables(thd, lock);        // Don't free join->lock
-      lock=0;
-    }
-  }
-
   if (full)
   {
     group_fields.delete_elements();
@@ -6217,8 +6227,7 @@ remove_const(JOIN *join,ORDER *first_order, COND *cond,
 static int
 return_zero_rows(JOIN *join, select_result *result,TABLE_LIST *tables,
 		 List<Item> &fields, bool send_row, uint select_options,
-		 const char *info, Item *having, Procedure *procedure,
-		 SELECT_LEX_UNIT *unit)
+		 const char *info, Item *having)
 {
   DBUG_ENTER("return_zero_rows");
 
diff --git a/sql/sql_select.h b/sql/sql_select.h
index e5266944251..d88fbbfc73f 100644
--- a/sql/sql_select.h
+++ b/sql/sql_select.h
@@ -325,7 +325,7 @@ class JOIN :public Sql_alloc
   int optimize();
   int reinit();
   void exec();
-  int cleanup();
+  int destroy();
   void restore_tmp();
   bool alloc_func_list();
   bool make_sum_func_list(List<Item> &all_fields, List<Item> &send_fields,
@@ -349,7 +349,15 @@ class JOIN :public Sql_alloc
   int rollup_send_data(uint idx);
   int rollup_write_data(uint idx, TABLE *table);
   bool test_in_subselect(Item **where);
+  /*
+    Release memory and, if possible, the open tables held by this execution
+    plan (and nested plans). It's used to release some tables before
+    the end of execution in order to increase concurrency and reduce
+    memory consumption.
+  */
   void join_free(bool full);
+  /* Cleanup this JOIN, possibly for reuse */
+  void cleanup(bool full);
   void clear();
   bool save_join_tab();
   bool send_row_on_empty_set()
diff --git a/sql/sql_union.cc b/sql/sql_union.cc
index f59d7fffe85..87b67a5127a 100644
--- a/sql/sql_union.cc
+++ b/sql/sql_union.cc
@@ -553,7 +553,6 @@ bool st_select_lex_unit::exec()
 bool st_select_lex_unit::cleanup()
 {
   int error= 0;
-  JOIN *join;
   DBUG_ENTER("st_select_lex_unit::cleanup");
 
   if (cleaned)
@@ -572,29 +571,17 @@ bool st_select_lex_unit::cleanup()
   }
 
   for (SELECT_LEX *sl= first_select_in_union(); sl; sl= sl->next_select())
+    error|= sl->cleanup();
+
+  if (fake_select_lex)
   {
-    if ((join= sl->join))
-    {
-      error|= sl->join->cleanup();
-      delete join;
-    }
-    else
+    JOIN *join;
+    if ((join= fake_select_lex->join))
     {
-      // it can be DO/SET with subqueries
-      for (SELECT_LEX_UNIT *lex_unit= sl->first_inner_unit();
-	   lex_unit != 0;
-	   lex_unit= lex_unit->next_unit())
-      {
-	error|= lex_unit->cleanup();
-      }
+      join->tables_list= 0;
+      join->tables= 0;
     }
-  }
-  if (fake_select_lex && (join= fake_select_lex->join))
-  {
-    join->tables_list= 0;
-    join->tables= 0;
-    error|= join->cleanup();
-    delete join;
+    error|= fake_select_lex->cleanup();
   }
 
   DBUG_RETURN(error);
@@ -650,3 +637,24 @@ bool st_select_lex_unit::change_result(select_subselect *result,
     res= fake_select_lex->join->change_result(result);
   return (res);
 }
+
+
+bool st_select_lex::cleanup()
+{
+  bool error= FALSE;
+  DBUG_ENTER("st_select_lex::cleanup()");
+
+  if (join)
+  {
+    error|= join->destroy();
+    delete join;
+    join= 0;
+  }
+  for (SELECT_LEX_UNIT *lex_unit= first_inner_unit(); lex_unit ;
+       lex_unit= lex_unit->next_unit())
+  {
+    error|= lex_unit->cleanup();
+  }
+  DBUG_RETURN(error);
+}
+