diff --git a/db/db_iter.cc b/db/db_iter.cc index 2eae6ae21..f2d94f70e 100644 --- a/db/db_iter.cc +++ b/db/db_iter.cc @@ -223,11 +223,13 @@ bool DBIter::FindNextUserEntryInternal(bool skipping_saved_key, is_key_seqnum_zero_ = (ikey_.sequence == 0); - assert(iterate_upper_bound_ == nullptr || iter_.MayBeOutOfUpperBound() || + assert(iterate_upper_bound_ == nullptr || + iter_.UpperBoundCheckResult() != IterBoundCheck::kInbound || user_comparator_.CompareWithoutTimestamp( ikey_.user_key, /*a_has_ts=*/true, *iterate_upper_bound_, /*b_has_ts=*/false) < 0); - if (iterate_upper_bound_ != nullptr && iter_.MayBeOutOfUpperBound() && + if (iterate_upper_bound_ != nullptr && + iter_.UpperBoundCheckResult() != IterBoundCheck::kInbound && user_comparator_.CompareWithoutTimestamp( ikey_.user_key, /*a_has_ts=*/true, *iterate_upper_bound_, /*b_has_ts=*/false) >= 0) { diff --git a/db/memtable.cc b/db/memtable.cc index 422433459..2f3b32aa8 100644 --- a/db/memtable.cc +++ b/db/memtable.cc @@ -382,7 +382,7 @@ class MemTableIterator : public InternalIterator { bool is_valid = valid_; if (is_valid) { result->key = key(); - result->may_be_out_of_upper_bound = true; + result->bound_check_result = IterBoundCheck::kUnknown; result->value_prepared = true; } return is_valid; diff --git a/db/version_set.cc b/db/version_set.cc index 5bbed8d18..2c8817cae 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -942,9 +942,12 @@ class LevelIterator final : public InternalIterator { return may_be_out_of_lower_bound_ && file_iter_.MayBeOutOfLowerBound(); } - inline bool MayBeOutOfUpperBound() override { - assert(Valid()); - return file_iter_.MayBeOutOfUpperBound(); + inline IterBoundCheck UpperBoundCheckResult() override { + if (Valid()) { + return file_iter_.UpperBoundCheckResult(); + } else { + return IterBoundCheck::kUnknown; + } } void SetPinnedItersMgr(PinnedIteratorsManager* pinned_iters_mgr) override { @@ -1148,7 +1151,7 @@ bool LevelIterator::NextAndGetResult(IterateResult* result) { is_valid = Valid(); if (is_valid) { result->key = key(); - result->may_be_out_of_upper_bound = MayBeOutOfUpperBound(); + result->bound_check_result = file_iter_.UpperBoundCheckResult(); // Ideally, we should return the real file_iter_.value_prepared but the // information is not here. It would casue an extra PrepareValue() // for the first key of a file. @@ -1168,7 +1171,8 @@ bool LevelIterator::SkipEmptyFileForward() { bool seen_empty_file = false; while (file_iter_.iter() == nullptr || (!file_iter_.Valid() && file_iter_.status().ok() && - !file_iter_.iter()->IsOutOfBound())) { + file_iter_.iter()->UpperBoundCheckResult() != + IterBoundCheck::kOutOfBound)) { seen_empty_file = true; // Move to next file if (file_index_ >= flevel_->num_files - 1) { diff --git a/table/block_based/block_based_table_iterator.cc b/table/block_based/block_based_table_iterator.cc index af81799a0..da854e873 100644 --- a/table/block_based/block_based_table_iterator.cc +++ b/table/block_based/block_based_table_iterator.cc @@ -189,7 +189,7 @@ bool BlockBasedTableIterator::NextAndGetResult(IterateResult* result) { bool is_valid = Valid(); if (is_valid) { result->key = key(); - result->may_be_out_of_upper_bound = MayBeOutOfUpperBound(); + result->bound_check_result = UpperBoundCheckResult(); result->value_prepared = !is_at_first_key_from_index_; } return is_valid; diff --git a/table/block_based/block_based_table_iterator.h b/table/block_based/block_based_table_iterator.h index ac3f47b68..e1f150573 100644 --- a/table/block_based/block_based_table_iterator.h +++ b/table/block_based/block_based_table_iterator.h @@ -99,13 +99,16 @@ class BlockBasedTableIterator : public InternalIteratorBase { } } - // Whether iterator invalidated for being out of bound. - bool IsOutOfBound() override { return is_out_of_bound_; } - - inline bool MayBeOutOfUpperBound() override { - assert(Valid()); - return block_upper_bound_check_ != - BlockUpperBound::kUpperBoundBeyondCurBlock; + inline IterBoundCheck UpperBoundCheckResult() override { + if (is_out_of_bound_) { + return IterBoundCheck::kOutOfBound; + } else if (block_upper_bound_check_ == + BlockUpperBound::kUpperBoundBeyondCurBlock) { + assert(!is_out_of_bound_); + return IterBoundCheck::kInbound; + } else { + return IterBoundCheck::kUnknown; + } } void SetPinnedItersMgr(PinnedIteratorsManager* pinned_iters_mgr) override { diff --git a/table/block_based/partitioned_index_iterator.h b/table/block_based/partitioned_index_iterator.h index 747c0553a..cdb8cf86d 100644 --- a/table/block_based/partitioned_index_iterator.h +++ b/table/block_based/partitioned_index_iterator.h @@ -78,18 +78,10 @@ class ParititionedIndexIterator : public InternalIteratorBase { return Status::OK(); } } - - // Whether iterator invalidated for being out of bound. - bool IsOutOfBound() override { - // Shoulldn't be called - assert(false); - return false; - } - - inline bool MayBeOutOfUpperBound() override { + inline IterBoundCheck UpperBoundCheckResult() override { // Shouldn't be called. assert(false); - return true; + return IterBoundCheck::kUnknown; } void SetPinnedItersMgr(PinnedIteratorsManager*) override { // Shouldn't be called. diff --git a/table/internal_iterator.h b/table/internal_iterator.h index 829cb2fc8..c4382a54e 100644 --- a/table/internal_iterator.h +++ b/table/internal_iterator.h @@ -17,9 +17,15 @@ namespace ROCKSDB_NAMESPACE { class PinnedIteratorsManager; +enum class IterBoundCheck : char { + kUnknown = 0, + kOutOfBound, + kInbound, +}; + struct IterateResult { Slice key; - bool may_be_out_of_upper_bound; + IterBoundCheck bound_check_result = IterBoundCheck::kUnknown; // If false, PrepareValue() needs to be called before value(). bool value_prepared = true; }; @@ -69,7 +75,7 @@ class InternalIteratorBase : public Cleanable { // Moves to the next entry in the source, and return result. Iterator // implementation should override this method to help methods inline better, - // or when MayBeOutOfUpperBound() is non-trivial. + // or when UpperBoundCheckResult() is non-trivial. // REQUIRES: Valid() virtual bool NextAndGetResult(IterateResult* result) { Next(); @@ -77,11 +83,11 @@ class InternalIteratorBase : public Cleanable { if (is_valid) { result->key = key(); // Default may_be_out_of_upper_bound to true to avoid unnecessary virtual - // call. If an implementation has non-trivial MayBeOutOfUpperBound(), + // call. If an implementation has non-trivial UpperBoundCheckResult(), // it should also override NextAndGetResult(). - result->may_be_out_of_upper_bound = true; + result->bound_check_result = IterBoundCheck::kUnknown; result->value_prepared = false; - assert(MayBeOutOfUpperBound()); + assert(UpperBoundCheckResult() != IterBoundCheck::kOutOfBound); } return is_valid; } @@ -126,19 +132,17 @@ class InternalIteratorBase : public Cleanable { // REQUIRES: Valid() virtual bool PrepareValue() { return true; } - // True if the iterator is invalidated because it reached a key that is above - // the iterator upper bound. Used by LevelIterator to decide whether it should - // stop or move on to the next file. - // Important: if iterator reached the end of the file without encountering any - // keys above the upper bound, IsOutOfBound() must return false. - virtual bool IsOutOfBound() { return false; } - // Keys return from this iterator can be smaller than iterate_lower_bound. virtual bool MayBeOutOfLowerBound() { return true; } - // Keys return from this iterator can be larger or equal to - // iterate_upper_bound. - virtual bool MayBeOutOfUpperBound() { return true; } + // If the iterator has checked the key against iterate_upper_bound, returns + // the result here. The function can be used by user of the iterator to skip + // their own checks. If Valid() = true, IterBoundCheck::kUnknown is always + // a valid value. If Valid() = false, IterBoundCheck::kOutOfBound indicates + // that the iterator is filtered out by upper bound checks. + virtual IterBoundCheck UpperBoundCheckResult() { + return IterBoundCheck::kUnknown; + } // Pass the PinnedIteratorsManager to the Iterator, most Iterators don't // communicate with PinnedIteratorsManager so default implementation is no-op diff --git a/table/iterator_wrapper.h b/table/iterator_wrapper.h index 010a23abf..6444b0a25 100644 --- a/table/iterator_wrapper.h +++ b/table/iterator_wrapper.h @@ -127,9 +127,9 @@ class IteratorWrapperBase { return iter_->MayBeOutOfLowerBound(); } - bool MayBeOutOfUpperBound() { + IterBoundCheck UpperBoundCheckResult() { assert(Valid()); - return result_.may_be_out_of_upper_bound; + return result_.bound_check_result; } void SetPinnedItersMgr(PinnedIteratorsManager* pinned_iters_mgr) { @@ -155,7 +155,7 @@ class IteratorWrapperBase { if (valid_) { assert(iter_->status().ok()); result_.key = iter_->key(); - result_.may_be_out_of_upper_bound = true; + result_.bound_check_result = IterBoundCheck::kUnknown; result_.value_prepared = false; } } diff --git a/table/merging_iterator.cc b/table/merging_iterator.cc index 695a03013..60830c128 100644 --- a/table/merging_iterator.cc +++ b/table/merging_iterator.cc @@ -194,7 +194,7 @@ class MergingIterator : public InternalIterator { bool is_valid = Valid(); if (is_valid) { result->key = key(); - result->may_be_out_of_upper_bound = MayBeOutOfUpperBound(); + result->bound_check_result = UpperBoundCheckResult(); result->value_prepared = current_->IsValuePrepared(); } return is_valid; @@ -261,9 +261,9 @@ class MergingIterator : public InternalIterator { return current_->MayBeOutOfLowerBound(); } - bool MayBeOutOfUpperBound() override { + IterBoundCheck UpperBoundCheckResult() override { assert(Valid()); - return current_->MayBeOutOfUpperBound(); + return current_->UpperBoundCheckResult(); } void SetPinnedItersMgr(PinnedIteratorsManager* pinned_iters_mgr) override { diff --git a/table/table_test.cc b/table/table_test.cc index 760154822..7e9e7098a 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -241,7 +241,9 @@ class KeyConvertingIterator : public InternalIterator { void SeekToLast() override { iter_->SeekToLast(); } void Next() override { iter_->Next(); } void Prev() override { iter_->Prev(); } - bool IsOutOfBound() override { return iter_->IsOutOfBound(); } + IterBoundCheck UpperBoundCheckResult() override { + return iter_->UpperBoundCheckResult(); + } Slice key() const override { assert(Valid()); @@ -4645,13 +4647,13 @@ TEST_P(BlockBasedTableTest, OutOfBoundOnSeek) { /*skip_filters=*/false, TableReaderCaller::kUncategorized))); iter->SeekToFirst(); ASSERT_FALSE(iter->Valid()); - ASSERT_TRUE(iter->IsOutOfBound()); + ASSERT_TRUE(iter->UpperBoundCheckResult() == IterBoundCheck::kOutOfBound); iter.reset(new KeyConvertingIterator(reader->NewIterator( read_opt, /*prefix_extractor=*/nullptr, /*arena=*/nullptr, /*skip_filters=*/false, TableReaderCaller::kUncategorized))); iter->Seek("foo"); ASSERT_FALSE(iter->Valid()); - ASSERT_TRUE(iter->IsOutOfBound()); + ASSERT_TRUE(iter->UpperBoundCheckResult() == IterBoundCheck::kOutOfBound); } // BlockBasedTableIterator should invalidate itself and return @@ -4686,7 +4688,7 @@ TEST_P(BlockBasedTableTest, OutOfBoundOnNext) { ASSERT_EQ("bar", iter->key()); iter->Next(); ASSERT_FALSE(iter->Valid()); - ASSERT_TRUE(iter->IsOutOfBound()); + ASSERT_TRUE(iter->UpperBoundCheckResult() == IterBoundCheck::kOutOfBound); std::string ub2 = "foo_after"; Slice ub_slice2(ub2); read_opt.iterate_upper_bound = &ub_slice2; @@ -4698,7 +4700,7 @@ TEST_P(BlockBasedTableTest, OutOfBoundOnNext) { ASSERT_EQ("foo", iter->key()); iter->Next(); ASSERT_FALSE(iter->Valid()); - ASSERT_FALSE(iter->IsOutOfBound()); + ASSERT_FALSE(iter->UpperBoundCheckResult() == IterBoundCheck::kOutOfBound); } } // namespace ROCKSDB_NAMESPACE