From 3bcc5f385e6c011d0441130f9d07a82780b9f25d Mon Sep 17 00:00:00 2001 From: Kevin Wilfong Date: Tue, 29 Oct 2024 12:51:02 -0700 Subject: [PATCH] Throw for zzzz (and beyond) in parse_datetime (#11331) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/11331 This diff throws a user exception for JODA's zzzz (or more) patterns (all equivalent) in Presto's parse_datetime function. JODA does not support parsing time zone long names (specified by the pattern zzzz) so neither should Velox to be consistent. Previously we were treating any number of 'z's as representing a time zone abbreviation in the pattern which is completely wrong. Reviewed By: amitkdutta, pedroerp Differential Revision: D64797504 fbshipit-source-id: 0d361f6439bd8f440a167dedafd39d282f2948a0 --- velox/functions/lib/DateTimeFormatter.cpp | 6 ++++++ .../prestosql/tests/DateTimeFunctionsTest.cpp | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/velox/functions/lib/DateTimeFormatter.cpp b/velox/functions/lib/DateTimeFormatter.cpp index 5676a63efc8c..32e1ab68f060 100644 --- a/velox/functions/lib/DateTimeFormatter.cpp +++ b/velox/functions/lib/DateTimeFormatter.cpp @@ -828,6 +828,12 @@ int32_t parseFromPattern( } cur += size; } else if (curPattern.specifier == DateTimeFormatSpecifier::TIMEZONE) { + // JODA does not support parsing time zone long names, so neither do we for + // consistency. The pattern for a time zone long name is 4 or more 'z's. + VELOX_USER_CHECK_LT( + curPattern.minRepresentDigits, + 4, + "Parsing time zone long names is not supported."); auto size = parseTimezone(cur, end, date); if (size == -1) { return -1; diff --git a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp index 30090173d8f8..9cce58653252 100644 --- a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp +++ b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp @@ -3022,6 +3022,19 @@ TEST_F(DateTimeFunctionsTest, parseDatetime) { parseDatetime( "2024-02-25+06:00:99 America/XYZ", "yyyy-MM-dd+HH:mm:99 ZZZ"), "Invalid date format: '2024-02-25+06:00:99 America/XYZ'"); + + // Test to ensure we do not support parsing time zone long names (to be + // consistent with JODA). + VELOX_ASSERT_THROW( + parseDatetime( + "2024-02-25+06:00:99 Pacific Standard Time", + "yyyy-MM-dd+HH:mm:99 zzzz"), + "Parsing time zone long names is not supported."); + VELOX_ASSERT_THROW( + parseDatetime( + "2024-02-25+06:00:99 Pacific Standard Time", + "yyyy-MM-dd+HH:mm:99 zzzzzzzzzz"), + "Parsing time zone long names is not supported."); } TEST_F(DateTimeFunctionsTest, formatDateTime) {