diff --git a/velox/functions/lib/DateTimeFormatter.cpp b/velox/functions/lib/DateTimeFormatter.cpp index 5300dc47d586..4aac4b6c7135 100644 --- a/velox/functions/lib/DateTimeFormatter.cpp +++ b/velox/functions/lib/DateTimeFormatter.cpp @@ -1446,10 +1446,14 @@ Expected DateTimeFormatter::parse( date.timezoneId}; } -std::shared_ptr buildMysqlDateTimeFormatter( +Expected> buildMysqlDateTimeFormatter( const std::string_view& format) { if (format.empty()) { - VELOX_USER_FAIL("Both printing and parsing not supported"); + if (threadSkipErrorDetails()) { + return folly::makeUnexpected(Status::UserError()); + } + return folly::makeUnexpected( + Status::UserError("Both printing and parsing not supported")); } // For %r we should reserve 1 extra space because it has 3 literals ':' ':' @@ -1557,8 +1561,11 @@ std::shared_ptr buildMysqlDateTimeFormatter( case 'V': case 'w': case 'X': - VELOX_UNSUPPORTED( - "Date format specifier is not supported: %{}", *tokenEnd); + if (threadSkipErrorDetails()) { + return folly::makeUnexpected(Status::UserError()); + } + return folly::makeUnexpected(Status::UserError( + "Date format specifier is not supported: %{}", *tokenEnd)); default: builder.appendLiteral(tokenEnd, 1); break; @@ -1575,10 +1582,14 @@ std::shared_ptr buildMysqlDateTimeFormatter( return builder.setType(DateTimeFormatterType::MYSQL).build(); } -std::shared_ptr buildJodaDateTimeFormatter( +Expected> buildJodaDateTimeFormatter( const std::string_view& format) { if (format.empty()) { - VELOX_USER_FAIL("Invalid pattern specification"); + if (threadSkipErrorDetails()) { + return folly::makeUnexpected(Status::UserError()); + } + return folly::makeUnexpected( + Status::UserError("Invalid pattern specification")); } DateTimeFormatterBuilder builder(format.size()); @@ -1598,16 +1609,19 @@ std::shared_ptr buildJodaDateTimeFormatter( // Case 2: find closing single quote int64_t count = numLiteralChars(startTokenPtr + 1, end); if (count == -1) { - VELOX_USER_FAIL("No closing single quote for literal"); - } else { - for (int64_t i = 1; i <= count; i++) { - builder.appendLiteral(startTokenPtr + i, 1); - if (*(startTokenPtr + i) == '\'') { - i += 1; - } + if (threadSkipErrorDetails()) { + return folly::makeUnexpected(Status::UserError()); } - cur += count + 2; + return folly::makeUnexpected( + Status::UserError("No closing single quote for literal")); } + for (int64_t i = 1; i <= count; i++) { + builder.appendLiteral(startTokenPtr + i, 1); + if (*(startTokenPtr + i) == '\'') { + i += 1; + } + } + cur += count + 2; } } else { int count = 1; @@ -1686,7 +1700,11 @@ std::shared_ptr buildJodaDateTimeFormatter( break; default: if (isalpha(*startTokenPtr)) { - VELOX_UNSUPPORTED("Specifier {} is not supported.", *startTokenPtr); + if (threadSkipErrorDetails()) { + return folly::makeUnexpected(Status::UserError()); + } + return folly::makeUnexpected(Status::UserError( + "Specifier {} is not supported.", *startTokenPtr)); } else { builder.appendLiteral(startTokenPtr, cur - startTokenPtr); } @@ -1697,10 +1715,16 @@ std::shared_ptr buildJodaDateTimeFormatter( return builder.setType(DateTimeFormatterType::JODA).build(); } -std::shared_ptr buildSimpleDateTimeFormatter( +Expected> buildSimpleDateTimeFormatter( const std::string_view& format, bool lenient) { - VELOX_USER_CHECK(!format.empty(), "Format pattern should not be empty."); + if (format.empty()) { + if (threadSkipErrorDetails()) { + return folly::makeUnexpected(Status::UserError()); + } + return folly::makeUnexpected( + Status::UserError("Format pattern should not be empty")); + } DateTimeFormatterBuilder builder(format.size()); const char* cur = format.data(); @@ -1721,7 +1745,13 @@ std::shared_ptr buildSimpleDateTimeFormatter( // Append literal characters from the start until the next closing // literal sequence single quote. int64_t count = numLiteralChars(startTokenPtr + 1, end); - VELOX_USER_CHECK_NE(count, -1, "No closing single quote for literal"); + if (count == -1) { + if (threadSkipErrorDetails()) { + return folly::makeUnexpected(Status::UserError()); + } + return folly::makeUnexpected( + Status::UserError("No closing single quote for literal")); + } for (int64_t i = 1; i <= count; i++) { builder.appendLiteral(startTokenPtr + i, 1); if (*(startTokenPtr + i) == '\'') { @@ -1809,7 +1839,11 @@ std::shared_ptr buildSimpleDateTimeFormatter( break; default: if (isalpha(*startTokenPtr)) { - VELOX_UNSUPPORTED("Specifier {} is not supported.", *startTokenPtr); + if (threadSkipErrorDetails()) { + return folly::makeUnexpected(Status::UserError()); + } + return folly::makeUnexpected(Status::UserError( + "Specifier {} is not supported.", *startTokenPtr)); } else { builder.appendLiteral(startTokenPtr, cur - startTokenPtr); } diff --git a/velox/functions/lib/DateTimeFormatter.h b/velox/functions/lib/DateTimeFormatter.h index 9fbbcc1eba42..82fba6037cb1 100644 --- a/velox/functions/lib/DateTimeFormatter.h +++ b/velox/functions/lib/DateTimeFormatter.h @@ -215,13 +215,13 @@ class DateTimeFormatter { DateTimeFormatterType type_; }; -std::shared_ptr buildMysqlDateTimeFormatter( +Expected> buildMysqlDateTimeFormatter( const std::string_view& format); -std::shared_ptr buildJodaDateTimeFormatter( +Expected> buildJodaDateTimeFormatter( const std::string_view& format); -std::shared_ptr buildSimpleDateTimeFormatter( +Expected> buildSimpleDateTimeFormatter( const std::string_view& format, bool lenient); diff --git a/velox/functions/lib/tests/DateTimeFormatterTest.cpp b/velox/functions/lib/tests/DateTimeFormatterTest.cpp index 1066bf6cbeba..e57a39b88d97 100644 --- a/velox/functions/lib/tests/DateTimeFormatterTest.cpp +++ b/velox/functions/lib/tests/DateTimeFormatterTest.cpp @@ -65,6 +65,20 @@ class DateTimeFormatterTest : public testing::Test { }); } + std::shared_ptr getJodaDateTimeFormatter( + const std::string_view& format) const { + return buildJodaDateTimeFormatter(format).thenOrThrow( + folly::identity, + [&](const Status& status) { VELOX_USER_FAIL("{}", status.message()); }); + } + + std::shared_ptr getMysqlDateTimeFormatter( + const std::string_view& format) const { + return buildMysqlDateTimeFormatter(format).thenOrThrow( + folly::identity, + [&](const Status& status) { VELOX_USER_FAIL("{}", status.message()); }); + } + void testTokenRange( char specifier, int numTokenStart, @@ -74,7 +88,7 @@ class DateTimeFormatterTest : public testing::Test { std::string pattern(i, specifier); std::vector expected; expected = {DateTimeToken(FormatPattern{token, i})}; - EXPECT_EQ(expected, buildJodaDateTimeFormatter(pattern)->tokens()); + EXPECT_EQ(expected, getJodaDateTimeFormatter(pattern)->tokens()); } } @@ -88,8 +102,8 @@ class DateTimeFormatterTest : public testing::Test { DateTimeResult parseJoda( const std::string_view& input, const std::string_view& format) { - auto dateTimeResultExpected = - buildJodaDateTimeFormatter(format)->parse(input); + auto formatter = getJodaDateTimeFormatter(format); + auto dateTimeResultExpected = formatter->parse(input); return dateTimeResult(dateTimeResultExpected); } @@ -97,7 +111,7 @@ class DateTimeFormatterTest : public testing::Test { const std::string_view& input, const std::string_view& format) { auto dateTimeResultExpected = - buildMysqlDateTimeFormatter(format)->parse(input); + getMysqlDateTimeFormatter(format)->parse(input); return dateTimeResult(dateTimeResultExpected).timestamp; } @@ -107,7 +121,7 @@ class DateTimeFormatterTest : public testing::Test { const std::string_view& input, const std::string_view& format) { auto dateTimeResultExpected = - buildJodaDateTimeFormatter(format)->parse(input); + getJodaDateTimeFormatter(format)->parse(input); auto result = dateTimeResult(dateTimeResultExpected); if (result.timezoneId == 0) { return "+00:00"; @@ -119,7 +133,7 @@ class DateTimeFormatterTest : public testing::Test { const std::string& format, const Timestamp& timestamp, const tz::TimeZone* timezone) const { - auto formatter = buildMysqlDateTimeFormatter(format); + auto formatter = getMysqlDateTimeFormatter(format); const auto maxSize = formatter->maxResultSize(timezone); std::string result(maxSize, '\0'); auto resultSize = @@ -241,11 +255,11 @@ TEST_F(JodaDateTimeFormatterTest, validJodaBuild) { // G specifier case expected = {DateTimeToken(FormatPattern{DateTimeFormatSpecifier::ERA, 2})}; - EXPECT_EQ(expected, buildJodaDateTimeFormatter("G")->tokens()); + EXPECT_EQ(expected, getJodaDateTimeFormatter("G")->tokens()); // minRepresentDigits should be unchanged with higher number of specifier for // ERA expected = {DateTimeToken(FormatPattern{DateTimeFormatSpecifier::ERA, 2})}; - EXPECT_EQ(expected, buildJodaDateTimeFormatter("GGGG")->tokens()); + EXPECT_EQ(expected, getJodaDateTimeFormatter("GGGG")->tokens()); // C specifier case testTokenRange('C', 1, 3, DateTimeFormatSpecifier::CENTURY_OF_ERA); @@ -281,12 +295,12 @@ TEST_F(JodaDateTimeFormatterTest, validJodaBuild) { // a specifier case expected = { DateTimeToken(FormatPattern{DateTimeFormatSpecifier::HALFDAY_OF_DAY, 2})}; - EXPECT_EQ(expected, buildJodaDateTimeFormatter("a")->tokens()); + EXPECT_EQ(expected, getJodaDateTimeFormatter("a")->tokens()); // minRepresentDigits should be unchanged with higher number of specifier for // HALFDAY_OF_DAY expected = { DateTimeToken(FormatPattern{DateTimeFormatSpecifier::HALFDAY_OF_DAY, 2})}; - EXPECT_EQ(expected, buildJodaDateTimeFormatter("aa")->tokens()); + EXPECT_EQ(expected, getJodaDateTimeFormatter("aa")->tokens()); // K specifier case testTokenRange('K', 1, 4, DateTimeFormatSpecifier::HOUR_OF_HALFDAY); @@ -317,22 +331,22 @@ TEST_F(JodaDateTimeFormatterTest, validJodaBuild) { // Literal case expected = {DateTimeToken(" ")}; - EXPECT_EQ(expected, buildJodaDateTimeFormatter(" ")->tokens()); + EXPECT_EQ(expected, getJodaDateTimeFormatter(" ")->tokens()); expected = {DateTimeToken("1234567890")}; - EXPECT_EQ(expected, buildJodaDateTimeFormatter("1234567890")->tokens()); + EXPECT_EQ(expected, getJodaDateTimeFormatter("1234567890")->tokens()); expected = {DateTimeToken("'")}; - EXPECT_EQ(expected, buildJodaDateTimeFormatter("''")->tokens()); + EXPECT_EQ(expected, getJodaDateTimeFormatter("''")->tokens()); expected = {DateTimeToken("abcdefghijklmnopqrstuvwxyz")}; EXPECT_EQ( expected, - buildJodaDateTimeFormatter("'abcdefghijklmnopqrstuvwxyz'")->tokens()); + getJodaDateTimeFormatter("'abcdefghijklmnopqrstuvwxyz'")->tokens()); expected = {DateTimeToken("'abcdefg'hijklmnop'qrstuv'wxyz'")}; EXPECT_EQ( expected, - buildJodaDateTimeFormatter("'''abcdefg''hijklmnop''qrstuv''wxyz'''") + getJodaDateTimeFormatter("'''abcdefg''hijklmnop''qrstuv''wxyz'''") ->tokens()); expected = {DateTimeToken("'1234abcd")}; - EXPECT_EQ(expected, buildJodaDateTimeFormatter("''1234'abcd'")->tokens()); + EXPECT_EQ(expected, getJodaDateTimeFormatter("''1234'abcd'")->tokens()); // Specifier combinations expected = { @@ -381,22 +395,22 @@ TEST_F(JodaDateTimeFormatterTest, validJodaBuild) { EXPECT_EQ( expected, - buildJodaDateTimeFormatter( + getJodaDateTimeFormatter( "''CCC-YYYY/xxx//www-00-eeee--EEEEEE---yyyyy///DDDDMM-MMMMddddKKhhhkkHHmmsSSSSSSzzzZZZGGGG'abcdefghijklmnopqrstuvwxyz'aaa") ->tokens()); } TEST_F(JodaDateTimeFormatterTest, invalidJodaBuild) { // Invalid specifiers - EXPECT_THROW(buildJodaDateTimeFormatter("q"), VeloxUserError); - EXPECT_THROW(buildJodaDateTimeFormatter("r"), VeloxUserError); - EXPECT_THROW(buildJodaDateTimeFormatter("g"), VeloxUserError); + EXPECT_TRUE(buildJodaDateTimeFormatter("q").hasError()); + EXPECT_TRUE(buildJodaDateTimeFormatter("r").hasError()); + EXPECT_TRUE(buildJodaDateTimeFormatter("g").hasError()); // Unclosed literal sequence - EXPECT_THROW(buildJodaDateTimeFormatter("'abcd"), VeloxUserError); + EXPECT_TRUE(buildJodaDateTimeFormatter("'abcd").hasError()); // Empty format string - EXPECT_THROW(buildJodaDateTimeFormatter(""), VeloxUserError); + EXPECT_TRUE(buildJodaDateTimeFormatter("").hasError()); } TEST_F(JodaDateTimeFormatterTest, invalid) { @@ -1317,20 +1331,20 @@ TEST_F(JodaDateTimeFormatterTest, formatResultSize) { auto* timezone = tz::locateZone("GMT"); EXPECT_EQ( - buildJodaDateTimeFormatter("yyyy-MM-dd")->maxResultSize(timezone), 12); - EXPECT_EQ(buildJodaDateTimeFormatter("yyyy-MM")->maxResultSize(timezone), 9); - EXPECT_EQ(buildJodaDateTimeFormatter("y")->maxResultSize(timezone), 6); + getJodaDateTimeFormatter("yyyy-MM-dd")->maxResultSize(timezone), 12); + EXPECT_EQ(getJodaDateTimeFormatter("yyyy-MM")->maxResultSize(timezone), 9); + EXPECT_EQ(getJodaDateTimeFormatter("y")->maxResultSize(timezone), 6); EXPECT_EQ( - buildJodaDateTimeFormatter("yyyy////MM////dd")->maxResultSize(timezone), + getJodaDateTimeFormatter("yyyy////MM////dd")->maxResultSize(timezone), 18); EXPECT_EQ( - buildJodaDateTimeFormatter("yyyy-MM-dd HH:mm:ss.SSS") + getJodaDateTimeFormatter("yyyy-MM-dd HH:mm:ss.SSS") ->maxResultSize(timezone), 31); // No padding. CENTURY_OF_ERA can be at most 3 digits. - EXPECT_EQ(buildJodaDateTimeFormatter("C")->maxResultSize(timezone), 3); + EXPECT_EQ(getJodaDateTimeFormatter("C")->maxResultSize(timezone), 3); // Needs to pad to make result contain 4 digits. - EXPECT_EQ(buildJodaDateTimeFormatter("CCCC")->maxResultSize(timezone), 4); + EXPECT_EQ(getJodaDateTimeFormatter("CCCC")->maxResultSize(timezone), 4); } TEST_F(JodaDateTimeFormatterTest, betterErrorMessaging) { @@ -1349,31 +1363,31 @@ TEST_F(MysqlDateTimeTest, validBuild) { std::vector expected; expected = {DateTimeToken(" ")}; - EXPECT_EQ(expected, buildMysqlDateTimeFormatter(" ")->tokens()); + EXPECT_EQ(expected, getMysqlDateTimeFormatter(" ")->tokens()); expected = { DateTimeToken(" "), DateTimeToken(FormatPattern{DateTimeFormatSpecifier::YEAR, 4}), }; - EXPECT_EQ(expected, buildMysqlDateTimeFormatter(" %Y")->tokens()); + EXPECT_EQ(expected, getMysqlDateTimeFormatter(" %Y")->tokens()); expected = { DateTimeToken(FormatPattern{DateTimeFormatSpecifier::YEAR, 2}), DateTimeToken(" "), }; - EXPECT_EQ(expected, buildMysqlDateTimeFormatter("%y ")->tokens()); + EXPECT_EQ(expected, getMysqlDateTimeFormatter("%y ")->tokens()); expected = {DateTimeToken(" 132&2618*673 *--+= }{[]\\:")}; EXPECT_EQ( expected, - buildMysqlDateTimeFormatter(" 132&2618*673 *--+= }{[]\\:")->tokens()); + getMysqlDateTimeFormatter(" 132&2618*673 *--+= }{[]\\:")->tokens()); expected = { DateTimeToken(" "), DateTimeToken(FormatPattern{DateTimeFormatSpecifier::YEAR, 4}), DateTimeToken(" &^ "), }; - EXPECT_EQ(expected, buildMysqlDateTimeFormatter(" %Y &^ ")->tokens()); + EXPECT_EQ(expected, getMysqlDateTimeFormatter(" %Y &^ ")->tokens()); expected = { DateTimeToken(FormatPattern{DateTimeFormatSpecifier::YEAR, 2}), @@ -1382,16 +1396,16 @@ TEST_F(MysqlDateTimeTest, validBuild) { DateTimeToken(" "), DateTimeToken(FormatPattern{DateTimeFormatSpecifier::YEAR, 4}), }; - EXPECT_EQ(expected, buildMysqlDateTimeFormatter("%y % & %Y %Y%")->tokens()); + EXPECT_EQ(expected, getMysqlDateTimeFormatter("%y % & %Y %Y%")->tokens()); expected = { DateTimeToken(FormatPattern{DateTimeFormatSpecifier::YEAR, 4}), DateTimeToken(" 'T'"), }; - EXPECT_EQ(expected, buildMysqlDateTimeFormatter("%Y 'T'")->tokens()); + EXPECT_EQ(expected, getMysqlDateTimeFormatter("%Y 'T'")->tokens()); expected = {DateTimeToken("1''2")}; - EXPECT_EQ(expected, buildMysqlDateTimeFormatter("1''2")->tokens()); + EXPECT_EQ(expected, getMysqlDateTimeFormatter("1''2")->tokens()); expected = { DateTimeToken(FormatPattern{DateTimeFormatSpecifier::YEAR, 4}), @@ -1404,19 +1418,19 @@ TEST_F(MysqlDateTimeTest, validBuild) { DateTimeToken(" "), DateTimeToken(FormatPattern{DateTimeFormatSpecifier::YEAR, 2}), }; - EXPECT_EQ(expected, buildMysqlDateTimeFormatter("%Y-%m-%d %i %y")->tokens()); + EXPECT_EQ(expected, getMysqlDateTimeFormatter("%Y-%m-%d %i %y")->tokens()); } TEST_F(MysqlDateTimeTest, invalidBuild) { // Unsupported specifiers - EXPECT_THROW(buildMysqlDateTimeFormatter("%D"), VeloxUserError); - EXPECT_THROW(buildMysqlDateTimeFormatter("%U"), VeloxUserError); - EXPECT_THROW(buildMysqlDateTimeFormatter("%u"), VeloxUserError); - EXPECT_THROW(buildMysqlDateTimeFormatter("%V"), VeloxUserError); - EXPECT_THROW(buildMysqlDateTimeFormatter("%w"), VeloxUserError); + EXPECT_TRUE(buildMysqlDateTimeFormatter("%D").hasError()); + EXPECT_TRUE(buildMysqlDateTimeFormatter("%U").hasError()); + EXPECT_TRUE(buildMysqlDateTimeFormatter("%u").hasError()); + EXPECT_TRUE(buildMysqlDateTimeFormatter("%V").hasError()); + EXPECT_TRUE(buildMysqlDateTimeFormatter("%w").hasError()); // Empty format string - EXPECT_THROW(buildMysqlDateTimeFormatter(""), VeloxUserError); + EXPECT_TRUE(buildMysqlDateTimeFormatter("").hasError()); } TEST_F(MysqlDateTimeTest, formatYear) { diff --git a/velox/functions/prestosql/DateTimeFunctions.h b/velox/functions/prestosql/DateTimeFunctions.h index 7672a2184bcb..6f149a545913 100644 --- a/velox/functions/prestosql/DateTimeFunctions.h +++ b/velox/functions/prestosql/DateTimeFunctions.h @@ -1482,8 +1482,12 @@ struct DateFormatFunction : public TimestampWithTimezoneSupport { private: FOLLY_ALWAYS_INLINE void setFormatter(const arg_type formatString) { - mysqlDateTime_ = buildMysqlDateTimeFormatter( - std::string_view(formatString.data(), formatString.size())); + mysqlDateTime_ = + buildMysqlDateTimeFormatter( + std::string_view(formatString.data(), formatString.size())) + .thenOrThrow(folly::identity, [&](const Status& status) { + VELOX_USER_FAIL("{}", status.message()); + }); maxResultSize_ = mysqlDateTime_->maxResultSize(sessionTimeZone_); } @@ -1564,8 +1568,12 @@ struct DateParseFunction { const arg_type* /*input*/, const arg_type* formatString) { if (formatString != nullptr) { - format_ = buildMysqlDateTimeFormatter( - std::string_view(formatString->data(), formatString->size())); + format_ = + buildMysqlDateTimeFormatter( + std::string_view(formatString->data(), formatString->size())) + .thenOrThrow(folly::identity, [&](const Status& status) { + VELOX_USER_FAIL("{}", status.message()); + }); isConstFormat_ = true; } @@ -1581,7 +1589,10 @@ struct DateParseFunction { const arg_type& format) { if (!isConstFormat_) { format_ = buildMysqlDateTimeFormatter( - std::string_view(format.data(), format.size())); + std::string_view(format.data(), format.size())) + .thenOrThrow(folly::identity, [&](const Status& status) { + VELOX_USER_FAIL("{}", status.message()); + }); } auto dateTimeResult = format_->parse((std::string_view)(input)); @@ -1643,9 +1654,12 @@ struct FormatDateTimeFunction { } FOLLY_ALWAYS_INLINE void setFormatter(const arg_type& formatString) { - jodaDateTime_ = buildJodaDateTimeFormatter( - std::string_view(formatString.data(), formatString.size())); - maxResultSize_ = jodaDateTime_->maxResultSize(sessionTimeZone_); + buildJodaDateTimeFormatter( + std::string_view(formatString.data(), formatString.size())) + .thenOrThrow([this](auto formatter) { + jodaDateTime_ = formatter; + maxResultSize_ = jodaDateTime_->maxResultSize(sessionTimeZone_); + }); } void format( @@ -1680,7 +1694,10 @@ struct ParseDateTimeFunction { const arg_type* format) { if (format != nullptr) { format_ = buildJodaDateTimeFormatter( - std::string_view(format->data(), format->size())); + std::string_view(format->data(), format->size())) + .thenOrThrow(folly::identity, [&](const Status& status) { + VELOX_USER_FAIL("{}", status.message()); + }); isConstFormat_ = true; } @@ -1696,7 +1713,10 @@ struct ParseDateTimeFunction { const arg_type& format) { if (!isConstFormat_) { format_ = buildJodaDateTimeFormatter( - std::string_view(format.data(), format.size())); + std::string_view(format.data(), format.size())) + .thenOrThrow(folly::identity, [&](const Status& status) { + VELOX_USER_FAIL("{}", status.message()); + }); } auto dateTimeResult = format_->parse(std::string_view(input.data(), input.size())); @@ -1770,6 +1790,13 @@ template struct ToISO8601Function { VELOX_DEFINE_FUNCTION_TYPES(T); + ToISO8601Function() { + auto formatter = + functions::buildJodaDateTimeFormatter("yyyy-MM-dd'T'HH:mm:ss.SSSZZ"); + VELOX_CHECK(!formatter.hasError(), "Default format should always be valid"); + formatter_ = formatter.value(); + } + FOLLY_ALWAYS_INLINE void initialize( const std::vector& inputTypes, const core::QueryConfig& config, @@ -1814,8 +1841,7 @@ struct ToISO8601Function { } const tz::TimeZone* timeZone_{nullptr}; - std::shared_ptr formatter_ = - functions::buildJodaDateTimeFormatter("yyyy-MM-dd'T'HH:mm:ss.SSSZZ"); + std::shared_ptr formatter_; }; template diff --git a/velox/functions/prestosql/types/TimestampWithTimeZoneType.cpp b/velox/functions/prestosql/types/TimestampWithTimeZoneType.cpp index bf8bd4a0cbd4..806535904bd9 100644 --- a/velox/functions/prestosql/types/TimestampWithTimeZoneType.cpp +++ b/velox/functions/prestosql/types/TimestampWithTimeZoneType.cpp @@ -109,26 +109,27 @@ void castToString( auto* flatResult = result.as>(); const auto* timestamps = input.as>(); - auto formatter = - functions::buildJodaDateTimeFormatter("yyyy-MM-dd HH:mm:ss.SSS zzzz"); - - context.applyToSelectedNoThrow(rows, [&](auto row) { - const auto timestampWithTimezone = timestamps->valueAt(row); - - const auto timestamp = unpackTimestampUtc(timestampWithTimezone); - const auto timeZoneId = unpackZoneKeyId(timestampWithTimezone); - const auto* timezonePtr = tz::locateZone(tz::getTimeZoneName(timeZoneId)); - - exec::StringWriter result(flatResult, row); - - const auto maxResultSize = formatter->maxResultSize(timezonePtr); - result.reserve(maxResultSize); - const auto resultSize = - formatter->format(timestamp, timezonePtr, maxResultSize, result.data()); - result.resize(resultSize); - - result.finalize(); - }); + functions::buildJodaDateTimeFormatter("yyyy-MM-dd HH:mm:ss.SSS zzzz") + .thenOrThrow([&](auto formatter) { + context.applyToSelectedNoThrow(rows, [&](auto row) { + const auto timestampWithTimezone = timestamps->valueAt(row); + + const auto timestamp = unpackTimestampUtc(timestampWithTimezone); + const auto timeZoneId = unpackZoneKeyId(timestampWithTimezone); + const auto* timezonePtr = + tz::locateZone(tz::getTimeZoneName(timeZoneId)); + + exec::StringWriter result(flatResult, row); + + const auto maxResultSize = formatter->maxResultSize(timezonePtr); + result.reserve(maxResultSize); + const auto resultSize = formatter->format( + timestamp, timezonePtr, maxResultSize, result.data()); + result.resize(resultSize); + + result.finalize(); + }); + }); } void castToTimestamp( diff --git a/velox/functions/sparksql/DateTimeFunctions.h b/velox/functions/sparksql/DateTimeFunctions.h index 89facdd01503..e76351a8eb48 100644 --- a/velox/functions/sparksql/DateTimeFunctions.h +++ b/velox/functions/sparksql/DateTimeFunctions.h @@ -26,7 +26,7 @@ namespace facebook::velox::functions::sparksql { namespace detail { -std::shared_ptr getDateTimeFormatter( +Expected> getDateTimeFormatter( const std::string_view& format, DateTimeFormatterType type) { switch (type) { @@ -172,10 +172,12 @@ struct UnixTimestampParseFunction { const std::vector& /*inputTypes*/, const core::QueryConfig& config, const arg_type* /*input*/) { - format_ = detail::getDateTimeFormatter( + auto formatter = detail::getDateTimeFormatter( kDefaultFormat_, config.sparkLegacyDateFormatter() ? DateTimeFormatterType::STRICT_SIMPLE : DateTimeFormatterType::JODA); + VELOX_CHECK(!formatter.hasError(), "Default format should always be valid"); + format_ = formatter.value(); setTimezone(config); } @@ -226,13 +228,14 @@ struct UnixTimestampParseWithFormatFunction const arg_type* format) { legacyFormatter_ = config.sparkLegacyDateFormatter(); if (format != nullptr) { - try { - this->format_ = detail::getDateTimeFormatter( - std::string_view(format->data(), format->size()), - legacyFormatter_ ? DateTimeFormatterType::STRICT_SIMPLE - : DateTimeFormatterType::JODA); - } catch (const VeloxUserError&) { + auto formatter = detail::getDateTimeFormatter( + std::string_view(format->data(), format->size()), + legacyFormatter_ ? DateTimeFormatterType::STRICT_SIMPLE + : DateTimeFormatterType::JODA); + if (formatter.hasError()) { invalidFormat_ = true; + } else { + this->format_ = formatter.value(); } isConstFormat_ = true; } @@ -248,15 +251,15 @@ struct UnixTimestampParseWithFormatFunction } // Format error returns null. - try { - if (!isConstFormat_) { - this->format_ = detail::getDateTimeFormatter( - std::string_view(format.data(), format.size()), - legacyFormatter_ ? DateTimeFormatterType::STRICT_SIMPLE - : DateTimeFormatterType::JODA); + if (!isConstFormat_) { + auto formatter = detail::getDateTimeFormatter( + std::string_view(format.data(), format.size()), + legacyFormatter_ ? DateTimeFormatterType::STRICT_SIMPLE + : DateTimeFormatterType::JODA); + if (formatter.hasError()) { + return false; } - } catch (const VeloxUserError&) { - return false; + this->format_ = formatter.value(); } auto dateTimeResult = this->format_->parse(std::string_view(input.data(), input.size())); @@ -311,9 +314,12 @@ struct FromUnixtimeFunction { private: FOLLY_ALWAYS_INLINE void setFormatter(const arg_type& format) { formatter_ = detail::getDateTimeFormatter( - std::string_view(format.data(), format.size()), - legacyFormatter_ ? DateTimeFormatterType::STRICT_SIMPLE - : DateTimeFormatterType::JODA); + std::string_view(format.data(), format.size()), + legacyFormatter_ ? DateTimeFormatterType::STRICT_SIMPLE + : DateTimeFormatterType::JODA) + .thenOrThrow(folly::identity, [&](const Status& status) { + VELOX_USER_FAIL("{}", status.message()); + }); maxResultSize_ = formatter_->maxResultSize(sessionTimeZone_); } @@ -402,9 +408,12 @@ struct GetTimestampFunction { } if (format != nullptr) { formatter_ = detail::getDateTimeFormatter( - std::string_view(*format), - legacyFormatter_ ? DateTimeFormatterType::STRICT_SIMPLE - : DateTimeFormatterType::JODA); + std::string_view(*format), + legacyFormatter_ ? DateTimeFormatterType::STRICT_SIMPLE + : DateTimeFormatterType::JODA) + .thenOrThrow(folly::identity, [&](const Status& status) { + VELOX_USER_FAIL("{}", status.message()); + }); isConstantTimeFormat_ = true; } } @@ -415,9 +424,12 @@ struct GetTimestampFunction { const arg_type& format) { if (!isConstantTimeFormat_) { formatter_ = detail::getDateTimeFormatter( - std::string_view(format), - legacyFormatter_ ? DateTimeFormatterType::STRICT_SIMPLE - : DateTimeFormatterType::JODA); + std::string_view(format), + legacyFormatter_ ? DateTimeFormatterType::STRICT_SIMPLE + : DateTimeFormatterType::JODA) + .thenOrThrow(folly::identity, [&](const Status& status) { + VELOX_USER_FAIL("{}", status.message()); + }); } auto dateTimeResult = formatter_->parse(std::string_view(input)); // Null as result for parsing error.