From 7915dea4a7f3bfc214511975e9446f763823b1a9 Mon Sep 17 00:00:00 2001 From: Weiguo Chen Date: Sat, 18 Feb 2023 05:51:54 -0800 Subject: [PATCH] Add parseIntegerAsBigint option to DuckParser (#4074) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/4074 Reviewed By: xiaoxmeng Differential Revision: D43416285 Pulled By: mbasmanova fbshipit-source-id: 94129bccbbd59e0756dedc6d7bfeb7f3ce373589 --- velox/duckdb/conversion/DuckParser.cpp | 3 ++- velox/duckdb/conversion/DuckParser.h | 1 + velox/duckdb/conversion/tests/DuckParserTest.cpp | 12 ++++++++++++ velox/parse/ExpressionsParser.cpp | 3 +++ velox/parse/ExpressionsParser.h | 1 + 5 files changed, 19 insertions(+), 1 deletion(-) diff --git a/velox/duckdb/conversion/DuckParser.cpp b/velox/duckdb/conversion/DuckParser.cpp index 9031ffdeb221..6a710266ff17 100644 --- a/velox/duckdb/conversion/DuckParser.cpp +++ b/velox/duckdb/conversion/DuckParser.cpp @@ -131,7 +131,8 @@ std::shared_ptr parseConstantExpr( // This is a hack to make DuckDB more compatible with the old Koski-based // parser. By default literal integer constants in DuckDB parser are INTEGER, // while in Koski parser they were BIGINT. - if (value.type().id() == LogicalTypeId::INTEGER) { + if (value.type().id() == LogicalTypeId::INTEGER && + options.parseIntegerAsBigint) { value = Value::BIGINT(value.GetValue()); } diff --git a/velox/duckdb/conversion/DuckParser.h b/velox/duckdb/conversion/DuckParser.h index 196e90672e88..e928b7ae9c2c 100644 --- a/velox/duckdb/conversion/DuckParser.h +++ b/velox/duckdb/conversion/DuckParser.h @@ -29,6 +29,7 @@ namespace facebook::velox::duckdb { struct ParseOptions { // Retain legacy behavior by default. bool parseDecimalAsDouble = true; + bool parseIntegerAsBigint = true; }; // Parses an input expression using DuckDB's internal postgresql-based parser, diff --git a/velox/duckdb/conversion/tests/DuckParserTest.cpp b/velox/duckdb/conversion/tests/DuckParserTest.cpp index 2278e532ce8c..cd991e282d29 100644 --- a/velox/duckdb/conversion/tests/DuckParserTest.cpp +++ b/velox/duckdb/conversion/tests/DuckParserTest.cpp @@ -529,6 +529,18 @@ TEST(DuckParserTest, parseDecimalConstant) { } } +TEST(DuckParserTest, parseInteger) { + ParseOptions options; + options.parseIntegerAsBigint = false; + auto expr = parseExpr("1234", options); + if (auto constant = + std::dynamic_pointer_cast(expr)) { + ASSERT_EQ(*constant->type(), *INTEGER()); + } else { + FAIL() << expr->toString() << " is not a constant"; + } +} + TEST(DuckParserTest, lambda) { // There is a bug in DuckDB in parsing lambda expressions that use // comparisons. This doesn't work: filter(a, x -> x = 10). This does: diff --git a/velox/parse/ExpressionsParser.cpp b/velox/parse/ExpressionsParser.cpp index cd07c86f7249..6ef233a1ceeb 100644 --- a/velox/parse/ExpressionsParser.cpp +++ b/velox/parse/ExpressionsParser.cpp @@ -23,6 +23,8 @@ std::shared_ptr parseExpr( const ParseOptions& options) { facebook::velox::duckdb::ParseOptions duckConversionOptions; duckConversionOptions.parseDecimalAsDouble = options.parseDecimalAsDouble; + duckConversionOptions.parseIntegerAsBigint = options.parseIntegerAsBigint; + return facebook::velox::duckdb::parseExpr(expr, duckConversionOptions); } @@ -31,6 +33,7 @@ std::vector> parseMultipleExpressions( const ParseOptions& options) { facebook::velox::duckdb::ParseOptions duckConversionOptions; duckConversionOptions.parseDecimalAsDouble = options.parseDecimalAsDouble; + duckConversionOptions.parseIntegerAsBigint = options.parseIntegerAsBigint; return facebook::velox::duckdb::parseMultipleExpressions( expr, duckConversionOptions); } diff --git a/velox/parse/ExpressionsParser.h b/velox/parse/ExpressionsParser.h index f02393e350d3..e5f5685bd3d9 100644 --- a/velox/parse/ExpressionsParser.h +++ b/velox/parse/ExpressionsParser.h @@ -26,6 +26,7 @@ namespace facebook::velox::parse { struct ParseOptions { // Retain legacy behavior by default. bool parseDecimalAsDouble = true; + bool parseIntegerAsBigint = true; }; std::shared_ptr parseExpr(