diff --git a/db/c.cc b/db/c.cc index 2bda00c41..87e08f806 100644 --- a/db/c.cc +++ b/db/c.cc @@ -1777,38 +1777,18 @@ void rocksdb_writebatch_wi_rollback_to_save_point(rocksdb_writebatch_wi_t* b, rocksdb_iterator_t* rocksdb_writebatch_wi_create_iterator_with_base( rocksdb_writebatch_wi_t* wbwi, rocksdb_iterator_t* base_iterator) { - rocksdb_readoptions_t options; - return rocksdb_writebatch_wi_create_iterator_with_base_and_readoptions( - &options, wbwi, base_iterator); -} - -rocksdb_iterator_t* rocksdb_writebatch_wi_create_iterator_with_base_cf( - rocksdb_writebatch_wi_t* wbwi, rocksdb_iterator_t* base_iterator, - rocksdb_column_family_handle_t* column_family) { - rocksdb_readoptions_t options; - return rocksdb_writebatch_wi_create_iterator_with_base_cf_and_readoptions( - &options, wbwi, base_iterator, column_family); -} - -rocksdb_iterator_t* -rocksdb_writebatch_wi_create_iterator_with_base_and_readoptions( - const rocksdb_readoptions_t* options, rocksdb_writebatch_wi_t* wbwi, - rocksdb_iterator_t* base_iterator) { rocksdb_iterator_t* result = new rocksdb_iterator_t; - result->rep = - wbwi->rep->NewIteratorWithBase(options->rep, base_iterator->rep); + result->rep = wbwi->rep->NewIteratorWithBase(base_iterator->rep); delete base_iterator; return result; } -rocksdb_iterator_t* -rocksdb_writebatch_wi_create_iterator_with_base_cf_and_readoptions( - const rocksdb_readoptions_t* options, rocksdb_writebatch_wi_t* wbwi, +rocksdb_iterator_t* rocksdb_writebatch_wi_create_iterator_with_base_cf( + rocksdb_writebatch_wi_t* wbwi, rocksdb_iterator_t* base_iterator, rocksdb_column_family_handle_t* column_family) { rocksdb_iterator_t* result = new rocksdb_iterator_t; - result->rep = wbwi->rep->NewIteratorWithBase(options->rep, column_family->rep, - base_iterator->rep); + result->rep = wbwi->rep->NewIteratorWithBase(column_family->rep, base_iterator->rep); delete base_iterator; return result; } diff --git a/db/c_test.c b/db/c_test.c index 76539e4b4..4f1c0da9f 100644 --- a/db/c_test.c +++ b/db/c_test.c @@ -917,9 +917,7 @@ int main(int argc, char** argv) { rocksdb_writebatch_wi_t* wbi = rocksdb_writebatch_wi_create(0, 1); rocksdb_writebatch_wi_put(wbi, "bar", 3, "b", 1); rocksdb_writebatch_wi_delete(wbi, "foo", 3); - rocksdb_iterator_t* iter = - rocksdb_writebatch_wi_create_iterator_with_base_and_readoptions( - roptions, wbi, base_iter); + rocksdb_iterator_t* iter = rocksdb_writebatch_wi_create_iterator_with_base(wbi, base_iter); CheckCondition(!rocksdb_iter_valid(iter)); rocksdb_iter_seek_to_first(iter); CheckCondition(rocksdb_iter_valid(iter)); @@ -1529,7 +1527,7 @@ int main(int argc, char** argv) { const rocksdb_snapshot_t* snapshot; snapshot = rocksdb_transactiondb_create_snapshot(txn_db); rocksdb_readoptions_set_snapshot(roptions, snapshot); - + rocksdb_transactiondb_put(txn_db, woptions, "foo", 3, "hey", 3, &err); CheckNoError(err); @@ -1645,7 +1643,6 @@ int main(int argc, char** argv) { // Check iterator with column family rocksdb_transaction_put_cf(txn, cfh1, "key1_cf", 7, "val1_cf", 7, &err); CheckNoError(err); - rocksdb_readoptions_set_iterate_upper_bound(roptions, "key2", 4); rocksdb_iterator_t* iter = rocksdb_transaction_create_iterator_cf(txn, roptions, cfh1); CheckCondition(!rocksdb_iter_valid(iter)); diff --git a/include/rocksdb/c.h b/include/rocksdb/c.h index e52c3fa8c..cf46054aa 100644 --- a/include/rocksdb/c.h +++ b/include/rocksdb/c.h @@ -636,14 +636,7 @@ extern ROCKSDB_LIBRARY_API rocksdb_iterator_t* rocksdb_writebatch_wi_create_iter rocksdb_writebatch_wi_t* wbwi, rocksdb_iterator_t* base_iterator, rocksdb_column_family_handle_t* cf); -extern ROCKSDB_LIBRARY_API rocksdb_iterator_t* -rocksdb_writebatch_wi_create_iterator_with_base_and_readoptions( - const rocksdb_readoptions_t* options, rocksdb_writebatch_wi_t* wbwi, - rocksdb_iterator_t* base_iterator); -extern ROCKSDB_LIBRARY_API rocksdb_iterator_t* -rocksdb_writebatch_wi_create_iterator_with_base_cf_and_readoptions( - const rocksdb_readoptions_t* options, rocksdb_writebatch_wi_t* wbwi, - rocksdb_iterator_t* base_iterator, rocksdb_column_family_handle_t* cf); + /* Block based table options */ diff --git a/include/rocksdb/iterator.h b/include/rocksdb/iterator.h index e1811bd26..e99b434a0 100644 --- a/include/rocksdb/iterator.h +++ b/include/rocksdb/iterator.h @@ -54,8 +54,6 @@ class Iterator : public Cleanable { // Position at the last key in the source that at or before target. // The iterator is Valid() after this call iff the source contains // an entry that comes at or before target. - // Note: If iterate_upper_bound is set and SeekForPrev is called with target - // greater or equal to iterate_upper_bound, the behavior is undefined virtual void SeekForPrev(const Slice& target) = 0; // Moves to the next entry in the source. After this call, Valid() is diff --git a/include/rocksdb/utilities/write_batch_with_index.h b/include/rocksdb/utilities/write_batch_with_index.h index 9c912d8f1..d25b9513b 100644 --- a/include/rocksdb/utilities/write_batch_with_index.h +++ b/include/rocksdb/utilities/write_batch_with_index.h @@ -161,15 +161,9 @@ class WriteBatchWithIndex : public WriteBatchBase { // key() and value() of the iterator. This invalidation happens even before // the write batch update finishes. The state may recover after Next() is // called. - Iterator* NewIteratorWithBase(const ReadOptions& read_options, - ColumnFamilyHandle* column_family, - Iterator* base_iterator); - Iterator* NewIteratorWithBase(ColumnFamilyHandle* column_family, Iterator* base_iterator); // default column family - Iterator* NewIteratorWithBase(const ReadOptions& read_options, - Iterator* base_iterator); Iterator* NewIteratorWithBase(Iterator* base_iterator); // Similar to DB::Get() but will only read the key from this batch. diff --git a/java/rocksjni/write_batch_with_index.cc b/java/rocksjni/write_batch_with_index.cc index 56e438a99..12ca299a9 100644 --- a/java/rocksjni/write_batch_with_index.cc +++ b/java/rocksjni/write_batch_with_index.cc @@ -457,14 +457,11 @@ jlong Java_org_rocksdb_WriteBatchWithIndex_iteratorWithBase(JNIEnv* /*env*/, jobject /*jobj*/, jlong jwbwi_handle, jlong jcf_handle, - jlong jbi_handle, - jlong jreadopt_handle) { + jlong jbi_handle) { auto* wbwi = reinterpret_cast(jwbwi_handle); auto* cf_handle = reinterpret_cast(jcf_handle); auto* base_iterator = reinterpret_cast(jbi_handle); - auto* readopt = reinterpret_cast(jreadopt_handle); - auto* iterator = - wbwi->NewIteratorWithBase(*readopt, cf_handle, base_iterator); + auto* iterator = wbwi->NewIteratorWithBase(cf_handle, base_iterator); return reinterpret_cast(iterator); } diff --git a/java/src/main/java/org/rocksdb/WriteBatchWithIndex.java b/java/src/main/java/org/rocksdb/WriteBatchWithIndex.java index e94a71205..2c0350837 100644 --- a/java/src/main/java/org/rocksdb/WriteBatchWithIndex.java +++ b/java/src/main/java/org/rocksdb/WriteBatchWithIndex.java @@ -14,9 +14,8 @@ package org.rocksdb; * * A user can call {@link org.rocksdb.WriteBatchWithIndex#newIterator()} to * create an iterator over the write batch or - * {@link org.rocksdb.WriteBatchWithIndex#newIteratorWithBase(org.rocksdb.ReadOptions, org.rocksdb.RocksIterator)} - * to get an iterator for the database with Read-Your-Own-Writes like - * capability + * {@link org.rocksdb.WriteBatchWithIndex#newIteratorWithBase(org.rocksdb.RocksIterator)} + * to get an iterator for the database with Read-Your-Own-Writes like capability */ public class WriteBatchWithIndex extends AbstractWriteBatch { /** @@ -110,34 +109,6 @@ public class WriteBatchWithIndex extends AbstractWriteBatch { return new WBWIRocksIterator(this, iterator0(nativeHandle_)); } - /** - * Provides Read-Your-Own-Writes like functionality by - * creating a new Iterator that will use {@link org.rocksdb.WBWIRocksIterator} - * as a delta and baseIterator as a base - * - * Updating write batch with the current key of the iterator is not safe. - * We strongly recommand users not to do it. It will invalidate the current - * key() and value() of the iterator. This invalidation happens even before - * the write batch update finishes. The state may recover after Next() is - * called. - * - * @param read_opts The read options to use - * @param columnFamilyHandle The column family to iterate over - * @param baseIterator The base iterator, - * e.g. {@link org.rocksdb.RocksDB#newIterator()} - * @return An iterator which shows a view comprised of both the database - * point-in-time from baseIterator and modifications made in this write batch. - */ - public RocksIterator newIteratorWithBase(final ReadOptions read_opts, - final ColumnFamilyHandle columnFamilyHandle, final RocksIterator baseIterator) { - RocksIterator iterator = new RocksIterator(baseIterator.parent_, - iteratorWithBase(nativeHandle_, columnFamilyHandle.nativeHandle_, - baseIterator.nativeHandle_, read_opts.nativeHandle_)); - //when the iterator is deleted it will also delete the baseIterator - baseIterator.disOwnNativeHandle(); - return iterator; - } - /** * Provides Read-Your-Own-Writes like functionality by * creating a new Iterator that will use {@link org.rocksdb.WBWIRocksIterator} @@ -158,26 +129,14 @@ public class WriteBatchWithIndex extends AbstractWriteBatch { public RocksIterator newIteratorWithBase( final ColumnFamilyHandle columnFamilyHandle, final RocksIterator baseIterator) { - ReadOptions read_opts = new ReadOptions(); - return newIteratorWithBase(read_opts, columnFamilyHandle, baseIterator); - } - - /** - * Provides Read-Your-Own-Writes like functionality by - * creating a new Iterator that will use {@link org.rocksdb.WBWIRocksIterator} - * as a delta and baseIterator as a base. Operates on the default column - * family. - * - * @param read_opts The read options to use - * @param baseIterator The base iterator, - * e.g. {@link org.rocksdb.RocksDB#newIterator()} - * @return An iterator which shows a view comprised of both the database - * point-in-timefrom baseIterator and modifications made in this write batch. - */ - public RocksIterator newIteratorWithBase( - final ReadOptions read_opts, final RocksIterator baseIterator) { - return newIteratorWithBase( - read_opts, baseIterator.parent_.getDefaultColumnFamily(), baseIterator); + RocksIterator iterator = new RocksIterator( + baseIterator.parent_, + iteratorWithBase(nativeHandle_, + columnFamilyHandle.nativeHandle_, + baseIterator.nativeHandle_)); + //when the iterator is deleted it will also delete the baseIterator + baseIterator.disOwnNativeHandle(); + return iterator; } /** @@ -192,8 +151,8 @@ public class WriteBatchWithIndex extends AbstractWriteBatch { * point-in-timefrom baseIterator and modifications made in this write batch. */ public RocksIterator newIteratorWithBase(final RocksIterator baseIterator) { - ReadOptions read_opts = new ReadOptions(); - return newIteratorWithBase(read_opts, baseIterator); + return newIteratorWithBase(baseIterator.parent_.getDefaultColumnFamily(), + baseIterator); } /** @@ -336,8 +295,8 @@ public class WriteBatchWithIndex extends AbstractWriteBatch { final boolean overwriteKey); private native long iterator0(final long handle); private native long iterator1(final long handle, final long cfHandle); - private native long iteratorWithBase(final long handle, final long baseIteratorHandle, - final long cfHandle, final long jreadopt_handle); + private native long iteratorWithBase(final long handle, + final long baseIteratorHandle, final long cfHandle); private native byte[] getFromBatch(final long handle, final long optHandle, final byte[] key, final int keyLen); private native byte[] getFromBatch(final long handle, final long optHandle, diff --git a/java/src/test/java/org/rocksdb/WriteBatchWithIndexTest.java b/java/src/test/java/org/rocksdb/WriteBatchWithIndexTest.java index 3a872c412..061af2b8f 100644 --- a/java/src/test/java/org/rocksdb/WriteBatchWithIndexTest.java +++ b/java/src/test/java/org/rocksdb/WriteBatchWithIndexTest.java @@ -47,6 +47,7 @@ public class WriteBatchWithIndexTest { try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(true); final RocksIterator base = db.newIterator(); final RocksIterator it = wbwi.newIteratorWithBase(base)) { + it.seek(k1); assertThat(it.isValid()).isTrue(); assertThat(it.key()).isEqualTo(k1); @@ -420,8 +421,8 @@ public class WriteBatchWithIndexTest { final ReadOptions readOptions, final WriteBatchWithIndex wbwi, final String skey) { final byte[] key = skey.getBytes(); - try (final RocksIterator baseIterator = db.newIterator(readOptions); - final RocksIterator iterator = wbwi.newIteratorWithBase(baseIterator)) { + try(final RocksIterator baseIterator = db.newIterator(readOptions); + final RocksIterator iterator = wbwi.newIteratorWithBase(baseIterator)) { iterator.seek(key); // Arrays.equals(key, iterator.key()) ensures an exact match in Rocks, diff --git a/utilities/transactions/optimistic_transaction_test.cc b/utilities/transactions/optimistic_transaction_test.cc index 12dd8377a..2c196d43b 100644 --- a/utilities/transactions/optimistic_transaction_test.cc +++ b/utilities/transactions/optimistic_transaction_test.cc @@ -852,51 +852,6 @@ TEST_F(OptimisticTransactionTest, UntrackedWrites) { delete txn; } -TEST_F(OptimisticTransactionTest, IteratorUpperBoundTest) { - WriteOptions write_options; - auto txn = unique_ptr(txn_db->BeginTransaction(write_options)); - - string key1 = "a1"; - string key2 = "a3"; - string key3 = "b1"; - string val = "123"; - txn->Put(key1, val); - txn->Put(key2, val); - - Status s = txn->Commit(); - ASSERT_OK(s); - txn = unique_ptr(txn_db->BeginTransaction(write_options)); - txn->Put(key3, val); - - string ubKey("a2"); - Slice upperbound(ubKey); - ReadOptions read_options; - read_options.iterate_upper_bound = &upperbound; - auto it = unique_ptr(txn->GetIterator(read_options)); - for (it->SeekToFirst(); it->Valid(); it->Next()) { - EXPECT_LT(it->key().ToString(), ubKey); - } - EXPECT_GE(it->key().ToString(), ubKey); - int key_count = 0; - for (it->SeekToFirst(); it->Valid(); it->Next()) { - EXPECT_LT(it->key().ToString(), ubKey); - key_count++; - } - ASSERT_EQ(key_count, 1); - // Test Seek to a key equal or over upper bound - it->Seek("a2"); - ASSERT_FALSE(it->Valid()); - it->Seek("a3"); - ASSERT_FALSE(it->Valid()); - it->Seek("a1"); - ASSERT_TRUE(it->Valid()); - it.reset(); - - s = txn->Commit(); - ASSERT_OK(s); - txn.reset(); -} - TEST_F(OptimisticTransactionTest, IteratorTest) { WriteOptions write_options; ReadOptions read_options, snapshot_read_options; diff --git a/utilities/transactions/transaction_base.cc b/utilities/transactions/transaction_base.cc index 049fc299c..ac459a256 100644 --- a/utilities/transactions/transaction_base.cc +++ b/utilities/transactions/transaction_base.cc @@ -178,7 +178,7 @@ Status TransactionBaseImpl::RollbackToSavePoint() { return Status::NotFound(); } } - + Status TransactionBaseImpl::PopSavePoint() { if (save_points_ == nullptr || save_points_->empty()) { @@ -187,7 +187,7 @@ Status TransactionBaseImpl::PopSavePoint() { return Status::NotFound(); } - assert(!save_points_->empty()); + assert(!save_points_->empty()); save_points_->pop(); return write_batch_.PopSavePoint(); } @@ -291,7 +291,7 @@ Iterator* TransactionBaseImpl::GetIterator(const ReadOptions& read_options) { Iterator* db_iter = db_->NewIterator(read_options); assert(db_iter); - return write_batch_.NewIteratorWithBase(read_options, db_iter); + return write_batch_.NewIteratorWithBase(db_iter); } Iterator* TransactionBaseImpl::GetIterator(const ReadOptions& read_options, @@ -299,7 +299,7 @@ Iterator* TransactionBaseImpl::GetIterator(const ReadOptions& read_options, Iterator* db_iter = db_->NewIterator(read_options, column_family); assert(db_iter); - return write_batch_.NewIteratorWithBase(read_options, column_family, db_iter); + return write_batch_.NewIteratorWithBase(column_family, db_iter); } Status TransactionBaseImpl::Put(ColumnFamilyHandle* column_family, diff --git a/utilities/transactions/write_prepared_txn.cc b/utilities/transactions/write_prepared_txn.cc index 112ab1829..cb20d1439 100644 --- a/utilities/transactions/write_prepared_txn.cc +++ b/utilities/transactions/write_prepared_txn.cc @@ -62,7 +62,7 @@ Iterator* WritePreparedTxn::GetIterator(const ReadOptions& options) { Iterator* db_iter = wpt_db_->NewIterator(options); assert(db_iter); - return write_batch_.NewIteratorWithBase(options, db_iter); + return write_batch_.NewIteratorWithBase(db_iter); } Iterator* WritePreparedTxn::GetIterator(const ReadOptions& options, @@ -71,7 +71,7 @@ Iterator* WritePreparedTxn::GetIterator(const ReadOptions& options, Iterator* db_iter = wpt_db_->NewIterator(options, column_family); assert(db_iter); - return write_batch_.NewIteratorWithBase(options, column_family, db_iter); + return write_batch_.NewIteratorWithBase(column_family, db_iter); } Status WritePreparedTxn::PrepareInternal() { diff --git a/utilities/transactions/write_unprepared_txn.cc b/utilities/transactions/write_unprepared_txn.cc index 4da4e4987..d4efe8ff9 100644 --- a/utilities/transactions/write_unprepared_txn.cc +++ b/utilities/transactions/write_unprepared_txn.cc @@ -506,7 +506,7 @@ Iterator* WriteUnpreparedTxn::GetIterator(const ReadOptions& options, Iterator* db_iter = wupt_db_->NewIterator(options, column_family, this); assert(db_iter); - return write_batch_.NewIteratorWithBase(options, column_family, db_iter); + return write_batch_.NewIteratorWithBase(column_family, db_iter); } const std::map& diff --git a/utilities/write_batch_with_index/write_batch_with_index.cc b/utilities/write_batch_with_index/write_batch_with_index.cc index 175cb2a98..2202d6baf 100644 --- a/utilities/write_batch_with_index/write_batch_with_index.cc +++ b/utilities/write_batch_with_index/write_batch_with_index.cc @@ -32,13 +32,11 @@ namespace rocksdb { // * equal_keys_ <=> base_iterator == delta_iterator class BaseDeltaIterator : public Iterator { public: - BaseDeltaIterator(const ReadOptions& read_options, Iterator* base_iterator, - WBWIIterator* delta_iterator, const Comparator* comparator) - : read_options_(read_options), - forward_(true), + BaseDeltaIterator(Iterator* base_iterator, WBWIIterator* delta_iterator, + const Comparator* comparator) + : forward_(true), current_at_base_(true), equal_keys_(false), - current_over_upper_bound_(false), status_(Status::OK()), base_iterator_(base_iterator), delta_iterator_(delta_iterator), @@ -47,7 +45,7 @@ class BaseDeltaIterator : public Iterator { virtual ~BaseDeltaIterator() {} bool Valid() const override { - return BaseDeltaValid() ? !current_over_upper_bound_ : false; + return current_at_base_ ? BaseValid() : DeltaValid(); } void SeekToFirst() override { @@ -218,15 +216,9 @@ class BaseDeltaIterator : public Iterator { } // equal_keys_ <=> compare == 0 assert((equal_keys_ || compare != 0) && (!equal_keys_ || compare == 0)); - #endif } - bool IsOverUpperBound() { - return read_options_.iterate_upper_bound != nullptr && - comparator_->Compare(key(), *read_options_.iterate_upper_bound) >= 0; - } - void Advance() { if (equal_keys_) { assert(BaseValid() && DeltaValid()); @@ -260,9 +252,6 @@ class BaseDeltaIterator : public Iterator { } bool BaseValid() const { return base_iterator_->Valid(); } bool DeltaValid() const { return delta_iterator_->Valid(); } - bool BaseDeltaValid() const { - return (current_at_base_ ? BaseValid() : DeltaValid()); - } void UpdateCurrent() { // Suppress false positive clang analyzer warnings. #ifndef __clang_analyzer__ @@ -275,37 +264,32 @@ class BaseDeltaIterator : public Iterator { } else if (!delta_iterator_->status().ok()) { // Expose the error status and stop. current_at_base_ = false; - break; + return; } equal_keys_ = false; if (!BaseValid()) { if (!base_iterator_->status().ok()) { // Expose the error status and stop. current_at_base_ = true; - break; + return; } // Base has finished. if (!DeltaValid()) { // Finished - break; + return; } if (delta_entry.type == kDeleteRecord || delta_entry.type == kSingleDeleteRecord) { AdvanceDelta(); - // If the new Delta is valid and >= iterate_upper_bound, stop - current_over_upper_bound_ = DeltaValid() && IsOverUpperBound(); - if (current_over_upper_bound_) { - return; - } } else { current_at_base_ = false; - break; + return; } } else if (!DeltaValid()) { // Delta has finished. current_at_base_ = true; - break; + return; } else { int compare = (forward_ ? 1 : -1) * @@ -317,7 +301,7 @@ class BaseDeltaIterator : public Iterator { if (delta_entry.type != kDeleteRecord && delta_entry.type != kSingleDeleteRecord) { current_at_base_ = false; - break; + return; } // Delta is less advanced and is delete. AdvanceDelta(); @@ -326,20 +310,18 @@ class BaseDeltaIterator : public Iterator { } } else { current_at_base_ = true; - break; + return; } } } - current_over_upper_bound_ = BaseDeltaValid() && IsOverUpperBound(); + AssertInvariants(); #endif // __clang_analyzer__ } - ReadOptions read_options_; bool forward_; bool current_at_base_; bool equal_keys_; - bool current_over_upper_bound_; Status status_; std::unique_ptr base_iterator_; std::unique_ptr delta_iterator_; @@ -660,39 +642,25 @@ WBWIIterator* WriteBatchWithIndex::NewIterator( } Iterator* WriteBatchWithIndex::NewIteratorWithBase( - const ReadOptions& read_options, ColumnFamilyHandle* column_family, - Iterator* base_iterator) { + ColumnFamilyHandle* column_family, Iterator* base_iterator) { if (rep->overwrite_key == false) { assert(false); return nullptr; } - return new BaseDeltaIterator(read_options, base_iterator, - NewIterator(column_family), + return new BaseDeltaIterator(base_iterator, NewIterator(column_family), GetColumnFamilyUserComparator(column_family)); } -Iterator* WriteBatchWithIndex::NewIteratorWithBase( - ColumnFamilyHandle* column_family, Iterator* base_iterator) { - ReadOptions read_options; - return NewIteratorWithBase(read_options, column_family, base_iterator); -} - -Iterator* WriteBatchWithIndex::NewIteratorWithBase( - const ReadOptions& read_options, Iterator* base_iterator) { +Iterator* WriteBatchWithIndex::NewIteratorWithBase(Iterator* base_iterator) { if (rep->overwrite_key == false) { assert(false); return nullptr; } // default column family's comparator - return new BaseDeltaIterator(read_options, base_iterator, NewIterator(), + return new BaseDeltaIterator(base_iterator, NewIterator(), rep->comparator.default_comparator()); } -Iterator* WriteBatchWithIndex::NewIteratorWithBase(Iterator* base_iterator) { - ReadOptions read_options; - return NewIteratorWithBase(read_options, base_iterator); -} - Status WriteBatchWithIndex::Put(ColumnFamilyHandle* column_family, const Slice& key, const Slice& value) { rep->SetLastEntryOffset(); diff --git a/utilities/write_batch_with_index/write_batch_with_index_test.cc b/utilities/write_batch_with_index/write_batch_with_index_test.cc index 94933295c..d477968ca 100644 --- a/utilities/write_batch_with_index/write_batch_with_index_test.cc +++ b/utilities/write_batch_with_index/write_batch_with_index_test.cc @@ -9,12 +9,11 @@ #ifndef ROCKSDB_LITE -#include "rocksdb/utilities/write_batch_with_index.h" -#include #include +#include #include "db/column_family.h" #include "port/stack_trace.h" -#include "rocksdb/comparator.h" +#include "rocksdb/utilities/write_batch_with_index.h" #include "util/random.h" #include "util/string_util.h" #include "util/testharness.h" @@ -507,19 +506,7 @@ typedef std::map KVMap; class KVIter : public Iterator { public: explicit KVIter(const KVMap* map) : map_(map), iter_(map_->end()) {} - explicit KVIter(const KVMap* map, const Slice* iterate_upper_bound) - : map_(map), - iter_(map_->end()), - iterate_upper_bound_(iterate_upper_bound) {} - virtual bool Valid() const { - if (iterate_upper_bound_ == nullptr) { - return iter_ != map_->end(); - } else { - if (iter_ == map_->end()) return false; - const Comparator* cmp = BytewiseComparator(); - return cmp->Compare(key(), *iterate_upper_bound_) < 0; - } - } + virtual bool Valid() const { return iter_ != map_->end(); } virtual void SeekToFirst() { iter_ = map_->begin(); } virtual void SeekToLast() { if (map_->empty()) { @@ -549,7 +536,6 @@ class KVIter : public Iterator { private: const KVMap* const map_; KVMap::const_iterator iter_; - const Slice* iterate_upper_bound_ = nullptr; }; void AssertIter(Iterator* iter, const std::string& key, @@ -567,7 +553,6 @@ void AssertItersEqual(Iterator* iter1, Iterator* iter2) { ASSERT_EQ(iter1->value().ToString(), iter2->value().ToString()); } } - } // namespace TEST_F(WriteBatchWithIndexTest, TestRandomIteraratorWithBase) { @@ -628,14 +613,9 @@ TEST_F(WriteBatchWithIndexTest, TestRandomIteraratorWithBase) { } } - auto rnd_key_idx = rnd.Uniform(static_cast(source_strings.size())); - Slice random_upper_bound(source_strings[rnd_key_idx]); - ReadOptions read_options; - read_options.iterate_upper_bound = &random_upper_bound; std::unique_ptr iter( - batch.NewIteratorWithBase(read_options, &cf1, new KVIter(&map))); - std::unique_ptr result_iter( - new KVIter(&merged_map, &random_upper_bound)); + batch.NewIteratorWithBase(&cf1, new KVIter(&map))); + std::unique_ptr result_iter(new KVIter(&merged_map)); bool is_valid = false; for (int i = 0; i < 128; i++) { @@ -857,46 +837,6 @@ TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBase) { iter->Prev(); AssertIter(iter.get(), "c", "cc"); } - - // Test iterate_upper_bound - { - KVMap empty_map; - Slice upper_bound("cd"); - ReadOptions read_options; - read_options.iterate_upper_bound = &upper_bound; - std::unique_ptr iter( - batch.NewIteratorWithBase(read_options, &cf1, new KVIter(&empty_map))); - - iter->SeekToFirst(); - AssertIter(iter.get(), "a", "aa"); - iter->Next(); - AssertIter(iter.get(), "c", "cc"); - iter->Next(); - ASSERT_OK(iter->status()); - ASSERT_TRUE(!iter->Valid()); - - iter->SeekToLast(); - ASSERT_OK(iter->status()); - ASSERT_TRUE(!iter->Valid()); - - iter->Seek("aa"); - AssertIter(iter.get(), "c", "cc"); - iter->Prev(); - AssertIter(iter.get(), "a", "aa"); - iter->Next(); - iter->Next(); - ASSERT_OK(iter->status()); - ASSERT_TRUE(!iter->Valid()); - - iter->Seek("ca"); - ASSERT_OK(iter->status()); - ASSERT_TRUE(!iter->Valid()); - - // Seek to outside of upper bound, should not crash - iter->Seek("zz"); - ASSERT_OK(iter->status()); - ASSERT_TRUE(!iter->Valid()); - } } TEST_F(WriteBatchWithIndexTest, TestIteraratorWithBaseReverseCmp) {