A fix and a test case for Bug#12736 "Server crash during a select".

The bug was in JOIN::join_free which was wrongly determining that
all joins have been already executed and therefore all used tables
can be closed.


mysql-test/r/subselect_innodb.result:
  - test results fixed (Bug#12736 "Server crash during a select
mysql-test/t/subselect_innodb.test:
  - a test case for Bug#12736 "Server crash during a select": test
  that ha_index_or_rnd_end and mysql_unlock_tables are called
  for all used tables in proper order.
sql/item_subselect.cc:
  - implement subselect_union_engine::is_executed
sql/item_subselect.h:
  - implement Item_subselect::is_evaluated. This function is used
  to check whether we can clean up a non-correlated join of a subquery
  when cleaning up the join of the outer query
sql/sql_lex.h:
  - declare st_select_lex::cleanup_all_joins
sql/sql_select.cc:
  - remove an argument from JOIN::join_free, it's now not used
  - reimplement JOIN::join_free to not unlock tables if there
    is a subquery that has not yet been evaluated. Make sure that the
    new implementation calls ha_index_or_rnd_end for every table in
    the join and inner joins, because all table cursors must be closed
    before mysql_unlock_tables.
sql/sql_select.h:
  - JOIN::join_free signature changed
sql/sql_union.cc:
  - implement a helper method st_select_lex::cleanup_all_joins, which
    recursively walks over a tree of joins and calls cleanup() for
    each join.

6 files changed, 109 insertions, 14 deletions

diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc
index 1ef3a92f548..8afc885e59b 100644
--- a/sql/item_subselect.cc
+++ b/sql/item_subselect.cc
@@ -1413,6 +1413,12 @@ void subselect_union_engine::cleanup()
 }
 
 
+bool subselect_union_engine::is_executed() const
+{
+  return unit->executed;
+}
+
+
 void subselect_uniquesubquery_engine::cleanup()
 {
   DBUG_ENTER("subselect_uniquesubquery_engine::cleanup");
diff --git a/sql/item_subselect.h b/sql/item_subselect.h
index 5b22930ae1f..f1c99f74498 100644
--- a/sql/item_subselect.h
+++ b/sql/item_subselect.h
@@ -110,6 +110,12 @@ public:
     return eng == 0;
   }
   /*
+    True if this subquery has been already evaluated. Implemented only for
+    single select and union subqueries only.
+  */
+  bool is_evaluated() const;
+
+  /*
     Used by max/min subquery to initialize value presence registration
     mechanism. Engine call this method before rexecution query.
   */
@@ -317,6 +323,7 @@ public:
   virtual void print(String *str)= 0;
   virtual bool change_result(Item_subselect *si, select_subselect *result)= 0;
   virtual bool no_tables()= 0;
+  virtual bool is_executed() const { return FALSE; }
 };
 
 
@@ -342,6 +349,7 @@ public:
   void print (String *str);
   bool change_result(Item_subselect *si, select_subselect *result);
   bool no_tables();
+  bool is_executed() const { return executed; }
 };
 
 
@@ -363,6 +371,7 @@ public:
   void print (String *str);
   bool change_result(Item_subselect *si, select_subselect *result);
   bool no_tables();
+  bool is_executed() const;
 };
 
 
@@ -411,3 +420,10 @@ public:
   int exec();
   void print (String *str);
 };
+
+
+inline bool Item_subselect::is_evaluated() const
+{
+  return engine->is_executed();
+}
+
diff --git a/sql/sql_lex.h b/sql/sql_lex.h
index 8d7ec25f97b..3c34c7aaaea 100644
--- a/sql/sql_lex.h
+++ b/sql/sql_lex.h
@@ -386,12 +386,12 @@ protected:
   select_result *result;
   ulong found_rows_for_union;
   bool res;
+public:
   bool  prepared, // prepare phase already performed for UNION (unit)
     optimized, // optimize phase already performed for UNION (unit)
     executed, // already executed
     cleaned;
 
-public:
   // list of fields which points to temporary table for union
   List<Item> item_list;
   /*
@@ -638,6 +638,11 @@ public:
     SELECT_LEX and all nested SELECT_LEXes and SELECT_LEX_UNITs).
   */
   bool cleanup();
+  /*
+    Recursively cleanup the join of this select lex and of all nested
+    select lexes.
+  */
+  void cleanup_all_joins(bool full);
 };
 typedef class st_select_lex SELECT_LEX;
 
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 1b1a35d2584..d6a9a64ef70 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -1377,7 +1377,7 @@ JOIN::exec()
       DBUG_PRINT("info",("Creating group table"));
       
       /* Free first data from old join */
-      curr_join->join_free(0);
+      curr_join->join_free();
       if (make_simple_join(curr_join, curr_tmp_table))
 	DBUG_VOID_RETURN;
       calc_group_buffer(curr_join, group_list);
@@ -1475,7 +1475,7 @@ JOIN::exec()
     if (curr_tmp_table->distinct)
       curr_join->select_distinct=0;		/* Each row is unique */
     
-    curr_join->join_free(0);			/* Free quick selects */
+    curr_join->join_free();			/* Free quick selects */
     if (curr_join->select_distinct && ! curr_join->group_list)
     {
       thd->proc_info="Removing duplicates";
@@ -5718,34 +5718,88 @@ void JOIN_TAB::cleanup()
   end_read_record(&read_record);
 }
 
+/*
+  Partially cleanup JOIN after it has executed: close index or rnd read
+  (table cursors), free quick selects.
+
+  DESCRIPTION
+    This function is called in the end of execution of a JOIN, before the used
+    tables are unlocked and closed.
+
+    For a join that is resolved using a temporary table, the first sweep is
+    performed against actual tables and an intermediate result is inserted
+    into the temprorary table.
+    The last sweep is performed against the temporary table. Therefore,
+    the base tables and associated buffers used to fill the temporary table
+    are no longer needed, and this function is called to free them.
+
+    For a join that is performed without a temporary table, this function
+    is called after all rows are sent, but before EOF packet is sent.
+
+    For a simple SELECT with no subqueries this function performs a full
+    cleanup of the JOIN and calls mysql_unlock_read_tables to free used base
+    tables.
 
-void JOIN::join_free(bool full)
+    If a JOIN is executed for a subquery or if it has a subquery, we can't
+    do the full cleanup and need to do a partial cleanup only.
+      o If a JOIN is not the top level join, we must not unlock the tables
+        because the outer select may not have been evaluated yet, and we
+        can't unlock only selected tables of a query.
+
+      o Additionally, if this JOIN corresponds to a correlated subquery, we
+        should not free quick selects and join buffers because they will be
+        needed for the next execution of the correlated subquery.
+
+      o However, if this is a JOIN for a [sub]select, which is not
+        a correlated subquery itself, but has subqueries, we can free it
+        fully and also free JOINs of all its subqueries. The exception
+        is a subquery in SELECT list, e.g:
+        SELECT a, (select max(b) from t1) group by c
+        This subquery will not be evaluated at first sweep and its value will
+        not be inserted into the temporary table. Instead, it's evaluated
+        when selecting from the temporary table. Therefore, it can't be freed
+        here even though it's not correlated.
+*/
+
+void JOIN::join_free()
 {
   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->describe);
+  bool full= (!select_lex->uncacheable && !thd->lex->describe);
+  bool can_unlock= full;
+  DBUG_ENTER("JOIN::join_free");
 
   cleanup(full);
 
   for (unit= select_lex->first_inner_unit(); unit; unit= unit->next_unit())
     for (sl= unit->first_select(); sl; sl= sl->next_select())
     {
-      JOIN *join= sl->join;
-      if (join)
-        join->join_free(full);
+      Item_subselect *subselect= sl->master_unit()->item;
+      bool full_local= full && (!subselect || subselect->is_evaluated());
+      /*
+        If this join is evaluated, we can fully clean it up and clean up all
+        its underlying joins even if they are correlated -- they will not be
+        used any more anyway.
+        If this join is not yet evaluated, we still must clean it up to
+        close its table cursors -- it may never get evaluated, as in case of
+        ... HAVING FALSE OR a IN (SELECT ...))
+        but all table cursors must be closed before the unlock.
+      */
+      sl->cleanup_all_joins(full_local);
+      /* Can't unlock if at least one JOIN is still needed */
+      can_unlock= can_unlock && full_local;
     }
 
   /*
     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) &&
+  if (can_unlock && 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)))
@@ -6059,7 +6113,7 @@ return_zero_rows(JOIN *join, select_result *result,TABLE_LIST *tables,
     DBUG_RETURN(0);
   }
 
-  join->join_free(0);
+  join->join_free();
 
   if (send_row)
   {
@@ -9004,7 +9058,7 @@ do_select(JOIN *join,List<Item> *fields,TABLE *table,Procedure *procedure)
 	The following will unlock all cursors if the command wasn't an
 	update command
       */
-      join->join_free(0);				// Unlock all cursors
+      join->join_free();			// Unlock all cursors
       if (join->result->send_eof())
 	rc= 1;                                  // Don't send error
     }
diff --git a/sql/sql_select.h b/sql/sql_select.h
index 0dc4be8c104..d6161eb6372 100644
--- a/sql/sql_select.h
+++ b/sql/sql_select.h
@@ -358,7 +358,7 @@ class JOIN :public Sql_alloc
     the end of execution in order to increase concurrency and reduce
     memory consumption.
   */
-  void join_free(bool full);
+  void join_free();
   /* Cleanup this JOIN, possibly for reuse */
   void cleanup(bool full);
   void clear();
diff --git a/sql/sql_union.cc b/sql/sql_union.cc
index 951248e8cd8..dee88af7d83 100644
--- a/sql/sql_union.cc
+++ b/sql/sql_union.cc
@@ -720,3 +720,17 @@ bool st_select_lex::cleanup()
   DBUG_RETURN(error);
 }
 
+
+void st_select_lex::cleanup_all_joins(bool full)
+{
+  SELECT_LEX_UNIT *unit;
+  SELECT_LEX *sl;
+
+  if (join)
+    join->cleanup(full);
+
+  for (unit= first_inner_unit(); unit; unit= unit->next_unit())
+    for (sl= unit->first_select(); sl; sl= sl->next_select())
+      sl->cleanup_all_joins(full);
+}
+