Skip to content

Commit 137ee83

Browse files
amitkduttafacebook-github-bot
authored andcommitted
misc: Throw unsupported type error for unregistered type. (facebookincubator#12629)
Summary: Throwing user error when unregistered type is found and fail on unsupported type is true. This helps to fail a presto query with user errors instead of generic errors, thus helping better classificaiton of user and system errors. Differential Revision: D71096915
1 parent 77f6eff commit 137ee83

File tree

2 files changed

+7
-8
lines changed

2 files changed

+7
-8
lines changed

velox/type/parser/ParserUtil.cpp

+2-3
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,8 @@ TypePtr typeFromString(
3030
upper = "DOUBLE";
3131
}
3232
auto inferredType = getType(upper, {});
33-
if (failIfNotRegistered) {
34-
VELOX_CHECK(
35-
inferredType, "Failed to parse type [{}]. Type not registered.", type);
33+
if (failIfNotRegistered == true && inferredType == nullptr) {
34+
VELOX_USER_FAIL("Failed to parse type [{}]. Type not registered.", type);
3635
}
3736
return inferredType;
3837
}

velox/type/parser/tests/TypeParserTest.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ TEST_F(TypeParserTest, invalidType) {
165165
VELOX_ASSERT_THROW(parseType("x"), "Failed to parse type [x]");
166166

167167
// Ensure this is not treated as a row type.
168-
VELOX_ASSERT_THROW(
168+
VELOX_ASSERT_USER_THROW(
169169
parseType("rowxxx(a)"), "Failed to parse type [a]. Type not registered.");
170170
}
171171

@@ -242,22 +242,22 @@ TEST_F(TypeParserTest, rowType) {
242242

243243
ASSERT_EQ(*parseType("row(array(Json))"), *ROW({ARRAY(JSON())}));
244244

245-
VELOX_ASSERT_THROW(
245+
VELOX_ASSERT_USER_THROW(
246246
*parseType("row(col0 row(array(HyperLogLog)))"),
247247
"Failed to parse type [HyperLogLog]. Type not registered.");
248248

249249
// Field type canonicalization.
250250
ASSERT_EQ(*parseType("row(col iNt)"), *ROW({"col"}, {INTEGER()}));
251251

252252
// Can only have names within rows.
253-
VELOX_ASSERT_THROW(
253+
VELOX_ASSERT_USER_THROW(
254254
parseType("asd bigint"),
255255
"Failed to parse type [asd bigint]. Type not registered.");
256256
}
257257

258258
TEST_F(TypeParserTest, typesWithSpaces) {
259259
// Type is not registered.
260-
VELOX_ASSERT_THROW(
260+
VELOX_ASSERT_USER_THROW(
261261
parseType("row(time time with time zone)"),
262262
"Failed to parse type [time with time zone]. Type not registered.");
263263

@@ -293,7 +293,7 @@ TEST_F(TypeParserTest, typesWithSpaces) {
293293
{TIMESTAMP_WITH_TIME_ZONE(), DOUBLE()}));
294294

295295
// quoted field name with invalid type with spaces.
296-
VELOX_ASSERT_THROW(
296+
VELOX_ASSERT_USER_THROW(
297297
parseType(
298298
"row(\"timestamp with time zone\" timestamp timestamp with time zone)"),
299299
"Failed to parse type [timestamp timestamp with time zone]. Type not registered.");

0 commit comments

Comments
 (0)