- 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.

1 files changed, 29 insertions, 21 deletions

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);
+}
+