Bug#36775910: Record buffer not set in index range scans

The member, "m_expected_rows" defined as part of the
IndexRangeScanIterator is meant to be a "double" but
was unintentionally changed to "bool" as part of
refactoring done in range optimizer. Because of which,
innodb's record buffer setting was affected.
We are correcting it in this patch. (No test case added
as this might impact performance more than the correctness).

When the record buffer was enabled for index range scans
a problem arised with multi-valued indexes used for
covering index range scans. Note that this is only
possible when they are used as part of index merge
scans as index merge scans force covering index
scans. Problem is that the current implementation of
Field_typed_array::key_cmp() needs the value of the
generated column for the indexed expression, and this
column is not available in the multi-valued index, so
the storage engine cannot safely evaluate the end range
condition when filling the record buffer when it's a
covering scan.
So we disable using the record buffer when multi-valued
indexes are used for covering index range scans.

Change-Id: I889ef0b8b3da7b2234d27d5dd1dc69a8ae8a6557

Author: Chaithra Gopalareddy

sql/range_optimizer/index_range_scan.cc

@@ -59,18 +59,18 @@ IndexRangeScanIterator::IndexRangeScanIterator(
     MEM_ROOT *return_mem_root, uint mrr_flags, uint mrr_buf_size,
     Bounds_checked_array<QUICK_RANGE *> ranges_arg)
     : RowIDCapableRowIterator(thd, table_arg),
-      ranges(ranges_arg),
-      free_file(false),
-      cur_range(nullptr),
-      last_range(nullptr),
-      mrr_flags(mrr_flags),
-      mrr_buf_size(mrr_buf_size),
-      mrr_buf_desc(nullptr),
-      need_rows_in_rowid_order(need_rows_in_rowid_order),
-      reuse_handler(reuse_handler),
-      mem_root(return_mem_root),
-      m_expected_rows(expected_rows),
-      m_examined_rows(examined_rows) {
+      ranges{ranges_arg},
+      free_file{false},
+      cur_range{nullptr},
+      last_range{nullptr},
+      mrr_flags{mrr_flags},
+      mrr_buf_size{mrr_buf_size},
+      mrr_buf_desc{nullptr},
+      need_rows_in_rowid_order{need_rows_in_rowid_order},
+      reuse_handler{reuse_handler},
+      mem_root{return_mem_root},
+      m_expected_rows{expected_rows},
+      m_examined_rows{examined_rows} {
   DBUG_TRACE;
 
   in_ror_merged_scan = false;
@@ -232,11 +232,19 @@ bool IndexRangeScanIterator::Init() {
     // outside of the record. So don't request a buffer in this case, even
     // though the current read_set gives the impression that using a
     // record buffer would be fine.
-    const bool skip_record_buffer =
-        need_rows_in_rowid_order &&
-        Overlaps(table()->file->ha_table_flags(),
-                 HA_PRIMARY_KEY_REQUIRED_FOR_POSITION) &&
-        has_blob_primary_key(table());
+    bool skip_record_buffer = need_rows_in_rowid_order &&
+                              Overlaps(table()->file->ha_table_flags(),
+                                       HA_PRIMARY_KEY_REQUIRED_FOR_POSITION) &&
+                              has_blob_primary_key(table());
+    // Skip the record buffer for covering multi-valued index range scans.
+    // The current implementation of Field_typed_array::key_cmp() needs the
+    // value of the generated column for the indexed expression, and this
+    // column is not available in the multi-valued index, so the storage
+    // engine cannot safely evaluate the end range condition when filling the
+    // record buffer when it's a covering scan.
+    skip_record_buffer |=
+        Overlaps(table()->key_info[index].flags, HA_MULTI_VALUED_KEY) &&
+        table()->key_read;
     if (!skip_record_buffer) {
       if (set_record_buffer(table(), m_expected_rows)) {
         return true; /* purecov: inspected */

sql/range_optimizer/index_range_scan.h

@@ -99,7 +99,7 @@ class IndexRangeScanIterator : public RowIDCapableRowIterator {
  private:
   MEM_ROOT *mem_root;
   bool inited = false;
-  const bool m_expected_rows;
+  const double m_expected_rows;
   ha_rows *m_examined_rows;
 
   int cmp_next(QUICK_RANGE *range);

sql/range_optimizer/reverse_index_range_scan.cc

@@ -34,17 +34,17 @@
 
 ReverseIndexRangeScanIterator::ReverseIndexRangeScanIterator(
     THD *thd, TABLE *table, ha_rows *examined_rows, double expected_rows,
-    int index, MEM_ROOT *return_mem_root, uint mrr_flags,
+    uint index, MEM_ROOT *return_mem_root, uint mrr_flags,
     Bounds_checked_array<QUICK_RANGE *> ranges, bool using_extended_key_parts)
     : TableRowIterator(thd, table),
-      m_index(index),
-      m_expected_rows(expected_rows),
-      m_examined_rows(examined_rows),
-      mem_root(return_mem_root),
-      m_mrr_flags(mrr_flags),
-      ranges(ranges),
-      last_range(nullptr),
-      m_using_extended_key_parts(using_extended_key_parts) {
+      m_index{index},
+      m_expected_rows{expected_rows},
+      m_examined_rows{examined_rows},
+      mem_root{return_mem_root},
+      m_mrr_flags{mrr_flags},
+      ranges{ranges},
+      last_range{nullptr},
+      m_using_extended_key_parts{using_extended_key_parts} {
   /*
     Use default MRR implementation for reverse scans. No table engine
     currently can do an MRR scan with output in reverse index order.

sql/range_optimizer/reverse_index_range_scan.h

@@ -42,7 +42,7 @@
 class ReverseIndexRangeScanIterator : public TableRowIterator {
  public:
   ReverseIndexRangeScanIterator(THD *thd, TABLE *table, ha_rows *examined_rows,
-                                double expected_rows, int index,
+                                double expected_rows, uint index,
                                 MEM_ROOT *return_mem_root, uint mrr_flags,
                                 Bounds_checked_array<QUICK_RANGE *> ranges,
                                 bool using_extended_key_parts);
@@ -54,7 +54,7 @@ class ReverseIndexRangeScanIterator : public TableRowIterator {
   static range_seq_t quick_range_rev_seq_init(void *init_param, uint, uint);
 
   const uint m_index; /* Index this quick select uses */
-  ha_rows m_expected_rows;
+  const double m_expected_rows;
   ha_rows *m_examined_rows;
 
   MEM_ROOT *mem_root;