From fac5a4b7dd80ea50444d110eec4bac84f9fbb689 Mon Sep 17 00:00:00 2001 From: Heidi Han Date: Tue, 9 Sep 2025 15:36:28 -0700 Subject: [PATCH] [presto] Add enum type flag to Prestissimo worker config (#25989) Summary: Added "enum-types-enabled" boolean flag to control the parsing/usage of Bigitnenum and VarcharEnum functions in Prestissimo workers. This is to help with Capella types roll out, in case we encounter issues with correctness, etc, and if we cannot block them from gateway. Reviewed By: kewang1024 Differential Revision: D82000691 --- .../presto_cpp/main/common/Configs.cpp | 5 ++ .../presto_cpp/main/common/Configs.h | 8 +++ .../presto_cpp/main/types/TypeParser.cpp | 6 +- .../main/types/tests/CMakeLists.txt | 2 +- .../main/types/tests/TypeParserTest.cpp | 63 +++++++++++++++++++ 5 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 presto-native-execution/presto_cpp/main/types/tests/TypeParserTest.cpp diff --git a/presto-native-execution/presto_cpp/main/common/Configs.cpp b/presto-native-execution/presto_cpp/main/common/Configs.cpp index 85e5f356ae815..76613ee440df4 100644 --- a/presto-native-execution/presto_cpp/main/common/Configs.cpp +++ b/presto-native-execution/presto_cpp/main/common/Configs.cpp @@ -262,6 +262,7 @@ SystemConfig::SystemConfig() { NUM_PROP(kMaxLocalExchangePartitionBufferSize, 65536), BOOL_PROP(kTextWriterEnabled, true), BOOL_PROP(kCharNToVarcharImplicitCast, false), + BOOL_PROP(kEnumTypesEnabled, true), }; } @@ -931,6 +932,10 @@ bool SystemConfig::charNToVarcharImplicitCast() const { return optionalProperty(kCharNToVarcharImplicitCast).value(); } +bool SystemConfig::enumTypesEnabled() const { + return optionalProperty(kEnumTypesEnabled).value(); +} + NodeConfig::NodeConfig() { registeredProps_ = std::unordered_map>{ diff --git a/presto-native-execution/presto_cpp/main/common/Configs.h b/presto-native-execution/presto_cpp/main/common/Configs.h index fc49549ca1695..c891aee3d2306 100644 --- a/presto-native-execution/presto_cpp/main/common/Configs.h +++ b/presto-native-execution/presto_cpp/main/common/Configs.h @@ -768,6 +768,12 @@ class SystemConfig : public ConfigBase { static constexpr std::string_view kCharNToVarcharImplicitCast{ "char-n-to-varchar-implicit-cast"}; + /// Enable BigintEnum and VarcharEnum types to be parsed and used in Velox. + /// When set to false, BigintEnum or VarcharEnum types will throw an + // unsupported error during type parsing. + static constexpr std::string_view kEnumTypesEnabled{ + "enum-types-enabled"}; + SystemConfig(); virtual ~SystemConfig() = default; @@ -1060,6 +1066,8 @@ class SystemConfig : public ConfigBase { bool textWriterEnabled() const; bool charNToVarcharImplicitCast() const; + + bool enumTypesEnabled() const; }; /// Provides access to node properties defined in node.properties file. diff --git a/presto-native-execution/presto_cpp/main/types/TypeParser.cpp b/presto-native-execution/presto_cpp/main/types/TypeParser.cpp index 4a19066c38fb0..15d7301c347d4 100644 --- a/presto-native-execution/presto_cpp/main/types/TypeParser.cpp +++ b/presto-native-execution/presto_cpp/main/types/TypeParser.cpp @@ -27,7 +27,11 @@ velox::TypePtr TypeParser::parse(const std::string& text) const { return velox::VARCHAR(); } } - + if (!SystemConfig::instance()->enumTypesEnabled()) { + if (text.find("BigintEnum") != std::string::npos || text.find("VarcharEnum") != std::string::npos) { + VELOX_UNSUPPORTED("Unsupported type: {}", text); + } + } auto it = cache_.find(text); if (it != cache_.end()) { return it->second; diff --git a/presto-native-execution/presto_cpp/main/types/tests/CMakeLists.txt b/presto-native-execution/presto_cpp/main/types/tests/CMakeLists.txt index 48b22e9a63328..8f98f3f76686f 100644 --- a/presto-native-execution/presto_cpp/main/types/tests/CMakeLists.txt +++ b/presto-native-execution/presto_cpp/main/types/tests/CMakeLists.txt @@ -35,7 +35,7 @@ target_link_libraries( velox_hive_partition_function) add_executable(presto_expressions_test RowExpressionTest.cpp ValuesPipeTest.cpp - PlanConverterTest.cpp) + PlanConverterTest.cpp TypeParserTest.cpp) add_test( NAME presto_expressions_test diff --git a/presto-native-execution/presto_cpp/main/types/tests/TypeParserTest.cpp b/presto-native-execution/presto_cpp/main/types/tests/TypeParserTest.cpp new file mode 100644 index 0000000000000..ee9036e5943bc --- /dev/null +++ b/presto-native-execution/presto_cpp/main/types/tests/TypeParserTest.cpp @@ -0,0 +1,63 @@ +/* + * 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. + */ + +#include + +#include "presto_cpp/main/common/Configs.h" +#include "presto_cpp/main/common/tests/MutableConfigs.h" +#include "presto_cpp/main/types/TypeParser.h" +#include "velox/common/file/FileSystems.h" +#include "velox/common/base/tests/GTestUtils.h" +#include "velox/functions/prestosql/types/BigintEnumRegistration.h" +#include "velox/functions/prestosql/types/VarcharEnumRegistration.h" + + +using namespace facebook::presto; +using namespace facebook::velox; + +class TypeParserTest : public ::testing::Test { + void SetUp() override { + filesystems::registerLocalFileSystem(); + test::setupMutableSystemConfig(); + registerBigintEnumType(); + registerVarcharEnumType(); + } +}; + +// Test basical functionality of TypeParser. +// More detailed tests for Presto TypeParser are in velox/functions/prestosql/types/parser/tests/TypeParserTest. +TEST_F(TypeParserTest, parseEnumTypes) { + TypeParser typeParser = TypeParser(); + + ASSERT_EQ( + typeParser.parse( + "test.enum.mood:BigintEnum(test.enum.mood{\"CURIOUS\":2, \"HAPPY\":0})")->toString(), + "test.enum.mood:BigintEnum({\"CURIOUS\": 2, \"HAPPY\": 0})"); + ASSERT_EQ( + typeParser.parse( + "test.enum.mood:VarcharEnum(test.enum.mood{\"CURIOUS\":\"ONXW2ZKWMFWHKZI=\", \"HAPPY\":\"ONXW2ZJAOZQWY5LF\" , \"SAD\":\"KNHU2RJAKZAUYVKF\"})")->toString(), + "test.enum.mood:VarcharEnum({\"CURIOUS\": \"someValue\", \"HAPPY\": \"some value\", \"SAD\": \"SOME VALUE\"})"); + + // When set to false, TypeParser will throw an unsupported error when it receives an enum type. + SystemConfig::instance()->setValue(std::string(SystemConfig::kEnumTypesEnabled), "false"); + + VELOX_ASSERT_THROW( + typeParser.parse( + "test.enum.mood:BigintEnum(test.enum.mood{\"CURIOUS\":2, \"HAPPY\":0})"), + "Unsupported type: test.enum.mood:BigintEnum(test.enum.mood{\"CURIOUS\":2, \"HAPPY\":0})"); + VELOX_ASSERT_THROW( + typeParser.parse( + "test.enum.mood:VarcharEnum(test.enum.mood{\"CURIOUS\":\"ONXW2ZKWMFWHKZI=\", \"HAPPY\":\"ONXW2ZJAOZQWY5LF\" , \"SAD\":\"KNHU2RJAKZAUYVKF\"})"), + "Unsupported type: test.enum.mood:VarcharEnum(test.enum.mood{\"CURIOUS\":\"ONXW2ZKWMFWHKZI=\", \"HAPPY\":\"ONXW2ZJAOZQWY5LF\" , \"SAD\":\"KNHU2RJAKZAUYVKF\"})"); +}