MWL#90: Address review feedback part #7

sql/opt_subselect.cc

@@ -1223,6 +1223,7 @@ static void create_subquery_temptable_name(char *to, uint number)
   to[1]= 0;
 }
 
+
 /*
   Convert subquery predicate into non-mergeable semi-join nest.
 

sql/sql_join_cache.cc

@@ -33,8 +33,7 @@
 
 #define NO_MORE_RECORDS_IN_BUFFER  (uint)(-1)
 
-void save_or_restore_used_tabs(JOIN_TAB *join_tab, bool save);
+static void save_or_restore_used_tabs(JOIN_TAB *join_tab, bool save);
-JOIN_TAB *next_linear_tab(JOIN* join, JOIN_TAB* tab, bool include_bush_roots);
 
 /*****************************************************************************
  *  Join cache module
@@ -168,27 +167,46 @@ void JOIN_CACHE::calc_record_fields()
     if (join_tab->bush_root_tab)
     {
       /* 
-        If the tab we're attached to is inside an SJM-nest, start from the
+        --ot1--SJM1--------------ot2--...
-        first tab in that SJM nest
+                |
+                |
+                +-it1--...--itN
+                        ^____________ this->join_tab is somewhere here, 
+                                      inside an sjm nest.
+
+        The join buffer should store the values of it1.*, it2.*, ..
+        It should not store values of ot1.*.
       */
       tab= join_tab->bush_root_tab->bush_children->start;
     }
     else
     {
       /*
-        The tab we're attached to is not inside an SJM-nest. Start from the
+        -ot1--ot2--SJM1--SJM2--------------ot3--...--otN
-        first non-const table.
+                    |     |                      ^   
+                    |     +-it21--...--it2N      |
+                    |                            \-- we're somewhere here,
+                    +-it11--...--it1N                at the top level
+        
+        The join buffer should store the values of 
+
+          ot1.*, ot2.*, it1{i}, it2{j}.*, ot3.*, ...
+        
+        that is, we should start from the first non-const top-level table. 
+
+        We will need to store columns of SJ-inner tables (it_X_Y.*), but we're
+        not interested in storing the columns of materialization tables
+        themselves. Beause of that, if the first non-const top-level table is a
+        materialized table, we move to its bush_children:
       */
       tab= join->join_tab + join->const_tables;
+      if (tab->bush_children)
+        tab= tab->bush_children->start;
     }
   }
-  start_tab= tab;
-  if (start_tab->bush_children)
-    start_tab= start_tab->bush_children->start;
   DBUG_ASSERT(!start_tab->bush_children);
 
-  tab= start_tab;
+  start_tab= tab;
-
   fields= 0;
   blobs= 0;
   flag_fields= 0;
@@ -254,7 +272,8 @@ void JOIN_CACHE::collect_info_on_key_args()
   cache= this;
   do
   {
-    for (tab= cache->start_tab; tab != cache->join_tab; tab= next_linear_tab(join, tab, FALSE))
+    for (tab= cache->start_tab; tab != cache->join_tab;
+         tab= next_linear_tab(join, tab, FALSE))
     { 
       uint key_args;
       bitmap_clear_all(&tab->table->tmp_set);
@@ -3248,7 +3267,8 @@ int JOIN_TAB_SCAN::next()
   @param save TRUE   save 
               FALSE  restore
 */
-void save_or_restore_used_tabs(JOIN_TAB *join_tab, bool save)
+
+static void save_or_restore_used_tabs(JOIN_TAB *join_tab, bool save)
 {
   JOIN_TAB *first= join_tab->bush_root_tab?
                      join_tab->bush_root_tab->bush_children->start :

sql/sql_select.cc

@@ -6378,12 +6378,28 @@ JOIN_TAB *first_linear_tab(JOIN *join, bool after_const_tables)
   A helper function to loop over all join's join_tab in sequential fashion
 
   DESCRIPTION
-    Depending on include_bush_roots parameter, JOIN_TABS that represent
+    Depending on include_bush_roots parameter, JOIN_TABs that represent
-    SJM-scan/lookups are produced or omitted.
+    SJM-scan/lookups are either returned or omitted.
 
     SJM-Bush children are returned right after (or in place of) their container
     join tab (TODO: does anybody depend on this? A: make_join_readinfo() seems
-    to.)
+    to)
+
+    For example, if we have this structure:
+      
+       ot1--ot2--sjm1----------------ot3-...
+                  |
+                  +--it1--it2--it3
+
+    calls to next_linear_tab( include_bush_roots=TRUE) will return:
+      
+      ot1 ot2 sjm1 it1 it2 it3 ot3 ...
+   
+   while calls to next_linear_tab( include_bush_roots=FALSE) will return:
+
+      ot1 ot2 it1 it2 it3 ot3 ...
+
+   (note that sjm1 won't be returned).
 */
 
 JOIN_TAB *next_linear_tab(JOIN* join, JOIN_TAB* tab, bool include_bush_roots)
@@ -6418,6 +6434,24 @@ JOIN_TAB *next_linear_tab(JOIN* join, JOIN_TAB* tab, bool include_bush_roots)
 }
 
 
+/*
+  Start to iterate over all join tables in bush-children-first order, excluding 
+  the const tables (see next_depth_first_tab() comment for details)
+*/
+
+JOIN_TAB *first_depth_first_tab(JOIN* join)
+{
+  JOIN_TAB* tab;
+  /* This means we're starting the enumeration */
+  if (join->const_tables == join->top_jtrange_tables)
+    return NULL;
+
+  tab= join->join_tab + join->const_tables;
+
+  return (tab->bush_children) ? tab->bush_children->start : tab;
+}
+
+
 /*
   A helper function to iterate over all join tables in bush-children-first order
 
@@ -6425,43 +6459,28 @@ JOIN_TAB *next_linear_tab(JOIN* join, JOIN_TAB* tab, bool include_bush_roots)
    
   For example, for this join plan
 
-     ot1 ot2  sjm            ot3 
+    ot1--ot2--sjm1------------ot3-...
-              | +--------+ 
+               |
-              |          |
+               |
-              it1  it2  it3 
+              it1--it2--it3 
   
+  call to first_depth_first_tab() will return ot1, and subsequent calls to
+  next_depth_first_tab() will return:
 
-  the function will return
+     ot2 it1 it2 it3 sjm ot3 ...
-
-    ot1-ot2-it1-it2-it3-sjm-ot3 ...
-
 */
 
 JOIN_TAB *next_depth_first_tab(JOIN* join, JOIN_TAB* tab)
 {
-  bool start= FALSE;
+  /* If we're inside SJM nest and have reached its end, get out */
-  if (tab == NULL)
-  {
-    /* This means we're starting the enumeration */
-    if (join->const_tables == join->top_jtrange_tables)
-      return NULL;
-
-    tab= join->join_tab + join->const_tables;
-    start= TRUE;
-  }
-   
   if (tab->last_leaf_in_bush)
     return tab->bush_root_tab;
   
-  /* Move to next tab in the array we're traversing*/
+  /* Move to next tab in the array we're traversing */
-  if (!start)
   tab++;
   
-  //psergey-remove: check:
-  DBUG_ASSERT(join->join_tab_ranges.head()->end == 
-              join->join_tab +join->top_jtrange_tables);
   if (tab == join->join_tab +join->top_jtrange_tables)
-    return NULL; /* End */
+    return NULL; /* Outside SJM nest and reached EOF */
 
   if (tab->bush_children)
     return tab->bush_children->start;
@@ -7384,7 +7403,7 @@ make_join_select(JOIN *join,SQL_SELECT *select,COND *cond)
     JOIN_TAB *tab;
     table_map current_map;
     uint i= join->const_tables;
-    for (tab= next_depth_first_tab(join, NULL); tab; 
+    for (tab= first_depth_first_tab(join); tab;
          tab= next_depth_first_tab(join, tab), i++)
     {
       bool is_hj;