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, pedroerp

Differential Revision: D64797504

fbshipit-source-id: 0d361f6439bd8f440a167dedafd39d282f2948a0
  • Loading branch information
Kevin Wilfong authored and facebook-github-bot committed Oct 29, 2024
1 parent 1b5b013 commit 3bcc5f3
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 @@ -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;
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 3bcc5f3

Please sign in to comment.