[native] Remove BaseVeloxQueryConfig (prestodb#25758)

Summary:

Remove BaseVeloxQueryConfig as it is not used.

Reviewed By: xiaoxmeng

Differential Revision: D80124169

Author: tanjialiang

presto-native-execution/presto_cpp/main/PrestoServer.cpp

@@ -196,7 +196,6 @@ PrestoServer::~PrestoServer() {}
 void PrestoServer::run() {
   auto systemConfig = SystemConfig::instance();
   auto nodeConfig = NodeConfig::instance();
-  auto baseVeloxQueryConfig = BaseVeloxQueryConfig::instance();
   int httpPort{0};
 
   std::string certPath;
@@ -212,9 +211,6 @@ void PrestoServer::run() {
         fmt::format("{}/config.properties", configDirectoryPath_));
     nodeConfig->initialize(
         fmt::format("{}/node.properties", configDirectoryPath_));
-    // velox.properties is optional.
-    baseVeloxQueryConfig->initialize(
-        fmt::format("{}/velox.properties", configDirectoryPath_), true);
 
     httpPort = systemConfig->httpServerHttpPort();
     if (systemConfig->httpServerHttpsEnabled()) {
@@ -319,9 +315,15 @@ void PrestoServer::run() {
       httpsSocketAddress.setFromLocalPort(httpsPort.value());
     }
 
-    const bool http2Enabled = SystemConfig::instance()->httpServerHttp2Enabled();
+    const bool http2Enabled =
+        SystemConfig::instance()->httpServerHttp2Enabled();
     httpsConfig = std::make_unique<http::HttpsConfig>(
-        httpsSocketAddress, certPath, keyPath, ciphers, reusePort, http2Enabled);
+        httpsSocketAddress,
+        certPath,
+        keyPath,
+        ciphers,
+        reusePort,
+        http2Enabled);
   }
 
   httpServer_ = std::make_unique<http::HttpServer>(

presto-native-execution/presto_cpp/main/PrestoServerOperations.cpp

@@ -177,7 +177,7 @@ std::string PrestoServerOperations::veloxQueryConfigOperation(
           "Have set system property value '{}' to '{}'. Old value was '{}'.\n",
           name,
           value,
-          BaseVeloxQueryConfig::instance()
+          SystemConfig::instance()
               ->setValue(name, value)
               .value_or("<default>"));
     }
@@ -190,7 +190,7 @@ std::string PrestoServerOperations::veloxQueryConfigOperation(
           ServerOperation::actionString(op.action));
       return fmt::format(
           "{}\n",
-          BaseVeloxQueryConfig::instance()->optionalProperty(name).value_or(
+          SystemConfig::instance()->optionalProperty(name).value_or(
               "<default>"));
     }
     default:

presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.cpp

@@ -168,9 +168,7 @@ void updateFromSystemConfigs(
 
 velox::core::QueryConfig toVeloxConfigs(
     const protocol::SessionRepresentation& session) {
-  // Use base velox query config as the starting point and add Presto session
-  // properties on top of it.
-  auto configs = BaseVeloxQueryConfig::instance()->values();
+  std::unordered_map<std::string, std::string> configs;
 
   // Firstly apply Presto system properties to Velox query config.
   updateFromSystemConfigs(configs);

presto-native-execution/presto_cpp/main/common/Configs.cpp

@@ -260,7 +260,7 @@ SystemConfig::SystemConfig() {
           NUM_PROP(kExchangeIoEvbViolationThresholdMs, 1000),
           NUM_PROP(kHttpSrvIoEvbViolationThresholdMs, 1000),
           NUM_PROP(kMaxLocalExchangePartitionBufferSize, 65536),
-        BOOL_PROP(kTextWriterEnabled, false),
+          BOOL_PROP(kTextWriterEnabled, false),
       };
 }
 
@@ -910,17 +910,16 @@ std::string SystemConfig::pluginDir() const {
 }
 
 int32_t SystemConfig::exchangeIoEvbViolationThresholdMs() const {
-  return optionalProperty<int32_t>(kExchangeIoEvbViolationThresholdMs)
-      .value();
+  return optionalProperty<int32_t>(kExchangeIoEvbViolationThresholdMs).value();
 }
 
 int32_t SystemConfig::httpSrvIoEvbViolationThresholdMs() const {
-  return optionalProperty<int32_t>(kHttpSrvIoEvbViolationThresholdMs)
-      .value();
+  return optionalProperty<int32_t>(kHttpSrvIoEvbViolationThresholdMs).value();
 }
 
 uint64_t SystemConfig::maxLocalExchangePartitionBufferSize() const {
-  return optionalProperty<uint64_t>(kMaxLocalExchangePartitionBufferSize).value();
+  return optionalProperty<uint64_t>(kMaxLocalExchangePartitionBufferSize)
+      .value();
 }
 
 bool SystemConfig::textWriterEnabled() const {
@@ -949,8 +948,7 @@ std::string NodeConfig::nodeEnvironment() const {
 }
 
 int NodeConfig::prometheusExecutorThreads() const {
-  static constexpr int
-      kNodePrometheusExecutorThreadsDefault = 2;
+  static constexpr int kNodePrometheusExecutorThreadsDefault = 2;
   auto resultOpt = optionalProperty<int>(kNodePrometheusExecutorThreads);
   if (resultOpt.has_value()) {
     return resultOpt.value();
@@ -992,115 +990,4 @@ std::string NodeConfig::nodeInternalAddress(
   }
 }
 
-BaseVeloxQueryConfig::BaseVeloxQueryConfig() {
-  // Use empty instance to get default property values.
-  velox::core::QueryConfig c{{}};
-  using namespace velox::core;
-  registeredProps_ =
-      std::unordered_map<std::string, folly::Optional<std::string>>{
-          BOOL_PROP(kMutableConfig, false),
-          STR_PROP(QueryConfig::kSessionTimezone, c.sessionTimezone()),
-          BOOL_PROP(
-              QueryConfig::kAdjustTimestampToTimezone,
-              c.adjustTimestampToTimezone()),
-          BOOL_PROP(QueryConfig::kExprEvalSimplified, c.exprEvalSimplified()),
-          BOOL_PROP(QueryConfig::kExprTrackCpuUsage, c.exprTrackCpuUsage()),
-          BOOL_PROP(
-              QueryConfig::kOperatorTrackCpuUsage, c.operatorTrackCpuUsage()),
-          BOOL_PROP(
-              QueryConfig::kCastMatchStructByName, c.isMatchStructByName()),
-          NUM_PROP(
-              QueryConfig::kMaxLocalExchangeBufferSize,
-              c.maxLocalExchangeBufferSize()),
-          NUM_PROP(
-              QueryConfig::kMaxPartialAggregationMemory,
-              c.maxPartialAggregationMemoryUsage()),
-          NUM_PROP(
-              QueryConfig::kMaxExtendedPartialAggregationMemory,
-              c.maxExtendedPartialAggregationMemoryUsage()),
-          NUM_PROP(
-              QueryConfig::kAbandonPartialAggregationMinRows,
-              c.abandonPartialAggregationMinRows()),
-          NUM_PROP(
-              QueryConfig::kAbandonPartialAggregationMinPct,
-              c.abandonPartialAggregationMinPct()),
-          NUM_PROP(
-              QueryConfig::kMaxPartitionedOutputBufferSize,
-              c.maxPartitionedOutputBufferSize()),
-          NUM_PROP(
-              QueryConfig::kPreferredOutputBatchBytes,
-              c.preferredOutputBatchBytes()),
-          NUM_PROP(
-              QueryConfig::kPreferredOutputBatchRows,
-              c.preferredOutputBatchRows()),
-          NUM_PROP(QueryConfig::kMaxOutputBatchRows, c.maxOutputBatchRows()),
-          BOOL_PROP(
-              QueryConfig::kHashAdaptivityEnabled, c.hashAdaptivityEnabled()),
-          BOOL_PROP(
-              QueryConfig::kAdaptiveFilterReorderingEnabled,
-              c.adaptiveFilterReorderingEnabled()),
-          BOOL_PROP(QueryConfig::kSpillEnabled, c.spillEnabled()),
-          BOOL_PROP(
-              QueryConfig::kAggregationSpillEnabled,
-              c.aggregationSpillEnabled()),
-          BOOL_PROP(QueryConfig::kJoinSpillEnabled, c.joinSpillEnabled()),
-          BOOL_PROP(QueryConfig::kOrderBySpillEnabled, c.orderBySpillEnabled()),
-          NUM_PROP(QueryConfig::kMaxSpillBytes, c.maxSpillBytes()),
-          NUM_PROP(QueryConfig::kMaxSpillLevel, c.maxSpillLevel()),
-          NUM_PROP(QueryConfig::kMaxSpillFileSize, c.maxSpillFileSize()),
-          NUM_PROP(
-              QueryConfig::kSpillStartPartitionBit, c.spillStartPartitionBit()),
-          NUM_PROP(
-              QueryConfig::kSpillNumPartitionBits, c.spillNumPartitionBits()),
-          NUM_PROP(
-              QueryConfig::kSpillableReservationGrowthPct,
-              c.spillableReservationGrowthPct()),
-          BOOL_PROP(
-              QueryConfig::kPrestoArrayAggIgnoreNulls,
-              c.prestoArrayAggIgnoreNulls()),
-          BOOL_PROP(
-              QueryConfig::kSelectiveNimbleReaderEnabled,
-              c.selectiveNimbleReaderEnabled()),
-          NUM_PROP(QueryConfig::kMaxOutputBufferSize, c.maxOutputBufferSize()),
-      };
-}
-
-BaseVeloxQueryConfig* BaseVeloxQueryConfig::instance() {
-  static std::unique_ptr<BaseVeloxQueryConfig> instance =
-      std::make_unique<BaseVeloxQueryConfig>();
-  return instance.get();
-}
-
-void BaseVeloxQueryConfig::updateLoadedValues(
-    std::unordered_map<std::string, std::string>& values) const {
-  // Update velox config with values from presto system config.
-  auto systemConfig = SystemConfig::instance();
-
-  using namespace velox::core;
-  std::unordered_map<std::string, std::string> updatedValues{};
-
-  auto taskWriterCount = systemConfig->taskWriterCount();
-  if (taskWriterCount.has_value()) {
-    updatedValues[QueryConfig::kTaskWriterCount] =
-        std::to_string(taskWriterCount.value());
-  }
-  auto taskPartitionedWriterCount = systemConfig->taskPartitionedWriterCount();
-  if (taskPartitionedWriterCount.has_value()) {
-    updatedValues[QueryConfig::kTaskPartitionedWriterCount] =
-        std::to_string(taskPartitionedWriterCount.value());
-  }
-
-  std::stringstream updated;
-  for (const auto& pair : updatedValues) {
-    updated << "  " << pair.first << "=" << pair.second << "\n";
-    values[pair.first] = pair.second;
-  }
-  auto str = updated.str();
-  if (!str.empty()) {
-    PRESTO_STARTUP_LOG(INFO)
-        << "Updated in '" << filePath_ << "' from SystemProperties:\n"
-        << str;
-  }
-}
-
 } // namespace facebook::presto

presto-native-execution/presto_cpp/main/common/Configs.h

@@ -43,12 +43,12 @@ class ConfigBase {
   }
 
   /// DO NOT DELETE THIS METHOD!
-  /// The method is used to register new properties after the config class is created.
-  /// Returns true if succeeded, false if failed (due to the property already
-  /// registered).
+  /// The method is used to register new properties after the config class is
+  /// created. Returns true if succeeded, false if failed (due to the property
+  /// already registered).
   bool registerProperty(
-    const std::string& propertyName,
-    const folly::Optional<std::string>& defaultValue = {});
+      const std::string& propertyName,
+      const folly::Optional<std::string>& defaultValue = {});
 
   /// Adds or replaces value at the given key. Can be used by debugging or
   /// testing code.
@@ -721,10 +721,6 @@ class SystemConfig : public ConfigBase {
   /// Optional string containing the path to the plugin directory
   static constexpr std::string_view kPluginDir{"plugin.dir"};
 
-  /// Below are the Presto properties from config.properties that get converted
-  /// to their velox counterparts in BaseVeloxQueryConfig and used solely from
-  /// BaseVeloxQueryConfig.
-
   /// Uses legacy version of array_agg which ignores nulls.
   static constexpr std::string_view kUseLegacyArrayAgg{
       "deprecated.legacy-array-agg"};
@@ -752,7 +748,7 @@ class SystemConfig : public ConfigBase {
 
   // Max wait time for exchange request in seconds.
   static constexpr std::string_view kRequestDataSizesMaxWaitSec{
-    "exchange.http-client.request-data-sizes-max-wait-sec"};
+      "exchange.http-client.request-data-sizes-max-wait-sec"};
 
   static constexpr std::string_view kExchangeIoEvbViolationThresholdMs{
       "exchange.io-evb-violation-threshold-ms"};
@@ -764,8 +760,7 @@ class SystemConfig : public ConfigBase {
 
   // Add to temporarily help with gradual rollout for text writer
   // TODO: remove once text writer is fully rolled out
-  static constexpr std::string_view kTextWriterEnabled{
-    "text-writer-enabled"};
+  static constexpr std::string_view kTextWriterEnabled{"text-writer-enabled"};
 
   SystemConfig();
 
@@ -1069,7 +1064,8 @@ class NodeConfig : public ConfigBase {
   static constexpr std::string_view kNodeInternalAddress{
       "node.internal-address"};
   static constexpr std::string_view kNodeLocation{"node.location"};
-  static constexpr std::string_view kNodePrometheusExecutorThreads{"node.prometheus.num-executor-threads"};
+  static constexpr std::string_view kNodePrometheusExecutorThreads{
+      "node.prometheus.num-executor-threads"};
 
   NodeConfig();
 
@@ -1089,19 +1085,4 @@ class NodeConfig : public ConfigBase {
   std::string nodeLocation() const;
 };
 
-/// Used only in the single instance as the source of the initial properties for
-/// velox::QueryConfig. Not designed for actual property access during a query
-/// run.
-class BaseVeloxQueryConfig : public ConfigBase {
- public:
-  BaseVeloxQueryConfig();
-
-  virtual ~BaseVeloxQueryConfig() = default;
-
-  void updateLoadedValues(
-      std::unordered_map<std::string, std::string>& values) const override;
-
-  static BaseVeloxQueryConfig* instance();
-};
-
 } // namespace facebook::presto