BUG#49902 - SELECT returns incorrect results

Queries optimized with GROUP_MIN_MAX didn't cleanup KEYREAD
optimization properly. As a result subsequent queries may
return incomplete rows (fields are initialized to default
values).

mysql-test/r/group_min_max.result:
  A test case for BUG#49902.
mysql-test/t/group_min_max.test:
  A test case for BUG#49902.
sql/opt_range.cc:
  Refactor of KEYREAD optimization switch so that KEYREAD
  handler state is in sync with st_table::key_read flag.
  
  All SQL code is supposed to switch KEYREAD optimization
  via st_table::set_keyread().
sql/opt_sum.cc:
  Refactor of KEYREAD optimization switch so that KEYREAD
  handler state is in sync with st_table::key_read flag.
  
  All SQL code is supposed to switch KEYREAD optimization
  via st_table::set_keyread().
sql/sql_select.cc:
  Refactor of KEYREAD optimization switch so that KEYREAD
  handler state is in sync with st_table::key_read flag.
  
  All SQL code is supposed to switch KEYREAD optimization
  via st_table::set_keyread().
sql/sql_update.cc:
  Refactor of KEYREAD optimization switch so that KEYREAD
  handler state is in sync with st_table::key_read flag.
  
  All SQL code is supposed to switch KEYREAD optimization
  via st_table::set_keyread().
sql/table.cc:
  Refactor of KEYREAD optimization switch so that KEYREAD
  handler state is in sync with st_table::key_read flag.
  
  All SQL code is supposed to switch KEYREAD optimization
  via st_table::set_keyread().
sql/table.h:
  Refactor of KEYREAD optimization switch so that KEYREAD
  handler state is in sync with st_table::key_read flag.
  
  All SQL code is supposed to switch KEYREAD optimization
  via st_table::set_keyread().

mysql-test/r/group_min_max.result

@@ -2524,4 +2524,17 @@ SELECT a, MAX(b) FROM t WHERE b GROUP BY a;
 a	MAX(b)
 2	1
 DROP TABLE t;
+CREATE TABLE t1(a INT NOT NULL, b INT NOT NULL, KEY (b));
+INSERT INTO t1 VALUES(1,1),(2,1);
+ANALYZE TABLE t1;
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	OK
+SELECT 1 AS c, b FROM t1 WHERE b IN (1,2) GROUP BY c, b;
+c	b
+1	1
+SELECT a FROM t1 WHERE b=1;
+a
+1
+2
+DROP TABLE t1;
 End of 5.1 tests

mysql-test/t/group_min_max.test

@@ -1044,4 +1044,14 @@ SELECT a, MAX(b) FROM t WHERE b GROUP BY a;
 
 DROP TABLE t;
 
+#
+# BUG#49902 - SELECT returns incorrect results
+#
+CREATE TABLE t1(a INT NOT NULL, b INT NOT NULL, KEY (b));
+INSERT INTO t1 VALUES(1,1),(2,1);
+ANALYZE TABLE t1;
+SELECT 1 AS c, b FROM t1 WHERE b IN (1,2) GROUP BY c, b;
+SELECT a FROM t1 WHERE b=1;
+DROP TABLE t1;
+
 --echo End of 5.1 tests

sql/opt_range.cc

@@ -1171,11 +1171,7 @@ QUICK_RANGE_SELECT::~QUICK_RANGE_SELECT()
     if (file) 
     {
       range_end();
-      if (head->key_read)
+      head->set_keyread(FALSE);
-      {
-        head->key_read= 0;
-        file->extra(HA_EXTRA_NO_KEYREAD);
-      }
       if (free_file)
       {
         DBUG_PRINT("info", ("Freeing separate handler 0x%lx (free: %d)", (long) file,
@@ -1377,10 +1373,7 @@ end:
   head->file= file;
   /* We don't have to set 'head->keyread' here as the 'file' is unique */
   if (!head->no_keyread)
-  {
-    head->key_read= 1;
     head->mark_columns_used_by_index(index);
-  }
   head->prepare_for_position();
   head->file= org_file;
   bitmap_copy(&column_bitmap, head->read_set);
@@ -8165,7 +8158,7 @@ int QUICK_INDEX_MERGE_SELECT::read_keys_and_merge()
   DBUG_ENTER("QUICK_INDEX_MERGE_SELECT::read_keys_and_merge");
 
   /* We're going to just read rowids. */
-  file->extra(HA_EXTRA_KEYREAD);
+  head->set_keyread(TRUE);
   head->prepare_for_position();
 
   cur_quick_it.rewind();
@@ -8241,7 +8234,7 @@ int QUICK_INDEX_MERGE_SELECT::read_keys_and_merge()
   delete unique;
   doing_pk_scan= FALSE;
   /* index_merge currently doesn't support "using index" at all */
-  file->extra(HA_EXTRA_NO_KEYREAD);
+  head->set_keyread(FALSE);
   init_read_record(&read_record, thd, head, (SQL_SELECT*) 0, 1 , 1, TRUE);
   DBUG_RETURN(result);
 }
@@ -10628,7 +10621,7 @@ int QUICK_GROUP_MIN_MAX_SELECT::reset(void)
   int result;
   DBUG_ENTER("QUICK_GROUP_MIN_MAX_SELECT::reset");
 
-  file->extra(HA_EXTRA_KEYREAD); /* We need only the key attributes */
+  head->set_keyread(TRUE); /* We need only the key attributes */
   if ((result= file->ha_index_init(index,1)))
     DBUG_RETURN(result);
   if (quick_prefix_select && quick_prefix_select->reset())

sql/opt_sum.cc

@@ -326,11 +326,7 @@ int opt_sum_query(TABLE_LIST *tables, List<Item> &all_fields,COND *conds)
 	  if (!error && reckey_in_range(0, &ref, item_field->field, 
 			                conds, range_fl, prefix_len))
 	    error= HA_ERR_KEY_NOT_FOUND;
-          if (table->key_read)
+          table->set_keyread(FALSE);
-          {
-            table->key_read= 0;
-            table->file->extra(HA_EXTRA_NO_KEYREAD);
-          }
           table->file->ha_index_end();
           if (error)
 	  {
@@ -413,11 +409,7 @@ int opt_sum_query(TABLE_LIST *tables, List<Item> &all_fields,COND *conds)
 	  if (!error && reckey_in_range(1, &ref, item_field->field,
 			                conds, range_fl, prefix_len))
 	    error= HA_ERR_KEY_NOT_FOUND;
-          if (table->key_read)
+          table->set_keyread(FALSE);
-          {
-            table->key_read=0;
-            table->file->extra(HA_EXTRA_NO_KEYREAD);
-          }
           table->file->ha_index_end();
           if (error)
           {
@@ -876,10 +868,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->key_read= 1;
-            table->file->extra(HA_EXTRA_KEYREAD);
-          }
           return 1;
         }
       }

sql/sql_select.cc

@@ -6594,10 +6594,7 @@ make_join_readinfo(JOIN *join, ulonglong options)
     case JT_CONST:				// Only happens with left join
       if (table->covering_keys.is_set(tab->ref.key) &&
 	  !table->no_keyread)
-      {
+        table->set_keyread(TRUE);
-	table->key_read=1;
-	table->file->extra(HA_EXTRA_KEYREAD);
-      }
       break;
     case JT_ALL:
       /*
@@ -6658,10 +6655,7 @@ make_join_readinfo(JOIN *join, ulonglong options)
 	  if (tab->select && tab->select->quick &&
               tab->select->quick->index != MAX_KEY && //not index_merge
 	      table->covering_keys.is_set(tab->select->quick->index))
-	  {
+            table->set_keyread(TRUE);
-	    table->key_read=1;
-	    table->file->extra(HA_EXTRA_KEYREAD);
-	  }
 	  else if (!table->covering_keys.is_clear_all() &&
 		   !(tab->select && tab->select->quick))
 	  {					// Only read index tree
@@ -6745,11 +6739,7 @@ void JOIN_TAB::cleanup()
   limit= 0;
   if (table)
   {
-    if (table->key_read)
+    table->set_keyread(FALSE);
-    {
-      table->key_read= 0;
-      table->file->extra(HA_EXTRA_NO_KEYREAD);
-    }
     table->file->ha_index_or_rnd_end();
     /*
       We need to reset this for next select
@@ -11595,16 +11585,11 @@ join_read_const_table(JOIN_TAB *tab, POSITION *pos)
 	!table->no_keyread &&
         (int) table->reginfo.lock_type <= (int) TL_READ_HIGH_PRIORITY)
     {
-      table->key_read=1;
+      table->set_keyread(TRUE);
-      table->file->extra(HA_EXTRA_KEYREAD);
       tab->index= tab->ref.key;
     }
     error=join_read_const(tab);
-    if (table->key_read)
+    table->set_keyread(FALSE);
-    {
-      table->key_read=0;
-      table->file->extra(HA_EXTRA_NO_KEYREAD);
-    }
     if (error)
     {
       tab->info="unique row not found";
@@ -11959,12 +11944,8 @@ join_read_first(JOIN_TAB *tab)
 {
   int error;
   TABLE *table=tab->table;
-  if (!table->key_read && table->covering_keys.is_set(tab->index) &&
+  if (table->covering_keys.is_set(tab->index) && !table->no_keyread)
-      !table->no_keyread)
+    table->set_keyread(TRUE);
-  {
-    table->key_read=1;
-    table->file->extra(HA_EXTRA_KEYREAD);
-  }
   tab->table->status=0;
   tab->read_record.read_record=join_read_next;
   tab->read_record.table=table;
@@ -11998,12 +11979,8 @@ join_read_last(JOIN_TAB *tab)
 {
   TABLE *table=tab->table;
   int error;
-  if (!table->key_read && table->covering_keys.is_set(tab->index) &&
+  if (table->covering_keys.is_set(tab->index) && !table->no_keyread)
-      !table->no_keyread)
+    table->set_keyread(TRUE);
-  {
-    table->key_read=1;
-    table->file->extra(HA_EXTRA_KEYREAD);
-  }
   tab->table->status=0;
   tab->read_record.read_record=join_read_prev;
   tab->read_record.table=table;
@@ -13413,11 +13390,8 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
            If ref_key used index tree reading only ('Using index' in EXPLAIN),
            and best_key doesn't, then revert the decision.
         */
-        if (!table->covering_keys.is_set(best_key) && table->key_read)
+        if (!table->covering_keys.is_set(best_key))
-        {
+          table->set_keyread(FALSE);
-          table->key_read= 0;
-          table->file->extra(HA_EXTRA_NO_KEYREAD);          
-        }        
         if (!quick_created)
 	{
           tab->index= best_key;
@@ -13430,10 +13404,7 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
             select->quick= 0;
           }
           if (table->covering_keys.is_set(best_key))
-          {
+            table->set_keyread(TRUE);
-            table->key_read=1;
-            table->file->extra(HA_EXTRA_KEYREAD);
-          }
           table->file->ha_index_or_rnd_end();
           if (join->select_options & SELECT_DESCRIBE)
           {
@@ -13607,11 +13578,8 @@ create_sort_index(THD *thd, JOIN *join, ORDER *order,
         We can only use 'Only index' if quick key is same as ref_key
         and in index_merge 'Only index' cannot be used
       */
-      if (table->key_read && ((uint) tab->ref.key != select->quick->index))
+      if (((uint) tab->ref.key != select->quick->index))
-      {
+        table->set_keyread(FALSE);
-	table->key_read=0;
-	table->file->extra(HA_EXTRA_NO_KEYREAD);
-      }
     }
     else
     {
@@ -13667,11 +13635,7 @@ create_sort_index(THD *thd, JOIN *join, ORDER *order,
   tab->type=JT_ALL;				// Read with normal read_record
   tab->read_first_record= join_init_read_record;
   tab->join->examined_rows+=examined_rows;
-  if (table->key_read)				// Restore if we used indexes
+  table->set_keyread(FALSE); // Restore if we used indexes
-  {
-    table->key_read=0;
-    table->file->extra(HA_EXTRA_NO_KEYREAD);
-  }
   DBUG_RETURN(table->sort.found_records == HA_POS_ERROR);
 err:
   DBUG_RETURN(-1);

sql/sql_update.cc

@@ -397,10 +397,7 @@ int mysql_update(THD *thd,
       matching rows before updating the table!
     */
     if (used_index < MAX_KEY && old_covering_keys.is_set(used_index))
-    {
-      table->key_read=1;
       table->mark_columns_used_by_index(used_index);
-    }
     else
     {
       table->use_all_columns();
@@ -844,11 +841,7 @@ int mysql_update(THD *thd,
 err:
   delete select;
   free_underlaid_joins(thd, select_lex);
-  if (table->key_read)
+  table->set_keyread(FALSE);
-  {
-    table->key_read=0;
-    table->file->extra(HA_EXTRA_NO_KEYREAD);
-  }
   thd->abort_on_warning= 0;
   DBUG_RETURN(1);
 }

sql/table.cc

@@ -4374,7 +4374,7 @@ void st_table::mark_columns_used_by_index(uint index)
   MY_BITMAP *bitmap= &tmp_set;
   DBUG_ENTER("st_table::mark_columns_used_by_index");
 
-  (void) file->extra(HA_EXTRA_KEYREAD);
+  set_keyread(TRUE);
   bitmap_clear_all(bitmap);
   mark_columns_used_by_index_no_reset(index, bitmap);
   column_bitmaps_set(bitmap, bitmap);
@@ -4397,8 +4397,7 @@ void st_table::restore_column_maps_after_mark_index()
 {
   DBUG_ENTER("st_table::restore_column_maps_after_mark_index");
 
-  key_read= 0;
+  set_keyread(FALSE);
-  (void) file->extra(HA_EXTRA_NO_KEYREAD);
   default_column_bitmaps();
   file->column_bitmaps_signal();
   DBUG_VOID_RETURN;

sql/table.h

@@ -902,6 +902,20 @@ struct st_table {
   inline bool needs_reopen_or_name_lock()
   { return s->version != refresh_version; }
   bool is_children_attached(void);
+  inline void set_keyread(bool flag)
+  {
+    DBUG_ASSERT(file);
+    if (flag && !key_read)
+    {
+      key_read= 1;
+      file->extra(HA_EXTRA_KEYREAD);
+    }
+    else if (!flag && key_read)
+    {
+      key_read= 0;
+      file->extra(HA_EXTRA_NO_KEYREAD);
+    }
+  }
 };
 
 enum enum_schema_table_state