[native] Serialize session property metadata with protocol

Author: pramodsatya

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

@@ -24,16 +24,6 @@ const std::string boolToString(bool value) {
 }
 } // namespace
 
-json SessionProperty::serialize() {
-  json j;
-  j["name"] = name_;
-  j["description"] = description_;
-  j["typeSignature"] = type_;
-  j["defaultValue"] = defaultValue_;
-  j["hidden"] = hidden_;
-  return j;
-}
-
 SessionProperties* SessionProperties::instance() {
   static std::unique_ptr<SessionProperties> instance =
       std::make_unique<SessionProperties>();
@@ -553,22 +543,18 @@ SessionProperties::SessionProperties() {
       std::to_string(c.unnestSplitOutput()));
 }
 
-const std::unordered_map<std::string, std::shared_ptr<SessionProperty>>&
-SessionProperties::testingSessionProperties() const {
-  return sessionProperties_;
-}
-
 const std::string SessionProperties::toVeloxConfig(
     const std::string& name) const {
   auto it = sessionProperties_.find(name);
-  return it == sessionProperties_.end() ? name
-                                        : it->second->getVeloxConfigName();
+  return it == sessionProperties_.end() ? name : it->second->getVeloxConfig();
 }
 
 json SessionProperties::serialize() const {
   json j = json::array();
-  for (const auto& entry : sessionProperties_) {
-    j.push_back(entry.second->serialize());
+  json tj;
+  for (const auto& sessionProperty : sessionProperties_) {
+    protocol::to_json(tj, sessionProperty.second->getMetadata());
+    j.push_back(tj);
   }
   return j;
 }

presto-native-execution/presto_cpp/main/SessionProperties.h

@@ -14,6 +14,7 @@
 #pragma once
 
 #include "presto_cpp/external/json/nlohmann/json.hpp"
+#include "presto_cpp/presto_protocol/core/presto_protocol_core.h"
 #include "velox/type/Type.h"
 
 using json = nlohmann::json;
@@ -27,44 +28,39 @@ class SessionProperty {
   SessionProperty(
       const std::string& name,
       const std::string& description,
-      const std::string& type,
+      const std::string& typeSignature,
       bool hidden,
-      const std::string& veloxConfigName,
+      const std::string& veloxConfig,
       const std::string& defaultValue)
-      : name_(name),
-        description_(description),
-        type_(type),
-        hidden_(hidden),
-        veloxConfigName_(veloxConfigName),
-        defaultValue_(defaultValue),
+      : metadata_({name, description, typeSignature, defaultValue, hidden}),
+        veloxConfig_(veloxConfig),
         value_(defaultValue) {}
 
-  const std::string getVeloxConfigName() {
-    return veloxConfigName_;
+  const protocol::SessionPropertyMetadata getMetadata() {
+    return metadata_;
+  }
+
+  const std::string getVeloxConfig() {
+    return veloxConfig_;
   }
 
   void updateValue(const std::string& value) {
     value_ = value;
   }
 
   bool operator==(const SessionProperty& other) const {
-    return name_ == other.name_ && description_ == other.description_ &&
-        type_ == other.type_ && hidden_ == other.hidden_ &&
-        veloxConfigName_ == other.veloxConfigName_ &&
-        defaultValue_ == other.defaultValue_;
+    const auto otherMetadata = other.metadata_;
+    return metadata_.name == otherMetadata.name &&
+        metadata_.description == otherMetadata.description &&
+        metadata_.typeSignature == otherMetadata.typeSignature &&
+        metadata_.hidden == otherMetadata.hidden &&
+        metadata_.defaultValue == otherMetadata.defaultValue &&
+        veloxConfig_ == other.veloxConfig_;
   }
 
-  json serialize();
-
  private:
-  const std::string name_;
-  const std::string description_;
-
-  // Datatype of presto native property.
-  const std::string type_;
-  const bool hidden_;
-  const std::string veloxConfigName_;
-  const std::string defaultValue_;
+  const protocol::SessionPropertyMetadata metadata_;
+  const std::string veloxConfig_;
   std::string value_;
 };
 
@@ -354,9 +350,6 @@ class SessionProperties {
 
   json serialize() const;
 
-  const std::unordered_map<std::string, std::shared_ptr<SessionProperty>>&
-  testingSessionProperties() const;
-
  private:
   void addSessionProperty(
       const std::string& name,

presto-native-execution/presto_cpp/main/tests/SessionPropertiesTest.cpp

@@ -127,26 +127,9 @@ TEST_F(SessionPropertiesTest, validateMapping) {
       {SessionProperties::kUnnestSplitOutput,
        core::QueryConfig::kUnnestSplitOutput}};
 
-  const auto& sessionProperties =
-      SessionProperties::instance()->testingSessionProperties();
-
-  ASSERT_EQ(expectedMappings.size(), sessionProperties.size());
-
-  for (const auto& [sessionPropertyName, expectedVeloxConfigName] :
-       expectedMappings) {
+  const auto sessionProperties = SessionProperties::instance();
+  for (const auto& [sessionProperty, expectedVeloxConfig] : expectedMappings) {
     ASSERT_EQ(
-        expectedVeloxConfigName,
-        sessionProperties.at(sessionPropertyName)->getVeloxConfigName());
-  }
-}
-
-TEST_F(SessionPropertiesTest, serializeProperty) {
-  auto* sessionProperties = SessionProperties::instance();
-  auto j = sessionProperties->serialize();
-  for (const auto& property : j) {
-    auto name = property["name"];
-    json expectedProperty =
-        sessionProperties->testingSessionProperties().at(name)->serialize();
-    EXPECT_EQ(property, expectedProperty);
+        expectedVeloxConfig, sessionProperties->toVeloxConfig(sessionProperty));
   }
 }