Skip to content

Commit e366be1

Browse files
authored
[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. Differential Revision: D82000691 ``` == NO RELEASE NOTE == ```
1 parent 07a1a88 commit e366be1

File tree

5 files changed

+82
-2
lines changed

5 files changed

+82
-2
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ SystemConfig::SystemConfig() {
262262
NUM_PROP(kMaxLocalExchangePartitionBufferSize, 65536),
263263
BOOL_PROP(kTextWriterEnabled, true),
264264
BOOL_PROP(kCharNToVarcharImplicitCast, false),
265+
BOOL_PROP(kEnumTypesEnabled, true),
265266
};
266267
}
267268

@@ -931,6 +932,10 @@ bool SystemConfig::charNToVarcharImplicitCast() const {
931932
return optionalProperty<bool>(kCharNToVarcharImplicitCast).value();
932933
}
933934

935+
bool SystemConfig::enumTypesEnabled() const {
936+
return optionalProperty<bool>(kEnumTypesEnabled).value();
937+
}
938+
934939
NodeConfig::NodeConfig() {
935940
registeredProps_ =
936941
std::unordered_map<std::string, folly::Optional<std::string>>{

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,12 @@ class SystemConfig : public ConfigBase {
768768
static constexpr std::string_view kCharNToVarcharImplicitCast{
769769
"char-n-to-varchar-implicit-cast"};
770770

771+
/// Enable BigintEnum and VarcharEnum types to be parsed and used in Velox.
772+
/// When set to false, BigintEnum or VarcharEnum types will throw an
773+
// unsupported error during type parsing.
774+
static constexpr std::string_view kEnumTypesEnabled{
775+
"enum-types-enabled"};
776+
771777
SystemConfig();
772778

773779
virtual ~SystemConfig() = default;
@@ -1060,6 +1066,8 @@ class SystemConfig : public ConfigBase {
10601066
bool textWriterEnabled() const;
10611067

10621068
bool charNToVarcharImplicitCast() const;
1069+
1070+
bool enumTypesEnabled() const;
10631071
};
10641072

10651073
/// Provides access to node properties defined in node.properties file.

presto-native-execution/presto_cpp/main/types/TypeParser.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@ velox::TypePtr TypeParser::parse(const std::string& text) const {
2727
return velox::VARCHAR();
2828
}
2929
}
30-
30+
if (!SystemConfig::instance()->enumTypesEnabled()) {
31+
if (text.find("BigintEnum") != std::string::npos || text.find("VarcharEnum") != std::string::npos) {
32+
VELOX_UNSUPPORTED("Unsupported type: {}", text);
33+
}
34+
}
3135
auto it = cache_.find(text);
3236
if (it != cache_.end()) {
3337
return it->second;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ target_link_libraries(
3535
velox_hive_partition_function)
3636

3737
add_executable(presto_expressions_test RowExpressionTest.cpp ValuesPipeTest.cpp
38-
PlanConverterTest.cpp)
38+
PlanConverterTest.cpp TypeParserTest.cpp)
3939

4040
add_test(
4141
NAME presto_expressions_test
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
15+
#include <gtest/gtest.h>
16+
17+
#include "presto_cpp/main/common/Configs.h"
18+
#include "presto_cpp/main/common/tests/MutableConfigs.h"
19+
#include "presto_cpp/main/types/TypeParser.h"
20+
#include "velox/common/file/FileSystems.h"
21+
#include "velox/common/base/tests/GTestUtils.h"
22+
#include "velox/functions/prestosql/types/BigintEnumRegistration.h"
23+
#include "velox/functions/prestosql/types/VarcharEnumRegistration.h"
24+
25+
26+
using namespace facebook::presto;
27+
using namespace facebook::velox;
28+
29+
class TypeParserTest : public ::testing::Test {
30+
void SetUp() override {
31+
filesystems::registerLocalFileSystem();
32+
test::setupMutableSystemConfig();
33+
registerBigintEnumType();
34+
registerVarcharEnumType();
35+
}
36+
};
37+
38+
// Test basical functionality of TypeParser.
39+
// More detailed tests for Presto TypeParser are in velox/functions/prestosql/types/parser/tests/TypeParserTest.
40+
TEST_F(TypeParserTest, parseEnumTypes) {
41+
TypeParser typeParser = TypeParser();
42+
43+
ASSERT_EQ(
44+
typeParser.parse(
45+
"test.enum.mood:BigintEnum(test.enum.mood{\"CURIOUS\":2, \"HAPPY\":0})")->toString(),
46+
"test.enum.mood:BigintEnum({\"CURIOUS\": 2, \"HAPPY\": 0})");
47+
ASSERT_EQ(
48+
typeParser.parse(
49+
"test.enum.mood:VarcharEnum(test.enum.mood{\"CURIOUS\":\"ONXW2ZKWMFWHKZI=\", \"HAPPY\":\"ONXW2ZJAOZQWY5LF\" , \"SAD\":\"KNHU2RJAKZAUYVKF\"})")->toString(),
50+
"test.enum.mood:VarcharEnum({\"CURIOUS\": \"someValue\", \"HAPPY\": \"some value\", \"SAD\": \"SOME VALUE\"})");
51+
52+
// When set to false, TypeParser will throw an unsupported error when it receives an enum type.
53+
SystemConfig::instance()->setValue(std::string(SystemConfig::kEnumTypesEnabled), "false");
54+
55+
VELOX_ASSERT_THROW(
56+
typeParser.parse(
57+
"test.enum.mood:BigintEnum(test.enum.mood{\"CURIOUS\":2, \"HAPPY\":0})"),
58+
"Unsupported type: test.enum.mood:BigintEnum(test.enum.mood{\"CURIOUS\":2, \"HAPPY\":0})");
59+
VELOX_ASSERT_THROW(
60+
typeParser.parse(
61+
"test.enum.mood:VarcharEnum(test.enum.mood{\"CURIOUS\":\"ONXW2ZKWMFWHKZI=\", \"HAPPY\":\"ONXW2ZJAOZQWY5LF\" , \"SAD\":\"KNHU2RJAKZAUYVKF\"})"),
62+
"Unsupported type: test.enum.mood:VarcharEnum(test.enum.mood{\"CURIOUS\":\"ONXW2ZKWMFWHKZI=\", \"HAPPY\":\"ONXW2ZJAOZQWY5LF\" , \"SAD\":\"KNHU2RJAKZAUYVKF\"})");
63+
}

0 commit comments

Comments
 (0)