Skip to content

Commit

Permalink
Use the generic iterator interface instead of multi_cf_iterator for u…
Browse files Browse the repository at this point in the history
…nified experience
  • Loading branch information
jaykorean committed Mar 4, 2024
1 parent de2f338 commit d221bf3
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 91 deletions.
18 changes: 9 additions & 9 deletions db/db_impl/db_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
// found in the LICENSE file. See the AUTHORS file for names of contributors.
#include "db/db_impl/db_impl.h"

#include <cstdint>
#include <stdint.h>

#include <cstdint>
#ifdef OS_SOLARIS
#include <alloca.h>
#endif
Expand Down Expand Up @@ -81,7 +82,6 @@
#include "rocksdb/db.h"
#include "rocksdb/env.h"
#include "rocksdb/merge_operator.h"
#include "rocksdb/multi_cf_iterator.h"
#include "rocksdb/statistics.h"
#include "rocksdb/stats_history.h"
#include "rocksdb/status.h"
Expand Down Expand Up @@ -3737,20 +3737,20 @@ ArenaWrappedDBIter* DBImpl::NewIteratorImpl(
return db_iter;
}

std::unique_ptr<MultiCfIterator> DBImpl::NewMultiCfIterator(
std::unique_ptr<Iterator> DBImpl::NewMultiCfIterator(
const ReadOptions& _read_options,
const std::vector<ColumnFamilyHandle*>& column_families) {
if (column_families.size() < 2) {
return std::make_unique<EmptyMultiCfIterator>(
Status::InvalidArgument("Less than 2 CFs were provided"));
if (column_families.size() == 0) {
return std::unique_ptr<Iterator>(NewErrorIterator(
Status::InvalidArgument("No Column Family was provided")));
}
const Comparator* first_comparator = column_families[0]->GetComparator();
for (size_t i = 1; i < column_families.size(); ++i) {
const Comparator* cf_comparator = column_families[i]->GetComparator();
if (first_comparator != cf_comparator &&
first_comparator->GetId().compare(cf_comparator->GetId()) != 0) {
return std::make_unique<EmptyMultiCfIterator>(Status::InvalidArgument(
"Different comparators are being used across CFs"));
return std::unique_ptr<Iterator>(NewErrorIterator(Status::InvalidArgument(
"Different comparators are being used across CFs")));
}
}
std::vector<Iterator*> child_iterators;
Expand All @@ -3759,7 +3759,7 @@ std::unique_ptr<MultiCfIterator> DBImpl::NewMultiCfIterator(
return std::make_unique<MultiCfIteratorImpl>(
first_comparator, column_families, std::move(child_iterators));
}
return std::make_unique<EmptyMultiCfIterator>(s);
return std::unique_ptr<Iterator>(NewErrorIterator(s));
}

Status DBImpl::NewIterators(
Expand Down
2 changes: 1 addition & 1 deletion db/db_impl/db_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ class DBImpl : public DB {

// UNDER CONSTRUCTION - DO NOT USE
// Return a cross-column-family iterator from a consistent database state.
std::unique_ptr<MultiCfIterator> NewMultiCfIterator(
std::unique_ptr<Iterator> NewMultiCfIterator(
const ReadOptions& options,
const std::vector<ColumnFamilyHandle*>& column_families) override;

Expand Down
2 changes: 1 addition & 1 deletion db/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3201,7 +3201,7 @@ class ModelDB : public DB {
}

// UNDER CONSTRUCTION - DO NOT USE
std::unique_ptr<MultiCfIterator> NewMultiCfIterator(
std::unique_ptr<Iterator> NewMultiCfIterator(
const ReadOptions& /*options*/,
const std::vector<ColumnFamilyHandle*>& /*column_families*/) override {
return nullptr;
Expand Down
34 changes: 4 additions & 30 deletions db/multi_cf_iterator_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@

#pragma once

#include "rocksdb/multi_cf_iterator.h"
#include "rocksdb/comparator.h"
#include "rocksdb/iterator.h"
#include "rocksdb/options.h"
#include "util/heap.h"

namespace ROCKSDB_NAMESPACE {
Expand All @@ -15,7 +17,7 @@ namespace ROCKSDB_NAMESPACE {
// When the same key exists in more than one column families, the iterator
// selects the value from the first column family containing the key, in the
// order provided in the `column_families` parameter.
class MultiCfIteratorImpl : public MultiCfIterator {
class MultiCfIteratorImpl : public Iterator {
public:
MultiCfIteratorImpl(const Comparator* comparator,
const std::vector<ColumnFamilyHandle*>& column_families,
Expand Down Expand Up @@ -116,32 +118,4 @@ class MultiCfIteratorImpl : public MultiCfIterator {
}
};

class EmptyMultiCfIterator : public MultiCfIterator {
public:
explicit EmptyMultiCfIterator(const Status& s) : status_(s) {}
bool Valid() const override { return false; }
void Seek(const Slice& /*target*/) override {}
void SeekForPrev(const Slice& /*target*/) override {}
void SeekToFirst() override {}
void SeekToLast() override {}
void Next() override { assert(false); }
void Prev() override { assert(false); }
Slice key() const override {
assert(false);
return Slice();
}
Slice value() const override {
assert(false);
return Slice();
}
const AttributeGroups& attribute_groups() const override {
assert(false);
return kNoAttributeGroups;
}
Status status() const override { return status_; }

private:
Status status_;
};

} // namespace ROCKSDB_NAMESPACE
14 changes: 4 additions & 10 deletions db/multi_cf_iterator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class MultiCfIteratorTest : public DBTestBase {
const std::optional<std::vector<AttributeGroups>>&
expected_attribute_groups = std::nullopt) {
int i = 0;
std::unique_ptr<MultiCfIterator> iter =
std::unique_ptr<Iterator> iter =
db_->NewMultiCfIterator(ReadOptions(), cfhs);
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
ASSERT_EQ(expected_keys[i], iter->key());
Expand Down Expand Up @@ -66,16 +66,10 @@ TEST_F(MultiCfIteratorTest, InvalidArguments) {
CreateAndReopenWithCF({"cf_1", "cf_2", "cf_3"}, options);

// Invalid - No CF is provided
std::unique_ptr<MultiCfIterator> iter_with_no_cf =
std::unique_ptr<Iterator> iter_with_no_cf =
db_->NewMultiCfIterator(ReadOptions(), {});
ASSERT_NOK(iter_with_no_cf->status());
ASSERT_TRUE(iter_with_no_cf->status().IsInvalidArgument());

// Invalid - Only one CF is provided
std::unique_ptr<MultiCfIterator> iter_with_one_cf =
db_->NewMultiCfIterator(ReadOptions(), {handles_[0]});
ASSERT_NOK(iter_with_one_cf->status());
ASSERT_TRUE(iter_with_one_cf->status().IsInvalidArgument());
}
}

Expand Down Expand Up @@ -233,7 +227,7 @@ TEST_F(MultiCfIteratorTest, DifferentComparatorsInMultiCFs) {
verifyExpectedKeys(handles_[0], {"key_1", "key_2", "key_3"});
verifyExpectedKeys(handles_[1], {"key_3", "key_2", "key_1"});

std::unique_ptr<MultiCfIterator> iter =
std::unique_ptr<Iterator> iter =
db_->NewMultiCfIterator(ReadOptions(), handles_);
ASSERT_NOK(iter->status());
ASSERT_TRUE(iter->status().IsInvalidArgument());
Expand Down Expand Up @@ -286,7 +280,7 @@ TEST_F(MultiCfIteratorTest, CustomComparatorsInMultiCFs) {
"value_0_4", "value_0_5", "value_0_6",
"value_1_4", "value_1_5", "value_1_6"};
int i = 0;
std::unique_ptr<MultiCfIterator> iter =
std::unique_ptr<Iterator> iter =
db_->NewMultiCfIterator(ReadOptions(), handles_);
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
ASSERT_EQ(expected_keys[i], iter->key());
Expand Down
3 changes: 1 addition & 2 deletions include/rocksdb/db.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "rocksdb/iterator.h"
#include "rocksdb/listener.h"
#include "rocksdb/metadata.h"
#include "rocksdb/multi_cf_iterator.h"
#include "rocksdb/options.h"
#include "rocksdb/snapshot.h"
#include "rocksdb/sst_file_writer.h"
Expand Down Expand Up @@ -961,7 +960,7 @@ class DB {
// When the same key is present in multiple column families, the iterator
// selects the value or columns from the first column family containing the
// key, in the order specified by the `column_families` parameter.
virtual std::unique_ptr<MultiCfIterator> NewMultiCfIterator(
virtual std::unique_ptr<Iterator> NewMultiCfIterator(
const ReadOptions& options,
const std::vector<ColumnFamilyHandle*>& column_families) = 0;

Expand Down
7 changes: 7 additions & 0 deletions include/rocksdb/iterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "rocksdb/slice.h"
#include "rocksdb/status.h"
#include "rocksdb/wide_columns.h"
#include "wide_columns.h"

namespace ROCKSDB_NAMESPACE {

Expand Down Expand Up @@ -102,6 +103,12 @@ class Iterator : public Cleanable {
return kNoWideColumns;
}

// UNDER CONSTRUCTION - DO NOT USE
virtual const AttributeGroups& attribute_groups() const {
assert(false);
return kNoAttributeGroups;
}

// If an error has occurred, return it. Else return an ok status.
// If non-blocking IO is requested and this operation cannot be
// satisfied without doing some IO, then this returns Status::Incomplete().
Expand Down
37 changes: 0 additions & 37 deletions include/rocksdb/multi_cf_iterator.h

This file was deleted.

2 changes: 1 addition & 1 deletion include/rocksdb/utilities/stackable_db.h
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ class StackableDB : public DB {
}

using DB::NewMultiCfIterator;
std::unique_ptr<MultiCfIterator> NewMultiCfIterator(
std::unique_ptr<Iterator> NewMultiCfIterator(
const ReadOptions& options,
const std::vector<ColumnFamilyHandle*>& column_families) override {
return db_->NewMultiCfIterator(options, column_families);
Expand Down

0 comments on commit d221bf3

Please sign in to comment.