Bug#52357: Assertion failed: join->best_read in

greedy_search optimizer_search_depth=0

The algorithm inside restore_prev_nj_state failed to
properly update the counters within the NESTED_JOIN
tree. The counter was decremented each time a table in the
node was removed from the QEP, the correct thing to do being
only to decrement it when the last table in the child node
was removed from the plan. This lead to node counters
getting negative values and the plan thus appeared
impossible. An assertion caught this.

Fixed by not recursing up the tree unless the last table in
the join nest node is removed from the plan

mysql-test/r/join_outer.result

@@ -1254,3 +1254,33 @@ SELECT * FROM t1 LEFT JOIN t2 ON e<>0 WHERE c=1 AND d<=>NULL;
 c	e	d
 1	0	NULL
 DROP TABLE t1,t2;
+#
+# Bug#52357: Assertion failed: join->best_read in greedy_search 
+# optimizer_search_depth=0
+#
+CREATE TABLE t1( a INT );
+INSERT INTO t1 VALUES (1),(2);
+SET optimizer_search_depth = 0;
+# Should not core dump on query preparation
+EXPLAIN
+SELECT 1
+FROM t1 tt3 LEFT  OUTER JOIN t1 tt4 ON 1
+LEFT  OUTER JOIN t1 tt5 ON 1
+LEFT  OUTER JOIN t1 tt6 ON 1
+LEFT  OUTER JOIN t1 tt7 ON 1
+LEFT  OUTER JOIN t1 tt8 ON 1
+RIGHT OUTER JOIN t1 tt2 ON 1
+RIGHT OUTER JOIN t1 tt1 ON 1
+STRAIGHT_JOIN    t1 tt9 ON 1;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	tt1	ALL	NULL	NULL	NULL	NULL	2	
+1	SIMPLE	tt2	ALL	NULL	NULL	NULL	NULL	2	
+1	SIMPLE	tt3	ALL	NULL	NULL	NULL	NULL	2	
+1	SIMPLE	tt4	ALL	NULL	NULL	NULL	NULL	2	
+1	SIMPLE	tt5	ALL	NULL	NULL	NULL	NULL	2	
+1	SIMPLE	tt6	ALL	NULL	NULL	NULL	NULL	2	
+1	SIMPLE	tt7	ALL	NULL	NULL	NULL	NULL	2	
+1	SIMPLE	tt8	ALL	NULL	NULL	NULL	NULL	2	
+1	SIMPLE	tt9	ALL	NULL	NULL	NULL	NULL	2	
+SET optimizer_search_depth = DEFAULT;
+DROP TABLE t1;

mysql-test/t/join_outer.test

@@ -861,3 +861,27 @@ SELECT * FROM t1 LEFT JOIN t2 ON e<>0 WHERE c=1 AND d<=>NULL;
 
 DROP TABLE t1,t2;
 
+ 
+--echo #
+--echo # Bug#52357: Assertion failed: join->best_read in greedy_search 
+--echo # optimizer_search_depth=0
+--echo #
+CREATE TABLE t1( a INT );
+
+INSERT INTO t1 VALUES (1),(2);
+SET optimizer_search_depth = 0;
+
+--echo # Should not core dump on query preparation
+EXPLAIN
+SELECT 1
+FROM t1 tt3 LEFT  OUTER JOIN t1 tt4 ON 1
+            LEFT  OUTER JOIN t1 tt5 ON 1
+            LEFT  OUTER JOIN t1 tt6 ON 1
+            LEFT  OUTER JOIN t1 tt7 ON 1
+            LEFT  OUTER JOIN t1 tt8 ON 1
+            RIGHT OUTER JOIN t1 tt2 ON 1
+            RIGHT OUTER JOIN t1 tt1 ON 1
+            STRAIGHT_JOIN    t1 tt9 ON 1;
+
+SET optimizer_search_depth = DEFAULT;
+DROP TABLE t1;

sql/sql_select.cc

@@ -8706,6 +8706,39 @@ static bool check_interleaving_with_nj(JOIN_TAB *next_tab)
 /*
   Nested joins perspective: Remove the last table from the join order
 
+  The algorithm is the reciprocal of check_interleaving_with_nj(), hence
+  parent join nest nodes are updated only when the last table in its child
+  node is removed. The ASCII graphic below will clarify.
+
+  %A table nesting such as <tt> t1 x [ ( t2 x t3 ) x ( t4 x t5 ) ] </tt>is
+  represented by the below join nest tree.
+
+  @verbatim
+                     NJ1
+                  _/ /  \
+                _/  /    NJ2
+              _/   /     / \ 
+             /    /     /   \
+   t1 x [ (t2 x t3) x (t4 x t5) ]
+  @endverbatim
+
+  At the point in time when check_interleaving_with_nj() adds the table t5 to
+  the query execution plan, QEP, it also directs the node named NJ2 to mark
+  the table as covered. NJ2 does so by incrementing its @c counter
+  member. Since all of NJ2's tables are now covered by the QEP, the algorithm
+  proceeds up the tree to NJ1, incrementing its counter as well. All join
+  nests are now completely covered by the QEP.
+
+  restore_prev_nj_state() does the above in reverse. As seen above, the node
+  NJ1 contains the nodes t2, t3, and NJ2. Its counter being equal to 3 means
+  that the plan covers t2, t3, and NJ2, @e and that the sub-plan (t4 x t5)
+  completely covers NJ2. The removal of t5 from the partial plan will first
+  decrement NJ2's counter to 1. It will then detect that NJ2 went from being
+  completely to partially covered, and hence the algorithm must continue
+  upwards to NJ1 and decrement its counter to 2. %A subsequent removal of t4
+  will however not influence NJ1 since it did not un-cover the last table in
+  NJ2.
+
   SYNOPSIS
     restore_prev_nj_state()
       last  join table to remove, it is assumed to be the last in current 
@@ -8722,19 +8755,20 @@ static void restore_prev_nj_state(JOIN_TAB *last)
 {
   TABLE_LIST *last_emb= last->table->pos_in_table_list->embedding;
   JOIN *join= last->join;
-  while (last_emb)
+  for (;last_emb != NULL; last_emb= last_emb->embedding)
   {
-    if (!(--last_emb->nested_join->counter))
-      join->cur_embedding_map&= ~last_emb->nested_join->nj_map;
-    else if (last_emb->nested_join->join_list.elements-1 ==
-             last_emb->nested_join->counter) 
-    {
-      join->cur_embedding_map|= last_emb->nested_join->nj_map;
-      break;
-    }
-    else
+    NESTED_JOIN *nest= last_emb->nested_join;
+    DBUG_ASSERT(nest->counter > 0);
+    
+    bool was_fully_covered= nest->is_fully_covered();
+    
+    if (--nest->counter == 0)
+      join->cur_embedding_map&= ~nest->nj_map;
+    
+    if (!was_fully_covered)
       break;
-    last_emb= last_emb->embedding;
+    
+    join->cur_embedding_map|= nest->nj_map;
   }
 }
 

sql/table.h

@@ -910,7 +910,11 @@ typedef struct st_nested_join
   List<TABLE_LIST>  join_list;       /* list of elements in the nested join */
   table_map         used_tables;     /* bitmap of tables in the nested join */
   table_map         not_null_tables; /* tables that rejects nulls           */
-  struct st_join_table *first_nested;/* the first nested table in the plan  */
+  /**
+    Used for pointing out the first table in the plan being covered by this
+    join nest. It is used exclusively within make_outerjoin_info().
+   */
+  struct st_join_table *first_nested;
   /* 
     Used to count tables in the nested join in 2 isolated places:
     1. In make_outerjoin_info(). 
@@ -920,6 +924,15 @@ typedef struct st_nested_join
   */
   uint              counter;
   nested_join_map   nj_map;          /* Bit used to identify this nested join*/
+  /**
+     True if this join nest node is completely covered by the query execution
+     plan. This means two things.
+
+     1. All tables on its @c join_list are covered by the plan.
+
+     2. All child join nest nodes are fully covered.
+   */
+  bool is_fully_covered() const { return join_list.elements == counter; }
 } NESTED_JOIN;