MDEV-11640 gcol.gcol_select_myisam fails in buildbot on Power

JOIN_CACHE's were initialized in  check_join_cache_usage()
from make_join_readinfo(). After that make_join_readinfo() was looking
whether it's possible to use keyread. Later, after make_join_readinfo(),
optimizer decided whether to use filesort. And even later, at the
execution time, from join_read_first(), keyread was actually enabled.

The problem is, that if a query uses a vcol, base columns that it
depends on are automatically added to the read_set - because they're
needed to calculate the vcol. But if we're doing keyread, vcol is taken
from the index, not calculated, and base columns do not need to  be
in the read set (even should not be - as they aren't getting values).

The bug was that JOIN_CACHE used read_set with base columns,
they were not read because of keyread, so it was caching garbage.

So read_set is only known after the keyread was decided. And after the
filesort was decided, as filesort doesn't use keyread. But
check_join_cache_usage() needs to be done in make_join_readinfo(),
as the code below depends on these checks,

Fix: keep JOIN_CACHE checks where they were, but move initialization
down to the very end of JOIN::optimize_inner. If keyread was enabled,
update the read_set to include only columns that are part of the index.
Copy the keyread logic from join_read_first() to happen at optimize time.

3 files changed, 50 insertions, 18 deletions

diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index a981e3adc60..efdbed93802 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -1111,6 +1111,34 @@ int JOIN::optimize()
 }
 
 
+int JOIN::init_join_caches()
+{
+  JOIN_TAB *tab;
+
+  for (tab= first_linear_tab(this, WITH_BUSH_ROOTS, WITHOUT_CONST_TABLES);
+       tab;
+       tab= next_linear_tab(this, tab, WITH_BUSH_ROOTS))
+  {
+    TABLE *table= tab->table;
+    if (table->file->keyread_enabled())
+    {
+      if (!(table->file->index_flags(table->file->keyread, 0, 1) & HA_CLUSTERED_INDEX))
+        table->mark_columns_used_by_index(table->file->keyread, table->read_set);
+    }
+    else if ((tab->read_first_record == join_read_first ||
+              tab->read_first_record == join_read_last) &&
+             !tab->filesort && table->covering_keys.is_set(tab->index) &&
+             !table->no_keyread)
+    {
+      table->prepare_for_keyread(tab->index, table->read_set);
+    }
+    if (tab->cache && tab->cache->init(select_options & SELECT_DESCRIBE))
+      revise_cache_usage(tab);
+  }
+  return 0;
+}
+
+
 /**
   global select optimisation.
 
@@ -2122,6 +2150,9 @@ JOIN::optimize_inner()
   if (make_aggr_tables_info())
     DBUG_RETURN(1);
 
+  if (init_join_caches())
+    DBUG_RETURN(1);
+
   error= 0;
 
   if (select_options & SELECT_DESCRIBE)
@@ -10408,11 +10439,10 @@ void set_join_cache_denial(JOIN_TAB *join_tab)
     if (join_tab->cache->prev_cache)
       join_tab->cache->prev_cache->next_cache= 0;
     /*
-      No need to do the same for next_cache since cache denial is done
-      backwards starting from the latest cache in the linked list (see
-      revise_cache_usage()).
+      Same for the next_cache
     */
-    DBUG_ASSERT(!join_tab->cache->next_cache);
+    if (join_tab->cache->next_cache)
+      join_tab->cache->next_cache->prev_cache= 0;
 
     join_tab->cache->free();
     join_tab->cache= 0;
@@ -10872,8 +10902,7 @@ uint check_join_cache_usage(JOIN_TAB *tab,
   case JT_ALL:
     if (cache_level == 1)
       prev_cache= 0;
-    if ((tab->cache= new (root) JOIN_CACHE_BNL(join, tab, prev_cache)) &&
-         !tab->cache->init(options & SELECT_DESCRIBE))
+    if ((tab->cache= new (root) JOIN_CACHE_BNL(join, tab, prev_cache)))
     {
       tab->icp_other_tables_ok= FALSE;
       return (2 - MY_TEST(!prev_cache));
@@ -10907,8 +10936,7 @@ uint check_join_cache_usage(JOIN_TAB *tab,
         goto no_join_cache;
       if (cache_level == 3)
         prev_cache= 0;
-      if ((tab->cache= new (root) JOIN_CACHE_BNLH(join, tab, prev_cache)) &&
-          !tab->cache->init(options & SELECT_DESCRIBE))
+      if ((tab->cache= new (root) JOIN_CACHE_BNLH(join, tab, prev_cache)))
       {
         tab->icp_other_tables_ok= FALSE;        
         return (4 - MY_TEST(!prev_cache));
@@ -10928,8 +10956,7 @@ uint check_join_cache_usage(JOIN_TAB *tab,
       {
         if (cache_level == 5)
           prev_cache= 0;
-        if ((tab->cache= new (root) JOIN_CACHE_BKA(join, tab, flags, prev_cache)) &&
-            !tab->cache->init(options & SELECT_DESCRIBE))
+        if ((tab->cache= new (root) JOIN_CACHE_BKA(join, tab, flags, prev_cache)))
           return (6 - MY_TEST(!prev_cache));
         goto no_join_cache;
       }
@@ -10937,8 +10964,7 @@ uint check_join_cache_usage(JOIN_TAB *tab,
       {
         if (cache_level == 7)
           prev_cache= 0;
-        if ((tab->cache= new (root) JOIN_CACHE_BKAH(join, tab, flags, prev_cache)) &&
-            !tab->cache->init(options & SELECT_DESCRIBE))
+        if ((tab->cache= new (root) JOIN_CACHE_BKAH(join, tab, flags, prev_cache)))
 	{
           tab->idx_cond_fact_out= FALSE;
           return (8 - MY_TEST(!prev_cache));
@@ -19347,8 +19373,9 @@ join_read_first(JOIN_TAB *tab)
   TABLE *table=tab->table;
   DBUG_ENTER("join_read_first");
 
-  if (table->covering_keys.is_set(tab->index) && !table->no_keyread)
-    table->file->ha_start_keyread(tab->index);
+  DBUG_ASSERT(table->no_keyread ||
+              !table->covering_keys.is_set(tab->index) ||
+              table->file->keyread == tab->index);
   tab->table->status=0;
   tab->read_record.read_record=join_read_next;
   tab->read_record.table=table;
@@ -19386,8 +19413,9 @@ join_read_last(JOIN_TAB *tab)
   int error= 0;
   DBUG_ENTER("join_read_first");
 
-  if (table->covering_keys.is_set(tab->index) && !table->no_keyread)
-    table->file->ha_start_keyread(tab->index);
+  DBUG_ASSERT(table->no_keyread ||
+              !table->covering_keys.is_set(tab->index) ||
+              table->file->keyread == tab->index);
   tab->table->status=0;
   tab->read_record.read_record=join_read_prev;
   tab->read_record.table=table;
diff --git a/sql/sql_select.h b/sql/sql_select.h
index f1d48b700d7..f5bbc6718a0 100644
--- a/sql/sql_select.h
+++ b/sql/sql_select.h
@@ -1512,6 +1512,7 @@ public:
   bool flatten_subqueries();
   bool optimize_unflattened_subqueries();
   bool optimize_constant_subqueries();
+  int init_join_caches();
   bool make_sum_func_list(List<Item> &all_fields, List<Item> &send_fields,
 			  bool before_group_by, bool recompute= FALSE);
 
diff --git a/sql/table.cc b/sql/table.cc
index 33317234c85..1c6cc4205a6 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -6076,8 +6076,11 @@ MY_BITMAP *TABLE::prepare_for_keyread(uint index, MY_BITMAP *map)
   DBUG_ENTER("TABLE::prepare_for_keyread");
   if (!no_keyread)
     file->ha_start_keyread(index);
-  mark_columns_used_by_index(index, map);
-  column_bitmaps_set(map);
+  if (map != read_set || !(file->index_flags(index, 0, 1) & HA_CLUSTERED_INDEX))
+  {
+    mark_columns_used_by_index(index, map);
+    column_bitmaps_set(map);
+  }
   DBUG_RETURN(backup);
 }