refactor(native): Abstract session property provider

presto-native-execution/presto_cpp/main/CMakeLists.txt

@@ -17,10 +17,7 @@ add_subdirectory(thrift)
 add_subdirectory(connectors)
 add_subdirectory(functions)
 add_subdirectory(tool)
-
-add_library(presto_session_properties SessionProperties.cpp)
-
-target_link_libraries(presto_session_properties ${FOLLY_WITH_DEPENDENCIES})
+add_subdirectory(properties)
 
 add_library(
   presto_server_lib

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

@@ -22,7 +22,6 @@
 #include "presto_cpp/main/CoordinatorDiscoverer.h"
 #include "presto_cpp/main/PeriodicMemoryChecker.h"
 #include "presto_cpp/main/PeriodicTaskManager.h"
-#include "presto_cpp/main/SessionProperties.h"
 #include "presto_cpp/main/SignalHandler.h"
 #include "presto_cpp/main/TaskResource.h"
 #include "presto_cpp/main/common/ConfigReader.h"
@@ -44,6 +43,7 @@
 #include "presto_cpp/main/operators/ShuffleExchangeSource.h"
 #include "presto_cpp/main/operators/ShuffleRead.h"
 #include "presto_cpp/main/operators/ShuffleWrite.h"
+#include "presto_cpp/main/properties/session/SessionProperties.h"
 #include "presto_cpp/main/types/ExpressionOptimizer.h"
 #include "presto_cpp/main/types/PrestoToVeloxQueryPlan.h"
 #include "presto_cpp/main/types/VeloxPlanConversion.h"

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

@@ -13,8 +13,8 @@
  */
 
 #include "presto_cpp/main/PrestoToVeloxQueryConfig.h"
-#include "presto_cpp/main/SessionProperties.h"
 #include "presto_cpp/main/common/Configs.h"
+#include "presto_cpp/main/properties/session/SessionProperties.h"
 #include "velox/common/compression/Compression.h"
 #include "velox/core/QueryConfig.h"
 #include "velox/type/tz/TimeZoneMap.h"

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

@@ -15,8 +15,8 @@
 #include "presto_cpp/main/QueryContextManager.h"
 #include <folly/executors/IOThreadPoolExecutor.h>
 #include "presto_cpp/main/PrestoToVeloxQueryConfig.h"
-#include "presto_cpp/main/SessionProperties.h"
 #include "presto_cpp/main/common/Configs.h"
+#include "presto_cpp/main/properties/session/SessionProperties.h"
 #include "velox/connectors/hive/HiveConfig.h"
 #include "velox/core/QueryConfig.h"
 

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

@@ -23,6 +23,10 @@
 
 namespace facebook::presto::util {
 
+std::string boolToLowerCaseString(bool value) {
+  return value ? "true" : "false";
+}
+
 DateTime toISOTimestamp(uint64_t timeMilli) {
   char buf[80];
   time_t timeSecond = timeMilli / 1000;

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

@@ -25,6 +25,9 @@ namespace facebook::presto::util {
 #define PRESTO_SHUTDOWN_LOG(severity) \
   LOG(severity) << PRESTO_SHUTDOWN_LOG_PREFIX
 
+/// Convert boolean to lowercase string representation.
+std::string boolToLowerCaseString(bool value);
+
 using DateTime = std::string;
 DateTime toISOTimestamp(uint64_t timeMilli);
 

presto-native-execution/presto_cpp/main/properties/CMakeLists.txt

@@ -0,0 +1,13 @@
+# 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.
+
+add_subdirectory(session)

presto-native-execution/presto_cpp/main/properties/session/CMakeLists.txt

@@ -0,0 +1,28 @@
+# 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.
+
+add_library(presto_session_properties_provider SessionPropertiesProvider.cpp)
+
+target_link_libraries(
+  presto_session_properties_provider
+  ${FOLLY_WITH_DEPENDENCIES}
+  presto_protocol
+  presto_common
+)
+
+add_library(presto_session_properties SessionProperties.cpp)
+
+target_link_libraries(presto_session_properties presto_session_properties_provider)
+
+if(PRESTO_ENABLE_TESTING)
+  add_subdirectory(tests)
+endif()

presto-native-execution/presto_cpp/main/SessionProperties.cpp → presto-native-execution/presto_cpp/main/properties/session/SessionProperties.cpp

@@ -11,36 +11,20 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "presto_cpp/main/SessionProperties.h"
+#include "presto_cpp/main/properties/session/SessionProperties.h"
+#include "presto_cpp/main/common/Utils.h"
 #include "velox/core/QueryConfig.h"
 
 using namespace facebook::velox;
 
 namespace facebook::presto {
 
-namespace {
-const std::string boolToString(bool value) {
-  return value ? "true" : "false";
-}
-} // namespace
-
 SessionProperties* SessionProperties::instance() {
   static std::unique_ptr<SessionProperties> instance =
       std::make_unique<SessionProperties>();
   return instance.get();
 }
 
-void SessionProperties::addSessionProperty(
-    const std::string& name,
-    const std::string& description,
-    const TypePtr& type,
-    bool isHidden,
-    const std::optional<std::string> veloxConfig,
-    const std::string& defaultValue) {
-  sessionProperties_[name] = std::make_shared<SessionProperty>(
-      name, description, type->toString(), isHidden, veloxConfig, defaultValue);
-}
-
 // List of native session properties is kept as the source of truth here.
 SessionProperties::SessionProperties() {
   using velox::core::QueryConfig;
@@ -53,7 +37,7 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kExprEvalSimplified,
-      boolToString(c.exprEvalSimplified()));
+      util::boolToLowerCaseString(c.exprEvalSimplified()));
 
   addSessionProperty(
       kExprMaxArraySizeInReduce,
@@ -169,23 +153,23 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kJoinSpillEnabled,
-      boolToString(c.joinSpillEnabled()));
+      util::boolToLowerCaseString(c.joinSpillEnabled()));
 
   addSessionProperty(
       kWindowSpillEnabled,
       "Native Execution only. Enable window spilling on native engine",
       BOOLEAN(),
       false,
       QueryConfig::kWindowSpillEnabled,
-      boolToString(c.windowSpillEnabled()));
+      util::boolToLowerCaseString(c.windowSpillEnabled()));
 
   addSessionProperty(
       kWriterSpillEnabled,
       "Native Execution only. Enable writer spilling on native engine",
       BOOLEAN(),
       false,
       QueryConfig::kWriterSpillEnabled,
-      boolToString(c.writerSpillEnabled()));
+      util::boolToLowerCaseString(c.writerSpillEnabled()));
 
   addSessionProperty(
       kWriterFlushThresholdBytes,
@@ -203,15 +187,15 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kRowNumberSpillEnabled,
-      boolToString(c.rowNumberSpillEnabled()));
+      util::boolToLowerCaseString(c.rowNumberSpillEnabled()));
 
   addSessionProperty(
       kMarkDistinctSpillEnabled,
       "Native Execution only. Enable mark distinct spilling on native engine",
       BOOLEAN(),
       false,
       QueryConfig::kMarkDistinctSpillEnabled,
-      boolToString(c.markDistinctSpillEnabled()));
+      util::boolToLowerCaseString(c.markDistinctSpillEnabled()));
 
   addSessionProperty(
       kSpillerNumPartitionBits,
@@ -228,7 +212,7 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kTopNRowNumberSpillEnabled,
-      boolToString(c.topNRowNumberSpillEnabled()));
+      util::boolToLowerCaseString(c.topNRowNumberSpillEnabled()));
 
   addSessionProperty(
       kValidateOutputFromOperators,
@@ -240,7 +224,7 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kValidateOutputFromOperators,
-      boolToString(c.validateOutputFromOperators()));
+      util::boolToLowerCaseString(c.validateOutputFromOperators()));
 
   addSessionProperty(
       kDebugDisableExpressionWithPeeling,
@@ -249,7 +233,7 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kDebugDisableExpressionWithPeeling,
-      boolToString(c.debugDisableExpressionsWithPeeling()));
+      util::boolToLowerCaseString(c.debugDisableExpressionsWithPeeling()));
 
   addSessionProperty(
       kDebugDisableCommonSubExpressions,
@@ -259,7 +243,7 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kDebugDisableCommonSubExpressions,
-      boolToString(c.debugDisableCommonSubExpressions()));
+      util::boolToLowerCaseString(c.debugDisableCommonSubExpressions()));
 
   addSessionProperty(
       kDebugDisableExpressionWithMemoization,
@@ -270,7 +254,7 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kDebugDisableExpressionWithMemoization,
-      boolToString(c.debugDisableExpressionsWithMemoization()));
+      util::boolToLowerCaseString(c.debugDisableExpressionsWithMemoization()));
 
   addSessionProperty(
       kDebugDisableExpressionWithLazyInputs,
@@ -280,7 +264,7 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kDebugDisableExpressionWithLazyInputs,
-      boolToString(c.debugDisableExpressionsWithLazyInputs()));
+      util::boolToLowerCaseString(c.debugDisableExpressionsWithLazyInputs()));
 
   addSessionProperty(
       kDebugMemoryPoolNameRegex,
@@ -314,15 +298,15 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kSelectiveNimbleReaderEnabled,
-      boolToString(c.selectiveNimbleReaderEnabled()));
+      util::boolToLowerCaseString(c.selectiveNimbleReaderEnabled()));
 
   addSessionProperty(
       kQueryTraceEnabled,
       "Enables query tracing.",
       BOOLEAN(),
       false,
       QueryConfig::kQueryTraceEnabled,
-      boolToString(c.queryTraceEnabled()));
+      util::boolToLowerCaseString(c.queryTraceEnabled()));
 
   addSessionProperty(
       kQueryTraceDir,
@@ -668,27 +652,8 @@ SessionProperties::SessionProperties() {
       BOOLEAN(),
       false,
       QueryConfig::kAggregationMemoryCompactionReclaimEnabled,
-      boolToString(c.aggregationMemoryCompactionReclaimEnabled()));
-}
-
-const std::string SessionProperties::toVeloxConfig(
-    const std::string& name) const {
-  auto it = sessionProperties_.find(name);
-  if (it != sessionProperties_.end() &&
-      it->second->getVeloxConfig().has_value()) {
-    return it->second->getVeloxConfig().value();
-  }
-  return name;
-}
-
-json SessionProperties::serialize() const {
-  json j = json::array();
-  json tj;
-  for (const auto& sessionProperty : sessionProperties_) {
-    protocol::to_json(tj, sessionProperty.second->getMetadata());
-    j.push_back(tj);
-  }
-  return j;
+      util::boolToLowerCaseString(
+          c.aggregationMemoryCompactionReclaimEnabled()));
 }
 
 bool SessionProperties::useVeloxGeospatialJoin() const {