Make max_dict_bytes optional in options string

Summary:
For backwards compatibility with older option strings, the parser needs
to treat this argument as optional.

Test Plan:
Updated unit test to cover case where compression_opts is present but
max_dict_bytes is omitted.

Reviewers: MarkCallaghan, sdong, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D57759

diff --git a/util/options_helper.cc b/util/options_helper.cc
index 77402188debec9cd0a208f62fa58cfa25d86c325..e4239413356af8d1feb3af7612cbc03f4a3e3880 100644 (file)
--- a/util/options_helper.cc
+++ b/util/options_helper.cc
@@ -806,20 +806,23 @@ Status ParseColumnFamilyOption(const std::string& name,
       new_options->compression_opts.level =
           ParseInt(value.substr(start, end - start));
       start = end + 1;
-      end = value.find(':', start);
-      if (end == std::string::npos) {
+      if (start >= value.size()) {
         return Status::InvalidArgument(
             "unable to parse the specified CF option " + name);
       }
+      end = value.find(':', start);
       new_options->compression_opts.strategy =
           ParseInt(value.substr(start, value.size() - start));
-      start = end + 1;
-      if (start >= value.size()) {
-        return Status::InvalidArgument(
-            "unable to parse the specified CF option " + name);
+      // max_dict_bytes is optional for backwards compatibility
+      if (end != std::string::npos) {
+        start = end + 1;
+        if (start >= value.size()) {
+          return Status::InvalidArgument(
+              "unable to parse the specified CF option " + name);
+        }
+        new_options->compression_opts.max_dict_bytes =
+            ParseInt(value.substr(start, value.size() - start));
       }
-      new_options->compression_opts.max_dict_bytes =
-          ParseInt(value.substr(start, value.size() - start));
     } else if (name == "compaction_options_fifo") {
       new_options->compaction_options_fifo.max_table_files_size =
           ParseUint64(value);

diff --git a/util/options_test.cc b/util/options_test.cc
index 9451bf5804d9f0b920f7a5ef22b778acc55b9c77..131a80a799fd0b731fe0394f184512a967114e67 100644 (file)
--- a/util/options_test.cc
+++ b/util/options_test.cc
@@ -600,9 +600,14 @@ TEST_F(OptionsTest, GetOptionsFromStringTest) {
       base_options,
       "write_buffer_size=10;max_write_buffer_number=16;"
       "block_based_table_factory={block_cache=1M;block_size=4;};"
-      "create_if_missing=true;max_open_files=1;rate_limiter_bytes_per_sec=1024",
+      "compression_opts=4:5:6;create_if_missing=true;max_open_files=1;"
+      "rate_limiter_bytes_per_sec=1024",
       &new_options));
 
+  ASSERT_EQ(new_options.compression_opts.window_bits, 4);
+  ASSERT_EQ(new_options.compression_opts.level, 5);
+  ASSERT_EQ(new_options.compression_opts.strategy, 6);
+  ASSERT_EQ(new_options.compression_opts.max_dict_bytes, 0);
   ASSERT_EQ(new_options.write_buffer_size, 10U);
   ASSERT_EQ(new_options.max_write_buffer_number, 16);
   BlockBasedTableOptions new_block_based_table_options =