From 53a86bf61e3598fc91bb5d8d3d5b9ee06ccab102 Mon Sep 17 00:00:00 2001
From: Andrew Kryczka <andrewkr@fb.com>
Date: Fri, 6 May 2016 11:27:28 -0700
Subject: [PATCH] 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
---
 util/options_helper.cc | 21 ++++++++++++---------
 util/options_test.cc   |  7 ++++++-
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/util/options_helper.cc b/util/options_helper.cc
index 77402188d..e42394133 100644
--- 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) {
-        return Status::InvalidArgument(
-            "unable to parse the specified CF option " + name);
-      }
-      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);
       }
-      new_options->compression_opts.max_dict_bytes =
+      end = value.find(':', start);
+      new_options->compression_opts.strategy =
           ParseInt(value.substr(start, value.size() - start));
+      // 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));
+      }
     } 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 9451bf580..131a80a79 100644
--- 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 =