Range scan with equal predicates returns wrong results with bloom filter

[yoshinorim]

Range scan with equal predicates returns wrong results with bloom filter

[yoshinorim]

@spetrunia In the above example, do you know if there is any way to know the length of equal conditions at ha_rocksdb::index_read_map()? ("id1=1 AND id2=1 AND id3>=2" => equal cond length is 4 + 8 + 4=16 (index_id + 8 byte id1 + 4 byte id2), and "id1=1 AND id2>=1 AND id3>=2" => equal cond length is 4+8=12)

[spetrunia]

One can't see the end of the range inside ha_rocksdb::index_read_map() call.

We need to overload handler::read_range_first(). It sees both ends of the range, so we can compare both endpoints and find the length of the common prefix.

[yoshinorim]

Thanks @spetrunia! I'm going to fix it.

[rven1]

One point to note is that if the range spans more than one prefix for rocksdb, you cannot use the bloom filter.

On Sep 11, 2015, at 2:49 PM, Sergei Petrunia <[email protected]mailto:[email protected]> wrote:

One can't see the end of the range inside ha_rocksdb::index_read_map() call.

We need to overload handler::read_range_first(). It sees both ends of the range, so we can compare both endpoints and find the length of the common prefix.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/109#issuecomment-139669070.

[yoshinorim]

Handling IN clause is also interesting.

example: select * from r1 where id1=1 and id2 in (1,2,3);

read_range_first() is called three times, and both start_key and end_key matched on both id1 and id2. can_use_bloom_filter() is called only once at ha_rocksdb::setup_index_scan(), because Iterator is reused in this statement. MyRocks needs either of the following I think.

1. Do not include id2 for bloom filter. Only (index_id + id1=1) should be used. The problem is this is less efficient, and I'm not sure how to get that from read_range_first().
2. Decide if bloom filter can be used or not per each setup_index_scan().

[yoshinorim]

https://reviews.facebook.net/D46851

@spetrunia Could you please review this diff? This diff calls kd->pack_index_tuple() twice (for end key and start key), and I'm still not 100% confident that it is safe.

    end_key_packed_size= kd->pack_index_tuple(table, pack_buffer,
                                              end_key_packed_tuple, end_key->key,
                                              end_key->keypart_map);

    packed_size= kd->pack_index_tuple(table, pack_buffer,
                                      sec_key_packed_tuple, key,
                                      keypart_map);

[spetrunia]

@yoshinorim, reviewed, ok to push. pack_index_tuple modifies table->record[0](where the record is returned), but this should be ok because the column values will be decoded back from the storage format when the record is read.