Skip to content

Commit

Permalink
ran make format
Browse files Browse the repository at this point in the history
  • Loading branch information
@adamretter
adamretter committed Aug 11, 2020
1 parent 49e5946 commit 4abb167
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 50 deletions.
8 changes: 2 additions & 6 deletions include/rocksdb/iterator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,18 +123,14 @@ class Iterator : public Cleanable {
* checking ReadOptions::iterate_lower_bound,
* false otherwise.
*/
virtual bool has_lower_bound() const {
return false;
}
virtual bool has_lower_bound() const { return false; }

/**
* true if this iterator is already
* checking ReadOptions::iterate_upper_bound,
* false otherwise.
*/
virtual bool has_upper_bound() const {
return false;
}
virtual bool has_upper_bound() const { return false; }
};

// Return an empty iterator (yields nothing).
Expand Down
33 changes: 16 additions & 17 deletions utilities/write_batch_with_index/write_batch_with_index.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ class BaseDeltaIterator : public Iterator {
progress_ = Progress::SEEK_TO_LAST;

// is there an upper bound constraint?
if (read_options_ != nullptr && read_options_->iterate_upper_bound != nullptr) {

if (read_options_ != nullptr &&
read_options_->iterate_upper_bound != nullptr) {
// yes, and is base_iterator already constrained by an upper_bound?
if (!base_iterator_->has_upper_bound()) {
// no, so we have to seek it to before iterate_upper_bound
Expand All @@ -71,7 +71,8 @@ class BaseDeltaIterator : public Iterator {
base_iterator_->Prev(); // upper bound should be exclusive!
}
} else {
// yes, so the base_iterator will take care of iterate_upper_bound for us
// yes, so the base_iterator will take care of iterate_upper_bound for
// us
base_iterator_->SeekToLast();
}

Expand Down Expand Up @@ -130,7 +131,6 @@ class BaseDeltaIterator : public Iterator {
delta_iterator_->SeekToFirst();
}
} else {

progress_ = Progress::FORWARD;
if (current_at_base_) {
// Change delta from larger than base to smaller
Expand All @@ -142,7 +142,7 @@ class BaseDeltaIterator : public Iterator {

if (DeltaValid() && BaseValid()) {
if (comparator_->Equal(delta_iterator_->Entry().key,
base_iterator_->key())) {
base_iterator_->key())) {
equal_keys_ = true;
}
}
Expand Down Expand Up @@ -179,7 +179,6 @@ class BaseDeltaIterator : public Iterator {
delta_iterator_->SeekToLast();
}
} else {

progress_ = Progress::BACKWARD;
if (current_at_base_) {
// Change delta from less advanced than base to more advanced
Expand Down Expand Up @@ -309,10 +308,13 @@ class BaseDeltaIterator : public Iterator {
// base_iterator if the base iterator has an
// upper_bounds_check already
return base_iterator_->Valid() &&
(base_iterator_->has_upper_bound() ? true : IsWithinBounds(base_iterator_->key()));
(base_iterator_->has_upper_bound()
? true
: IsWithinBounds(base_iterator_->key()));
}
bool DeltaValid() const {
return delta_iterator_->Valid() && IsWithinBounds(delta_iterator_->Entry().key);
return delta_iterator_->Valid() &&
IsWithinBounds(delta_iterator_->Entry().key);
}
void UpdateCurrent() {
// Suppress false positive clang analyzer warnings.
Expand Down Expand Up @@ -358,7 +360,6 @@ class BaseDeltaIterator : public Iterator {
return;

} else {

// Base and Delta are both unfinished.

int compare =
Expand Down Expand Up @@ -395,24 +396,22 @@ class BaseDeltaIterator : public Iterator {
if (read_options_ != nullptr) {
// TODO(AR) should this only be used when moving backward?
if (read_options_->iterate_lower_bound != nullptr) {
return comparator_->Compare(key, *(read_options_->iterate_lower_bound)) >= 0;
return comparator_->Compare(key,
*(read_options_->iterate_lower_bound)) >= 0;
}

// TODO(AR) should this only be used when moving forward?
if (read_options_->iterate_upper_bound != nullptr) {
return comparator_->Compare(key, *(read_options_->iterate_upper_bound)) < 0;
return comparator_->Compare(key,
*(read_options_->iterate_upper_bound)) < 0;
}
}
return true;
}

inline bool IsMovingForward() const {
return progress_ < Progress::BACKWARD;
}
inline bool IsMovingForward() const { return progress_ < Progress::BACKWARD; }

inline bool IsMovingBackward() const {
return progress_ > Progress::FORWARD;
}
inline bool IsMovingBackward() const { return progress_ > Progress::FORWARD; }

enum Progress {
TO_BE_DETERMINED = 0,
Expand Down
70 changes: 43 additions & 27 deletions utilities/write_batch_with_index/write_batch_with_index_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -521,11 +521,12 @@ class KVIter : public Iterator {
} else {
if (read_options_ != nullptr &&
read_options_->iterate_upper_bound != nullptr) {

// we can seek to before the iterate_upper_bound

// NOTE: std::map::lower_bound is equivalent to RocksDB's `iterate_upper_bound`
iter_ = map_->lower_bound(read_options_->iterate_upper_bound->ToString());
// NOTE: std::map::lower_bound is equivalent to RocksDB's
// `iterate_upper_bound`
iter_ =
map_->lower_bound(read_options_->iterate_upper_bound->ToString());
if (iter_ != map_->begin()) {
// lower_bound gives us the first element not
// less than the `iterate_upper_bound` so we have
Expand Down Expand Up @@ -561,11 +562,13 @@ class KVIter : public Iterator {
Status status() const override { return Status::OK(); }

bool has_lower_bound() const override {
return read_options_ != nullptr && read_options_->iterate_lower_bound != nullptr;
return read_options_ != nullptr &&
read_options_->iterate_lower_bound != nullptr;
};

bool has_upper_bound() const override {
return read_options_ != nullptr && read_options_->iterate_upper_bound != nullptr;
return read_options_ != nullptr &&
read_options_->iterate_upper_bound != nullptr;
};

private:
Expand All @@ -578,36 +581,43 @@ class KVIter : public Iterator {
if (read_options_ != nullptr) {
// TODO(AR) should this only be used when moving backward?
if (read_options_->iterate_lower_bound != nullptr) {
return comparator_->Compare(iter_->first, *(read_options_->iterate_lower_bound)) >= 0;
return comparator_->Compare(iter_->first,
*(read_options_->iterate_lower_bound)) >= 0;
}

// TODO(AR) should this only be used when moving forward?
if (read_options_->iterate_upper_bound != nullptr) {
return comparator_->Compare(iter_->first, *(read_options_->iterate_upper_bound)) < 0;
return comparator_->Compare(iter_->first,
*(read_options_->iterate_upper_bound)) < 0;
}
}

return true;
}
};

::testing::AssertionResult IterEquals(Iterator* iter,
const std::string& key,const std::string& value) {
::testing::AssertionResult IterEquals(Iterator* iter, const std::string& key,
const std::string& value) {
auto s = iter->status();
if (!s.ok()) {
return ::testing::AssertionFailure() << "Iterator NOT OK; status is: " << s.ToString();
return ::testing::AssertionFailure()
<< "Iterator NOT OK; status is: " << s.ToString();
}

if (!iter->Valid()) {
return ::testing::AssertionFailure() << "Iterator is invalid";
}

if (key != iter->key()) {
return ::testing::AssertionFailure() << "Iterator::key(): '" << iter->key().ToString(false) << "' is not equal to '" << key << "'";
return ::testing::AssertionFailure()
<< "Iterator::key(): '" << iter->key().ToString(false)
<< "' is not equal to '" << key << "'";
}

if (value != iter->value()) {
return ::testing::AssertionFailure() << "Iterator::value(): '" << iter->value().ToString(false) << "' is not equal to '" << value << "'";
return ::testing::AssertionFailure()
<< "Iterator::value(): '" << iter->value().ToString(false)
<< "' is not equal to '" << value << "'";
}

return ::testing::AssertionSuccess();
Expand Down Expand Up @@ -1211,7 +1221,8 @@ TEST_F(WriteBatchWithIndexTest,
ASSERT_FALSE(iter->Valid()) << "Should have reached upper_bound";
}

TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseSeekToLastOnBaseAndBatch) {
TEST_F(WriteBatchWithIndexTest,
TestIteraratorWithBaseSeekToLastOnBaseAndBatch) {
ColumnFamilyHandleImplDummy cf1(6, BytewiseComparator());
WriteBatchWithIndex batch(BytewiseComparator(), 0, true);

Expand All @@ -1224,8 +1235,8 @@ TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseSeekToLastOnBaseAndBatch)
batch.Put(&cf1, "k05", "v05");
batch.Put(&cf1, "k06", "v06");

std::unique_ptr<Iterator> iter(batch.NewIteratorWithBase(
&cf1, new KVIter(&base, BytewiseComparator())));
std::unique_ptr<Iterator> iter(
batch.NewIteratorWithBase(&cf1, new KVIter(&base, BytewiseComparator())));

ASSERT_OK(iter->status());

Expand Down Expand Up @@ -1307,7 +1318,8 @@ TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseSeekToLastOnBaseAndBatch)
ASSERT_FALSE(iter->Valid()) << "Should have reached end";
}

TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseSeekToLastOnBatchAndBase) {
TEST_F(WriteBatchWithIndexTest,
TestIteraratorWithBaseSeekToLastOnBatchAndBase) {
ColumnFamilyHandleImplDummy cf1(6, BytewiseComparator());
WriteBatchWithIndex batch(BytewiseComparator(), 0, true);

Expand All @@ -1320,8 +1332,8 @@ TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseSeekToLastOnBatchAndBase)
batch.Put(&cf1, "k02", "v02");
batch.Put(&cf1, "k03", "v03");

std::unique_ptr<Iterator> iter(batch.NewIteratorWithBase(
&cf1, new KVIter(&base, BytewiseComparator())));
std::unique_ptr<Iterator> iter(
batch.NewIteratorWithBase(&cf1, new KVIter(&base, BytewiseComparator())));

ASSERT_OK(iter->status());

Expand Down Expand Up @@ -1399,7 +1411,8 @@ TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseSeekToLastOnBatchAndBase)
ASSERT_TRUE(IterEquals(iter.get(), "k06", "v06"));
}

TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseUpperBoundOnBaseWithoutBaseConstraint) {
TEST_F(WriteBatchWithIndexTest,
TestIteraratorWithBaseUpperBoundOnBaseWithoutBaseConstraint) {
ColumnFamilyHandleImplDummy cf1(6, BytewiseComparator());
WriteBatchWithIndex batch(BytewiseComparator(), 0, true);

Expand All @@ -1416,10 +1429,10 @@ TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseUpperBoundOnBaseWithoutBas
ReadOptions read_options;
read_options.iterate_upper_bound = &upper_bound;

// NOTE: read_options are NOT passed to KVIter, so WBWIIterator imposes iterate_upper_bound on base
// NOTE: read_options are NOT passed to KVIter, so WBWIIterator imposes
// iterate_upper_bound on base
std::unique_ptr<Iterator> iter(batch.NewIteratorWithBase(
&cf1, new KVIter(&base, BytewiseComparator()),
&read_options));
&cf1, new KVIter(&base, BytewiseComparator()), &read_options));

ASSERT_OK(iter->status());

Expand Down Expand Up @@ -1460,7 +1473,8 @@ TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseUpperBoundOnBaseWithoutBas
ASSERT_FALSE(iter->Valid()) << "Should have reached upper_bound";
}

TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseUpperBoundOnBaseWithBaseConstraint) {
TEST_F(WriteBatchWithIndexTest,
TestIteraratorWithBaseUpperBoundOnBaseWithBaseConstraint) {
ColumnFamilyHandleImplDummy cf1(6, BytewiseComparator());
WriteBatchWithIndex batch(BytewiseComparator(), 0, true);

Expand All @@ -1477,7 +1491,8 @@ TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseUpperBoundOnBaseWithBaseCo
ReadOptions read_options;
read_options.iterate_upper_bound = &upper_bound;

// NOTE: read_options are also passed to KVIter, so KVIter imposes iterate_upper_bound on base
// NOTE: read_options are also passed to KVIter, so KVIter imposes
// iterate_upper_bound on base
std::unique_ptr<Iterator> iter(batch.NewIteratorWithBase(
&cf1, new KVIter(&base, BytewiseComparator(), &read_options),
&read_options));
Expand Down Expand Up @@ -1538,8 +1553,9 @@ TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseUpperBoundOnBatch) {
read_options.iterate_upper_bound = &upper_bound;

KVMap empty_map;
std::unique_ptr<Iterator> iter(
batch.NewIteratorWithBase(&cf1, new KVIter(&empty_map, BytewiseComparator(), &read_options), &read_options));
std::unique_ptr<Iterator> iter(batch.NewIteratorWithBase(
&cf1, new KVIter(&empty_map, BytewiseComparator(), &read_options),
&read_options));

ASSERT_OK(iter->status());

Expand Down

0 comments on commit 4abb167

Please sign in to comment.