revision-id: 71f5fa77faeae64d653012db88f0a9e224227d5f (fb-prod201903-170-g71f5fa77fae) parent(s): 40182db11b506f97183a8c41af747fb34eb239ec author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-10-12 00:09:11 +0300 message: Issue #790, MyRocks/MRR: address more review input Fix the code ha_rocksdb::read_row_from_secondary_key(). The idea is that index-only scans can now Pushed Index Conditions (ICP): one can run MRR scan with ICP, and MyRocks/MRR will use an index-only scan under the hood to collect the rowids. Previous variant of the code broke the "covered lookups" feature. --- storage/rocksdb/ha_rocksdb.cc | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 8019be03401..2735d9d6adb 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -7998,6 +7998,8 @@ int ha_rocksdb::read_row_from_secondary_key(uchar *const buf, int rc = 0; uint pk_size; + /* Get the key columns and primary key value */ + const rocksdb::Slice &rkey = m_scan_it->key(); const rocksdb::Slice &value = m_scan_it->value(); #ifndef DBUG_OFF @@ -8011,21 +8013,17 @@ int ha_rocksdb::read_row_from_secondary_key(uchar *const buf, #ifndef DBUG_OFF m_keyread_only = save_keyread_only; #endif + bool have_icp = (pushed_idx_cond && pushed_idx_cond_keyno == active_index); - if (covered_lookup && m_lock_rows == RDB_LOCK_NONE) { - // Due to MRR, we can have ICP enabled with covered_lookup == true - if (!(rc = find_icp_matching_index_rec(move_forward, buf))) { - const rocksdb::Slice &rkey = m_scan_it->key(); - const rocksdb::Slice &rval = m_scan_it->value(); - pk_size = - kd.get_primary_key_tuple(table, *m_pk_descr, &rkey, m_pk_packed_tuple); - if (pk_size == RDB_INVALID_KEY_LEN) { - rc = HA_ERR_ROCKSDB_CORRUPT_DATA; - } else { - rc = kd.unpack_record(table, buf, &rkey, &rval, - m_converter->get_verify_row_debug_checksums()); - global_stats.covered_secondary_key_lookups.inc(); - } + if (covered_lookup && m_lock_rows == RDB_LOCK_NONE && !have_icp) { + pk_size = + kd.get_primary_key_tuple(table, *m_pk_descr, &rkey, m_pk_packed_tuple); + if (pk_size == RDB_INVALID_KEY_LEN) { + rc = HA_ERR_ROCKSDB_CORRUPT_DATA; + } else { + rc = kd.unpack_record(table, buf, &rkey, &value, + m_converter->get_verify_row_debug_checksums()); + global_stats.covered_secondary_key_lookups.inc(); } } else { if (kd.m_is_reverse_cf) move_forward = !move_forward; @@ -8038,7 +8036,8 @@ int ha_rocksdb::read_row_from_secondary_key(uchar *const buf, if (pk_size == RDB_INVALID_KEY_LEN) { rc = HA_ERR_ROCKSDB_CORRUPT_DATA; } else { - rc = get_row_by_rowid(buf, m_pk_packed_tuple, pk_size); + if (!covered_lookup || m_lock_rows != RDB_LOCK_NONE) + rc = get_row_by_rowid(buf, m_pk_packed_tuple, pk_size); } } }