Check that newIteratorWithBase regardless of WBWI Overwrite Mode (#8134)

Summary:
The behaviour of WBWI has changed when calling newIteratorWithBase when overwrite is set to true or false. This PR simply adds tests to assert the new correct behaviour.

Closes https://github.com/facebook/rocksdb/issues/7370
Closes https://github.com/facebook/rocksdb/pull/8134

Pull Request resolved: https://github.com/facebook/rocksdb/pull/9107

Reviewed By: pdillinger

Differential Revision: D32099475

Pulled By: mrambacher

fbshipit-source-id: 245f483f73db866cc8a51219a2bff2e09e59faa0

java/src/main/java/org/rocksdb/WriteBatchWithIndex.java

@@ -117,7 +117,7 @@ public class WriteBatchWithIndex extends AbstractWriteBatch {
    * 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
+   * We strongly recommend 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.
@@ -140,7 +140,7 @@ public class WriteBatchWithIndex extends AbstractWriteBatch {
    * 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
+   * We strongly recommend 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.

java/src/test/java/org/rocksdb/WriteBatchWithIndexTest.java

@@ -654,4 +654,106 @@ public class WriteBatchWithIndexTest {
       assertThat(db.get("key4".getBytes())).isEqualTo("xyz".getBytes());
     }
   }
+
+  @Test
+  public void iteratorWithBaseOverwriteTrue() throws RocksDBException {
+    try (final Options options = new Options().setCreateIfMissing(true);
+         final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath())) {
+      try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(true);
+           final RocksIterator baseIter = db.newIterator();
+           final RocksIterator wbwiIter = wbwi.newIteratorWithBase(baseIter)) {
+        assertThat(wbwiIter).isNotNull();
+        assertThat(wbwiIter.nativeHandle_).isGreaterThan(0);
+        wbwiIter.status();
+      }
+
+      try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(true);
+           final RocksIterator baseIter = db.newIterator();
+           final ReadOptions readOptions = new ReadOptions();
+           final RocksIterator wbwiIter = wbwi.newIteratorWithBase(baseIter, readOptions)) {
+        assertThat(wbwiIter).isNotNull();
+        assertThat(wbwiIter.nativeHandle_).isGreaterThan(0);
+        wbwiIter.status();
+      }
+    }
+
+    final List<ColumnFamilyDescriptor> cfNames =
+        Arrays.asList(new ColumnFamilyDescriptor(RocksDB.DEFAULT_COLUMN_FAMILY),
+            new ColumnFamilyDescriptor("new_cf".getBytes()));
+    final List<ColumnFamilyHandle> columnFamilyHandleList = new ArrayList<>();
+    try (final DBOptions options =
+             new DBOptions().setCreateIfMissing(true).setCreateMissingColumnFamilies(true);
+         final RocksDB db = RocksDB.open(
+             options, dbFolder.getRoot().getAbsolutePath(), cfNames, columnFamilyHandleList)) {
+      try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(true);
+           final RocksIterator baseIter = db.newIterator();
+           final RocksIterator wbwiIter =
+               wbwi.newIteratorWithBase(columnFamilyHandleList.get(1), baseIter)) {
+        assertThat(wbwiIter).isNotNull();
+        assertThat(wbwiIter.nativeHandle_).isGreaterThan(0);
+        wbwiIter.status();
+      }
+
+      try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(true);
+           final RocksIterator baseIter = db.newIterator();
+           final ReadOptions readOptions = new ReadOptions();
+           final RocksIterator wbwiIter =
+               wbwi.newIteratorWithBase(columnFamilyHandleList.get(1), baseIter, readOptions)) {
+        assertThat(wbwiIter).isNotNull();
+        assertThat(wbwiIter.nativeHandle_).isGreaterThan(0);
+        wbwiIter.status();
+      }
+    }
+  }
+
+  @Test
+  public void iteratorWithBaseOverwriteFalse() throws RocksDBException {
+    try (final Options options = new Options().setCreateIfMissing(true);
+         final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath())) {
+      try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(false);
+           final RocksIterator baseIter = db.newIterator();
+           final RocksIterator wbwiIter = wbwi.newIteratorWithBase(baseIter)) {
+        assertThat(wbwiIter).isNotNull();
+        assertThat(wbwiIter.nativeHandle_).isGreaterThan(0);
+        wbwiIter.status();
+      }
+
+      try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(false);
+           final RocksIterator baseIter = db.newIterator();
+           final ReadOptions readOptions = new ReadOptions();
+           final RocksIterator wbwiIter = wbwi.newIteratorWithBase(baseIter, readOptions)) {
+        assertThat(wbwiIter).isNotNull();
+        assertThat(wbwiIter.nativeHandle_).isGreaterThan(0);
+        wbwiIter.status();
+      }
+    }
+
+    final List<ColumnFamilyDescriptor> cfNames =
+        Arrays.asList(new ColumnFamilyDescriptor(RocksDB.DEFAULT_COLUMN_FAMILY),
+            new ColumnFamilyDescriptor("new_cf".getBytes()));
+    final List<ColumnFamilyHandle> columnFamilyHandleList = new ArrayList<>();
+    try (final DBOptions options =
+             new DBOptions().setCreateIfMissing(true).setCreateMissingColumnFamilies(true);
+         final RocksDB db = RocksDB.open(
+             options, dbFolder.getRoot().getAbsolutePath(), cfNames, columnFamilyHandleList)) {
+      try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(false);
+           final RocksIterator baseIter = db.newIterator();
+           final RocksIterator wbwiIter =
+               wbwi.newIteratorWithBase(columnFamilyHandleList.get(1), baseIter)) {
+        assertThat(wbwiIter).isNotNull();
+        assertThat(wbwiIter.nativeHandle_).isGreaterThan(0);
+        wbwiIter.status();
+      }
+
+      try (final WriteBatchWithIndex wbwi = new WriteBatchWithIndex(false);
+           final RocksIterator baseIter = db.newIterator();
+           final ReadOptions readOptions = new ReadOptions();
+           final RocksIterator wbwiIter =
+               wbwi.newIteratorWithBase(columnFamilyHandleList.get(1), baseIter, readOptions)) {
+        assertThat(wbwiIter).isNotNull();
+        assertThat(wbwiIter.nativeHandle_).isGreaterThan(0);
+        wbwiIter.status();
+      }
+    }
+  }
 }

utilities/write_batch_with_index/write_batch_with_index_test.cc

@@ -1634,6 +1634,20 @@ TEST_F(WBWIOverwriteTest, MutateWhileIteratingBaseStressTest) {
   ASSERT_OK(iter->status());
 }
 
+TEST_P(WriteBatchWithIndexTest, TestNewIteratorWithBaseFromWbwi) {
+  ColumnFamilyHandleImplDummy cf1(6, BytewiseComparator());
+  KVMap map;
+  map["a"] = "aa";
+  map["c"] = "cc";
+  map["e"] = "ee";
+  std::unique_ptr<Iterator> iter(
+      batch_->NewIteratorWithBase(&cf1, new KVIter(&map)));
+  ASSERT_NE(nullptr, iter);
+  iter->SeekToFirst();
+  ASSERT_TRUE(iter->Valid());
+  ASSERT_OK(iter->status());
+}
+
 TEST_P(WriteBatchWithIndexTest, SavePointTest) {
   ColumnFamilyHandleImplDummy cf1(1, BytewiseComparator());
   KVMap empty_map;