Separate dwrf config definition from impl (#3393)

Summary:
Pull Request resolved: #3393

Separate the config definition from the actual mechanism. Rename the impl class as `ConfigBase` and concrete config classes will derive `ConfigBase<ConcreteConfig>` like `dwrf::Config` and just define all the entries.

Reviewed By: xiaoxmeng, Yuhta

Differential Revision: D41658954

fbshipit-source-id: d3fa63c0a6bb6d86701142fa3be834fc590ba338

velox/common/config/Config.h

@@ -0,0 +1,77 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <functional>
+#include <map>
+#include <unordered_map>
+#include "folly/Conv.h"
+
+namespace facebook::velox::common {
+// The concrete config class would inherit the config base
+// and then just define all the entries.
+template <class ConcreteConfig>
+class ConfigBase {
+ public:
+  template <typename T>
+  class Entry {
+    Entry(
+        const std::string& key,
+        const T& val,
+        std::function<std::string(const T&)> toStr =
+            [](const T& val) { return folly::to<std::string>(val); },
+        std::function<T(const std::string&)> toT =
+            [](const std::string& val) { return folly::to<T>(val); })
+        : key_{key}, default_{val}, toStr_{toStr}, toT_{toT} {}
+
+    const std::string key_;
+    const T default_;
+    const std::function<std::string(const T&)> toStr_;
+    const std::function<T(const std::string&)> toT_;
+
+    friend ConfigBase;
+    friend ConcreteConfig;
+  };
+
+  template <typename T>
+  ConfigBase& set(const Entry<T>& entry, const T& val) {
+    configs_[entry.key_] = entry.toStr_(val);
+    return *this;
+  }
+
+  template <typename T>
+  ConfigBase& unset(const Entry<T>& entry) {
+    configs_.erase(entry.key_);
+    return *this;
+  }
+
+  ConfigBase& reset() {
+    configs_.clear();
+    return *this;
+  }
+
+  template <typename T>
+  T get(const Entry<T>& entry) const {
+    auto iter = configs_.find(entry.key_);
+    return iter != configs_.end() ? entry.toT_(iter->second) : entry.default_;
+  }
+
+ protected:
+  std::unordered_map<std::string, std::string> configs_;
+};
+
+} // namespace facebook::velox::common

velox/dwio/dwrf/common/Config.h

@@ -18,65 +18,16 @@
 
 #include <functional>
 #include <unordered_map>
-#include "folly/Conv.h"
+#include "velox/common/config/Config.h"
 #include "velox/dwio/common/Common.h"
 #include "velox/dwio/dwrf/common/Common.h"
 
 namespace facebook::velox::dwrf {
 
-class Config {
+class Config : public common::ConfigBase<Config> {
  public:
   template <typename T>
-  class Entry {
-    Entry(
-        const std::string& key,
-        const T& val,
-        std::function<std::string(const T&)> toStr =
-            [](const T& val) { return folly::to<std::string>(val); },
-        std::function<T(const std::string&)> toT =
-            [](const std::string& val) { return folly::to<T>(val); })
-        : key_{key}, default_{val}, toStr_{toStr}, toT_{toT} {}
-
-    const std::string key_;
-    const T default_;
-    const std::function<std::string(const T&)> toStr_;
-    const std::function<T(const std::string&)> toT_;
-
-    friend Config;
-  };
-
-  template <typename T>
-  Config& set(const Entry<T>& entry, const T& val) {
-    configs_[entry.key_] = entry.toStr_(val);
-    return *this;
-  }
-
-  template <typename T>
-  Config& unset(const Entry<T>& entry) {
-    auto iter = configs_.find(entry.key_);
-    if (iter != configs_.end()) {
-      configs_.erase(iter);
-    }
-    return *this;
-  }
-
-  Config& reset() {
-    configs_.clear();
-    return *this;
-  }
-
-  template <typename T>
-  T get(const Entry<T>& entry) const {
-    auto iter = configs_.find(entry.key_);
-    return iter != configs_.end() ? entry.toT_(iter->second) : entry.default_;
-  }
-
-  static std::shared_ptr<Config> fromMap(
-      std::map<std::string, std::string> map) {
-    auto ret = std::make_shared<Config>();
-    ret->configs_.insert(map.begin(), map.end());
-    return ret;
-  }
+  using Entry = common::ConfigBase<Config>::Entry<T>;
 
   static Entry<WriterVersion> WRITER_VERSION;
   static Entry<dwio::common::CompressionKind> COMPRESSION;
@@ -124,8 +75,12 @@ class Config {
   static Entry<uint64_t> RAW_DATA_SIZE_PER_BATCH;
   static Entry<bool> MAP_STATISTICS;
 
- private:
-  std::unordered_map<std::string, std::string> configs_;
+  static std::shared_ptr<Config> fromMap(
+      std::map<std::string, std::string> map) {
+    auto ret = std::make_shared<Config>();
+    ret->configs_.insert(map.begin(), map.end());
+    return ret;
+  }
 };
 
 } // namespace facebook::velox::dwrf

velox/dwio/dwrf/test/E2EFilterTest.cpp

@@ -135,11 +135,11 @@ class E2EFilterTest : public E2EFilterTestBase {
         }
         flatmapNodeIdsAsStruct_[child->id] = mapFlatColsStructKeys[i];
       }
-      config->set(Config::FLATTEN_MAP, true);
+      config->set(dwrf::Config::FLATTEN_MAP, true);
       config->set<const std::vector<uint32_t>>(
-          Config::MAP_FLAT_COLS, mapFlatCols);
+          dwrf::Config::MAP_FLAT_COLS, mapFlatCols);
       config->set<const std::vector<std::vector<std::string>>>(
-          Config::MAP_FLAT_COLS_STRUCT_KEYS, mapFlatColsStructKeys);
+          dwrf::Config::MAP_FLAT_COLS_STRUCT_KEYS, mapFlatColsStructKeys);
     }
     WriterOptions options;
     options.config = config;