Skip to content

Commit

Permalink
Throw for zzzz (and beyond) in parse_datetime (#11331)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #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

Differential Revision: D64797504
  • Loading branch information
Kevin Wilfong authored and facebook-github-bot committed Oct 24, 2024
1 parent 14d71d1 commit 251addf
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
6 changes: 6 additions & 0 deletions velox/functions/lib/DateTimeFormatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,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;
Expand Down
13 changes: 13 additions & 0 deletions velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 251addf

Please sign in to comment.