From 0c433cd1eb27930868128e396ee2e75689565698 Mon Sep 17 00:00:00 2001 From: Islam AbdelRahman Date: Tue, 26 Jan 2016 14:47:42 -0800 Subject: [PATCH] Fix issue in Iterator::Seek when using Block based filter block with prefix_extractor Summary: Similar to D53385 we need to check InDomain before checking the filter block. Test Plan: unit tests Reviewers: yhchiang, rven, sdong Reviewed By: sdong Subscribers: dhruba Differential Revision: https://reviews.facebook.net/D53421 --- db/db_test.cc | 34 +++++++++++++++++++++++++++++++ table/block_based_table_reader.cc | 7 +++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/db/db_test.cc b/db/db_test.cc index 5111c05ce..1296bad06 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -10082,6 +10082,7 @@ class SliceTransformLimitedDomain : public SliceTransform { TEST_F(DBTest, PrefixExtractorFullFilter) { BlockBasedTableOptions bbto; + // Full Filter Block bbto.filter_policy.reset(rocksdb::NewBloomFilterPolicy(10, false)); bbto.whole_key_filtering = false; @@ -10108,6 +10109,39 @@ TEST_F(DBTest, PrefixExtractorFullFilter) { ASSERT_EQ(Get("zzzzz_AAAA"), "val5"); } +TEST_F(DBTest, PrefixExtractorBlockFilter) { + BlockBasedTableOptions bbto; + // Block Filter Block + bbto.filter_policy.reset(rocksdb::NewBloomFilterPolicy(10, true)); + + Options options = CurrentOptions(); + options.prefix_extractor = std::make_shared(); + options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + + DestroyAndReopen(options); + + ASSERT_OK(Put("x1113_AAAA", "val3")); + ASSERT_OK(Put("x1114_AAAA", "val4")); + // Not in domain, wont be added to filter + ASSERT_OK(Put("zzzzz_AAAA", "val1")); + ASSERT_OK(Put("zzzzz_AAAB", "val2")); + ASSERT_OK(Put("zzzzz_AAAC", "val3")); + ASSERT_OK(Put("zzzzz_AAAD", "val4")); + + ASSERT_OK(Flush()); + + std::vector iter_res; + auto iter = db_->NewIterator(ReadOptions()); + // Seek to a key that was not in Domain + for (iter->Seek("zzzzz_AAAA"); iter->Valid(); iter->Next()) { + iter_res.emplace_back(iter->value().ToString()); + } + + std::vector expected_res = {"val1", "val2", "val3", "val4"}; + ASSERT_EQ(iter_res, expected_res); + delete iter; +} + #ifndef ROCKSDB_LITE class BloomStatsTestWithParam : public DBTest, diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc index 3114a6f15..00997f3ea 100644 --- a/table/block_based_table_reader.cc +++ b/table/block_based_table_reader.cc @@ -1120,8 +1120,11 @@ bool BlockBasedTable::PrefixMayMatch(const Slice& internal_key) { } assert(rep_->ioptions.prefix_extractor != nullptr); - auto prefix = rep_->ioptions.prefix_extractor->Transform( - ExtractUserKey(internal_key)); + auto user_key = ExtractUserKey(internal_key); + if (!rep_->ioptions.prefix_extractor->InDomain(user_key)) { + return true; + } + auto prefix = rep_->ioptions.prefix_extractor->Transform(user_key); InternalKey internal_key_prefix(prefix, kMaxSequenceNumber, kTypeValue); auto internal_prefix = internal_key_prefix.Encode();