MDEV-11836 vcol.vcol_keys_myisam fails in buildbot and outside

move TABLE::key_read into handler. Because in index merge and DS-MRR
there can be many handlers per table, and some of them use
key read while others don't. "keyread" is really per handler,
not per TABLE property.

mysql-test/suite/vcol/disabled.def

@@ -1 +0,0 @@
-vcol_keys_myisam : MDEV-11836

sql/handler.cc

@@ -2580,7 +2580,7 @@ int handler::ha_rnd_next(uchar *buf)
   {
     update_rows_read();
     if (table->vfield && buf == table->record[0])
-      table->update_virtual_fields(VCOL_UPDATE_FOR_READ);
+      table->update_virtual_fields(this, VCOL_UPDATE_FOR_READ);
     increment_statistics(&SSV::ha_read_rnd_next_count);
   }
   else if (result == HA_ERR_RECORD_DELETED)
@@ -2608,7 +2608,7 @@ int handler::ha_rnd_pos(uchar *buf, uchar *pos)
   {
     update_rows_read();
     if (table->vfield && buf == table->record[0])
-      table->update_virtual_fields(VCOL_UPDATE_FOR_READ);
+      table->update_virtual_fields(this, VCOL_UPDATE_FOR_READ);
   }
   table->status=result ? STATUS_NOT_FOUND: 0;
   DBUG_RETURN(result);
@@ -2631,7 +2631,7 @@ int handler::ha_index_read_map(uchar *buf, const uchar *key,
   {
     update_index_statistics();
     if (table->vfield && buf == table->record[0])
-      table->update_virtual_fields(VCOL_UPDATE_FOR_READ);
+      table->update_virtual_fields(this, VCOL_UPDATE_FOR_READ);
   }
   table->status=result ? STATUS_NOT_FOUND: 0;
   DBUG_RETURN(result);
@@ -2660,7 +2660,7 @@ int handler::ha_index_read_idx_map(uchar *buf, uint index, const uchar *key,
     update_rows_read();
     index_rows_read[index]++;
     if (table->vfield && buf == table->record[0])
-      table->update_virtual_fields(VCOL_UPDATE_FOR_READ);
+      table->update_virtual_fields(this, VCOL_UPDATE_FOR_READ);
   }
   table->status=result ? STATUS_NOT_FOUND: 0;
   return result;
@@ -2681,7 +2681,7 @@ int handler::ha_index_next(uchar * buf)
   {
     update_index_statistics();
     if (table->vfield && buf == table->record[0])
-      table->update_virtual_fields(VCOL_UPDATE_FOR_READ);
+      table->update_virtual_fields(this, VCOL_UPDATE_FOR_READ);
   }
   table->status=result ? STATUS_NOT_FOUND: 0;
   DBUG_RETURN(result);
@@ -2702,7 +2702,7 @@ int handler::ha_index_prev(uchar * buf)
   {
     update_index_statistics();
     if (table->vfield && buf == table->record[0])
-      table->update_virtual_fields(VCOL_UPDATE_FOR_READ);
+      table->update_virtual_fields(this, VCOL_UPDATE_FOR_READ);
   }
   table->status=result ? STATUS_NOT_FOUND: 0;
   DBUG_RETURN(result);
@@ -2722,7 +2722,7 @@ int handler::ha_index_first(uchar * buf)
   {
     update_index_statistics();
     if (table->vfield && buf == table->record[0])
-      table->update_virtual_fields(VCOL_UPDATE_FOR_READ);
+      table->update_virtual_fields(this, VCOL_UPDATE_FOR_READ);
   }
   table->status=result ? STATUS_NOT_FOUND: 0;
   return result;
@@ -2742,7 +2742,7 @@ int handler::ha_index_last(uchar * buf)
   {
     update_index_statistics();
     if (table->vfield && buf == table->record[0])
-      table->update_virtual_fields(VCOL_UPDATE_FOR_READ);
+      table->update_virtual_fields(this, VCOL_UPDATE_FOR_READ);
   }
   table->status=result ? STATUS_NOT_FOUND: 0;
   return result;
@@ -2762,7 +2762,7 @@ int handler::ha_index_next_same(uchar *buf, const uchar *key, uint keylen)
   {
     update_index_statistics();
     if (table->vfield && buf == table->record[0])
-      table->update_virtual_fields(VCOL_UPDATE_FOR_READ);
+      table->update_virtual_fields(this, VCOL_UPDATE_FOR_READ);
   }
   table->status=result ? STATUS_NOT_FOUND: 0;
   return result;
@@ -5911,7 +5911,7 @@ int handler::ha_reset()
               table->s->column_bitmap_size ==
               (uchar*) table->def_write_set.bitmap);
   DBUG_ASSERT(bitmap_is_set_all(&table->s->all_set));
-  DBUG_ASSERT(table->key_read == 0);
+  DBUG_ASSERT(table->file->key_read == 0);
   /* ensure that ha_index_end / ha_rnd_end has been called */
   DBUG_ASSERT(inited == NONE);
   /* reset the bitmaps to point to defaults */

sql/handler.h

@@ -2654,6 +2654,7 @@ class handler :public Sql_alloc
   uint errkey;                             /* Last dup key */
   uint key_used_on_scan;
   uint active_index;
+  bool key_read;
 
   /** Length of ref (1-8 or the clustered key length) */
   uint ref_length;
@@ -2695,7 +2696,6 @@ class handler :public Sql_alloc
 public:
   void set_time_tracker(Exec_time_tracker *tracker_arg) { tracker=tracker_arg;}
 
-
   Item *pushed_idx_cond;
   uint pushed_idx_cond_keyno;  /* The index which the above condition is for */
 
@@ -2751,6 +2751,7 @@ class handler :public Sql_alloc
     in_range_check_pushed_down(FALSE),
     key_used_on_scan(MAX_KEY),
     active_index(MAX_KEY),
+    key_read(false),
     ref_length(sizeof(my_off_t)),
     ft_handler(0), inited(NONE),
     pushed_cond(0), next_insert_id(0), insert_id_for_cur_row(0),
@@ -2855,6 +2856,21 @@ class handler :public Sql_alloc
   int ha_delete_row(const uchar * buf);
   void ha_release_auto_increment();
 
+  int ha_start_keyread()
+  {
+    if (key_read)
+      return 0;
+    key_read= 1;
+    return extra(HA_EXTRA_KEYREAD);
+  }
+  int ha_end_keyread()
+  {
+    if (!key_read)
+      return 0;
+    key_read= 0;
+    return extra(HA_EXTRA_NO_KEYREAD);
+  }
+
   int check_collation_compatibility();
   int ha_check_for_upgrade(HA_CHECK_OPT *check_opt);
   /** to be actually called to get 'check()' functionality*/

sql/opt_range.cc

@@ -1236,7 +1236,7 @@ QUICK_SELECT_I::QUICK_SELECT_I()
 QUICK_RANGE_SELECT::QUICK_RANGE_SELECT(THD *thd, TABLE *table, uint key_nr,
                                        bool no_alloc, MEM_ROOT *parent_alloc,
                                        bool *create_error)
-  :doing_key_read(0),free_file(0),cur_range(NULL),last_range(0),dont_free(0)
+  :free_file(0),cur_range(NULL),last_range(0),dont_free(0)
 {
   my_bitmap_map *bitmap;
   DBUG_ENTER("QUICK_RANGE_SELECT::QUICK_RANGE_SELECT");
@@ -1318,8 +1318,7 @@ QUICK_RANGE_SELECT::~QUICK_RANGE_SELECT()
     if (file) 
     {
       range_end();
-      if (doing_key_read)
-        file->extra(HA_EXTRA_NO_KEYREAD);
+      file->ha_end_keyread();
       if (free_file)
       {
         DBUG_PRINT("info", ("Freeing separate handler 0x%lx (free: %d)", (long) file,
@@ -1475,7 +1474,6 @@ int QUICK_RANGE_SELECT::init_ror_merged_scan(bool reuse_handler,
                                              MEM_ROOT *local_alloc)
 {
   handler *save_file= file, *org_file;
-  my_bool org_key_read;
   THD *thd= head->in_use;
   MY_BITMAP * const save_vcol_set= head->vcol_set;
   MY_BITMAP * const save_read_set= head->read_set;
@@ -1537,21 +1535,15 @@ int QUICK_RANGE_SELECT::init_ror_merged_scan(bool reuse_handler,
     key. The 'column_bitmap' is used in ::get_next()
   */
   org_file= head->file;
-  org_key_read= head->key_read;
   head->file= file;
-  head->key_read= 0;
   head->mark_columns_used_by_index_no_reset(index, &column_bitmap);
 
   if (!head->no_keyread)
-  {
-    doing_key_read= 1;
-    head->set_keyread(true);
-  }
+    head->file->ha_start_keyread();
 
   head->prepare_for_position();
 
   head->file= org_file;
-  head->key_read= org_key_read;
 
   /* Restore head->read_set (and write_set) to what they had before the call */
   head->column_bitmaps_set(save_read_set, save_write_set);
@@ -10631,7 +10623,7 @@ QUICK_RANGE_SELECT *get_quick_select_for_ref(THD *thd, TABLE *table,
 
   /* Call multi_range_read_info() to get the MRR flags and buffer size */
   quick->mrr_flags= HA_MRR_NO_ASSOCIATION | 
-                    (table->key_read ? HA_MRR_INDEX_ONLY : 0);
+                    (table->file->key_read ? HA_MRR_INDEX_ONLY : 0);
   if (thd->lex->sql_command != SQLCOM_SELECT)
     quick->mrr_flags |= HA_MRR_USE_DEFAULT_IMPL;
 
@@ -10681,15 +10673,10 @@ int read_keys_and_merge_scans(THD *thd,
   Unique *unique= *unique_ptr;
   handler *file= head->file;
   bool with_cpk_filter= pk_quick_select != NULL;
-  bool enabled_keyread= 0;
   DBUG_ENTER("read_keys_and_merge");
 
   /* We're going to just read rowids. */
-  if (!head->key_read)
-  {
-    enabled_keyread= 1;
-    head->set_keyread(true);
-  }
+  head->file->ha_start_keyread();
   head->prepare_for_position();
 
   cur_quick_it.rewind();
@@ -10780,16 +10767,14 @@ int read_keys_and_merge_scans(THD *thd,
   /*
     index merge currently doesn't support "using index" at all
   */
-  if (enabled_keyread)
-    head->set_keyread(false);
+  head->file->ha_end_keyread();
   if (init_read_record(read_record, thd, head, (SQL_SELECT*) 0,
                        &unique->sort, 1 , 1, TRUE))
     result= 1;
  DBUG_RETURN(result);
 
 err:
-  if (enabled_keyread)
-    head->set_keyread(false);
+  head->file->ha_end_keyread();
   DBUG_RETURN(1);
 }
 
@@ -13340,7 +13325,7 @@ QUICK_GROUP_MIN_MAX_SELECT(TABLE *table, JOIN *join_arg, bool have_min_arg,
    group_prefix_len(group_prefix_len_arg),
    group_key_parts(group_key_parts_arg), have_min(have_min_arg),
    have_max(have_max_arg), have_agg_distinct(have_agg_distinct_arg),
-   seen_first_key(FALSE), doing_key_read(FALSE), min_max_arg_part(min_max_arg_part_arg),
+   seen_first_key(FALSE), min_max_arg_part(min_max_arg_part_arg),
    key_infix(key_infix_arg), key_infix_len(key_infix_len_arg),
    min_functions_it(NULL), max_functions_it(NULL),
    is_index_scan(is_index_scan_arg)
@@ -13480,8 +13465,7 @@ QUICK_GROUP_MIN_MAX_SELECT::~QUICK_GROUP_MIN_MAX_SELECT()
   if (file->inited != handler::NONE) 
   {
     DBUG_ASSERT(file == head->file);
-    if (doing_key_read)
-      head->set_keyread(false);
+    head->file->ha_end_keyread();
     /*
       There may be a code path when the same table was first accessed by index,
       then the index is closed, and the table is scanned (order by + loose scan).
@@ -13671,11 +13655,8 @@ int QUICK_GROUP_MIN_MAX_SELECT::reset(void)
   DBUG_ENTER("QUICK_GROUP_MIN_MAX_SELECT::reset");
 
   seen_first_key= FALSE;
-  if (!head->key_read)
-  {
-    doing_key_read= 1;
-    head->set_keyread(true); /* We need only the key attributes */
-  }
+  head->file->ha_start_keyread(); /* We need only the key attributes */
+
   if ((result= file->ha_index_init(index,1)))
   {
     head->file->print_error(result, MYF(0));

sql/opt_range.h

@@ -1037,7 +1037,6 @@ class QUICK_RANGE_SELECT : public QUICK_SELECT_I
 {
 protected:
   /* true if we enabled key only reads */
-  bool doing_key_read;
   handler *file;
 
   /* Members to deal with case when this quick select is a ROR-merged scan */

sql/opt_sum.cc

@@ -406,7 +406,7 @@ int opt_sum_query(THD *thd,
 	  if (!error && reckey_in_range(is_max, &ref, item_field->field, 
 			                conds, range_fl, prefix_len))
 	    error= HA_ERR_KEY_NOT_FOUND;
-          table->set_keyread(false);
+          table->file->ha_end_keyread();
           table->file->ha_index_end();
           if (error)
 	  {
@@ -968,7 +968,7 @@ static bool find_key_for_maxmin(bool max_fl, TABLE_REF *ref,
             converted (for example to upper case)
           */
           if (field->part_of_key.is_set(idx))
-            table->set_keyread(true);
+            table->file->ha_start_keyread();
           DBUG_RETURN(TRUE);
         }
       }

sql/sql_base.cc

@@ -859,7 +859,7 @@ void close_thread_table(THD *thd, TABLE **table_ptr)
   DBUG_ENTER("close_thread_table");
   DBUG_PRINT("tcache", ("table: '%s'.'%s' 0x%lx", table->s->db.str,
                         table->s->table_name.str, (long) table));
-  DBUG_ASSERT(table->key_read == 0);
+  DBUG_ASSERT(table->file->key_read == 0);
   DBUG_ASSERT(!table->file || table->file->inited == handler::NONE);
 
   /*
@@ -7918,7 +7918,7 @@ fill_record(THD *thd, TABLE *table_arg, List<Item> &fields, List<Item> &values,
     goto err;
   /* Update virtual fields */
   if (table_arg->vfield &&
-      table_arg->update_virtual_fields(VCOL_UPDATE_FOR_WRITE))
+      table_arg->update_virtual_fields(table_arg->file, VCOL_UPDATE_FOR_WRITE))
     goto err;
   thd->abort_on_warning= save_abort_on_warning;
   thd->no_errors=        save_no_errors;
@@ -8067,7 +8067,8 @@ fill_record_n_invoke_before_triggers(THD *thd, TABLE *table,
       if (item_field)
       {
         DBUG_ASSERT(table == item_field->field->table);
-        result|= table->update_virtual_fields(VCOL_UPDATE_FOR_WRITE);
+        result|= table->update_virtual_fields(table->file,
+                                              VCOL_UPDATE_FOR_WRITE);
       }
     }
   }
@@ -8163,7 +8164,7 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values,
   /* Update virtual fields */
   thd->abort_on_warning= FALSE;
   if (table->vfield &&
-      table->update_virtual_fields(VCOL_UPDATE_FOR_WRITE))
+      table->update_virtual_fields(table->file, VCOL_UPDATE_FOR_WRITE))
     goto err;
   thd->abort_on_warning= abort_on_warning_saved;
   DBUG_RETURN(thd->is_error());
@@ -8217,7 +8218,7 @@ fill_record_n_invoke_before_triggers(THD *thd, TABLE *table, Field **ptr,
   {
     DBUG_ASSERT(table == (*ptr)->table);
     if (table->vfield)
-      result= table->update_virtual_fields(VCOL_UPDATE_FOR_WRITE);
+      result= table->update_virtual_fields(table->file, VCOL_UPDATE_FOR_WRITE);
   }
   return result;
 

sql/sql_delete.cc

@@ -562,7 +562,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds,
     explain->tracker.on_record_read();
     thd->inc_examined_row_count(1);
     if (table->vfield)
-      (void) table->update_virtual_fields(VCOL_UPDATE_FOR_DELETE);
+      (void) table->update_virtual_fields(table->file, VCOL_UPDATE_FOR_DELETE);
     if (!select || select->skip_record(thd) > 0)
     {
       explain->tracker.on_record_after_where();

sql/sql_insert.cc

@@ -1732,7 +1732,7 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info)
           in handler methods for the just read row in record[1].
         */
         table->move_fields(table->field, table->record[1], table->record[0]);
-        table->update_virtual_fields(VCOL_UPDATE_FOR_REPLACE);
+        table->update_virtual_fields(table->file, VCOL_UPDATE_FOR_REPLACE);
         table->move_fields(table->field, table->record[0], table->record[1]);
       }
       if (info->handle_duplicates == DUP_UPDATE)
@@ -3268,7 +3268,8 @@ bool Delayed_insert::handle_inserts(void)
         TABLE object used had vcol_set empty. Better to calculate them
         here to make the caller faster.
       */
-      tmp_error= table->update_virtual_fields(VCOL_UPDATE_FOR_WRITE);
+      tmp_error= table->update_virtual_fields(table->file,
+                                              VCOL_UPDATE_FOR_WRITE);
     }
 
     if (tmp_error || write_record(&thd, table, &info))