From e503f5e0a05157ce201e70f6d86f66c6b0393aa8 Mon Sep 17 00:00:00 2001
From: Adam Retter <adam.retter@googlemail.com>
Date: Thu, 13 Aug 2020 20:42:06 -0700
Subject: [PATCH] RocksJava should not limit valid format_version (#7242)

Summary:
Previously RocksJava limited the format_version to 4. However, the C++ API is now at 5, and this will likely increase again in future. The Java API now allows any positive integer, and an exception is raised from JNI if the format_version is out-of-bounds.

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

Reviewed By: cheng-chang

Differential Revision: D23077941

Pulled By: pdillinger

fbshipit-source-id: ee69f7203448acddc41c6d86b470ed987d3d366d
---
 .../org/rocksdb/BlockBasedTableConfig.java    | 11 +++++----
 .../rocksdb/BlockBasedTableConfigTest.java    | 23 +++++++++++--------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/java/src/main/java/org/rocksdb/BlockBasedTableConfig.java b/java/src/main/java/org/rocksdb/BlockBasedTableConfig.java
index bbd2ca082..6730e6452 100644
--- a/java/src/main/java/org/rocksdb/BlockBasedTableConfig.java
+++ b/java/src/main/java/org/rocksdb/BlockBasedTableConfig.java
@@ -709,10 +709,13 @@ public class BlockBasedTableConfig extends TableFormatConfig {
    * <li><strong>4</strong> - Can be read by RocksDB's versions since 5.16. Changes the way we
    * encode the values in index blocks. If you don't plan to run RocksDB before
    * version 5.16 and you are using index_block_restart_interval &gt; 1, you should
-   * probably use this as it would reduce the index size.</li>
+   * probably use this as it would reduce the index size.
+   * This option only affects newly written tables. When reading existing
+   * tables, the information about version is read from the footer.</li>
+   * <li><strong>5</strong> - Can be read by RocksDB's versions since 6.6.0.
+   * Full and partitioned filters use a generally faster and more accurate
+   * Bloom filter implementation, with a different schema.</li>
    * </ul>
-   * <p> This option only affects newly written tables. When reading existing
-   * tables, the information about version is read from the footer.</p>
    *
    * @param formatVersion integer representing the version to be used.
    *
@@ -720,7 +723,7 @@ public class BlockBasedTableConfig extends TableFormatConfig {
    */
   public BlockBasedTableConfig setFormatVersion(
       final int formatVersion) {
-    assert(formatVersion >= 0 && formatVersion <= 4);
+    assert (formatVersion >= 0);
     this.formatVersion = formatVersion;
     return this;
   }
diff --git a/java/src/test/java/org/rocksdb/BlockBasedTableConfigTest.java b/java/src/test/java/org/rocksdb/BlockBasedTableConfigTest.java
index 4b5927ebe..560dcff4a 100644
--- a/java/src/test/java/org/rocksdb/BlockBasedTableConfigTest.java
+++ b/java/src/test/java/org/rocksdb/BlockBasedTableConfigTest.java
@@ -5,16 +5,16 @@
 
 package org.rocksdb;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.fail;
+
+import java.nio.charset.StandardCharsets;
 import org.junit.ClassRule;
 import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
-import java.nio.charset.StandardCharsets;
-
-import static org.assertj.core.api.Assertions.assertThat;
-
 public class BlockBasedTableConfigTest {
 
   @ClassRule
@@ -321,7 +321,7 @@ public class BlockBasedTableConfigTest {
   @Test
   public void formatVersion() {
     final BlockBasedTableConfig blockBasedTableConfig = new BlockBasedTableConfig();
-    for (int version = 0; version < 5; version++) {
+    for (int version = 0; version <= 5; version++) {
       blockBasedTableConfig.setFormatVersion(version);
       assertThat(blockBasedTableConfig.formatVersion()).isEqualTo(version);
     }
@@ -333,10 +333,15 @@ public class BlockBasedTableConfigTest {
     blockBasedTableConfig.setFormatVersion(-1);
   }
 
-  @Test(expected = AssertionError.class)
-  public void formatVersionFailIllegalVersion() {
-    final BlockBasedTableConfig blockBasedTableConfig = new BlockBasedTableConfig();
-    blockBasedTableConfig.setFormatVersion(99);
+  @Test(expected = RocksDBException.class)
+  public void invalidFormatVersion() throws RocksDBException {
+    final BlockBasedTableConfig blockBasedTableConfig =
+        new BlockBasedTableConfig().setFormatVersion(99999);
+
+    try (final Options options = new Options().setTableFormatConfig(blockBasedTableConfig);
+         final RocksDB db = RocksDB.open(options, dbFolder.getRoot().getAbsolutePath())) {
+      fail("Opening the database with an invalid format_version should have raised an exception");
+    }
   }
 
   @Test