- 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

@@ -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;
 

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,
+                            List<Item> &fields, bool send_row,
-			    uint select_options, const char *info,
+                            uint select_options, const char *info,
-			    Item *having, Procedure *proc,
+                            Item *having);
-			    SELECT_LEX_UNIT *unit);
 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,
+			    having);
-			    unit);
     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
+  cleanup(1);
-  join_free(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)
-  {
+    (void) unit->cleanup();
-    /* In case of UNIONs JOIN is freed inside unit->cleanup() */
-    unit->cleanup();
-  }
   else
-  {
+    (void) join->select_lex->cleanup();
-    join->cleanup();
-    delete join;
-  }
   {
     /* 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();
+    err= select_lex->cleanup();
-    delete join;
     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 &&
+void JOIN::cleanup(bool full)
-                 !thd->lex->subqueries &&
+{
-                 !thd->lex->describe); // do not cleanup too early on EXPLAIN
+  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,
+		 const char *info, Item *having)
-		 SELECT_LEX_UNIT *unit)
 {
   DBUG_ENTER("return_zero_rows");
 

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()

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))
+    JOIN *join;
+    if ((join= fake_select_lex->join))
     {
-      error|= sl->join->cleanup();
+      join->tables_list= 0;
-      delete join;
+      join->tables= 0;
     }
-    else
+    error|= fake_select_lex->cleanup();
-    {
-      // 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();
-      }
-    }
-  }
-  if (fake_select_lex && (join= fake_select_lex->join))
-  {
-    join->tables_list= 0;
-    join->tables= 0;
-    error|= join->cleanup();
-    delete join;
   }
 
   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);
+}
+