Skip to content

Commit 74df94d

Browse files
Krishna Paifacebook-github-bot
authored andcommitted
Fix DateTimeFormatter to set weekDateFormat only if it has week and day of week along with year. (facebookincubator#12456)
Summary: This is a targeted fix that ensures that date_parse formats which contain %a but without additional specifiers like %v does not set weekDateFormat. An example of such a format is '%d-%b-%Y (%a)' which when parsing a string like '11-Mar-2024 (Mon)' will incorrectly ignore the month and day and instead return a date like '2024-01-01' . There are likely other cases where additional or inconsistent specifiers are handled incorrectly. A full fix of DateTimeFormatter is beyond scope of this PR, but I will send out additional PR's in the future to address these issues. Mitigates S493660. Reviewed By: pedroerp Differential Revision: D70253624
1 parent 6983c4b commit 74df94d

File tree

2 files changed

+14
-3
lines changed

2 files changed

+14
-3
lines changed

velox/functions/lib/DateTimeFormatter.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ struct Date {
6060
bool isYearOfEra = false; // Year of era cannot be zero or negative.
6161
bool hasYear = false; // Whether year was explicitly specified.
6262
bool hasDayOfWeek = false; // Whether dayOfWeek was explicitly specified.
63+
bool hasWeek = false; // Whether week was explicitly specified.
6364

6465
int32_t hour = 0;
6566
int32_t minute = 0;
@@ -870,9 +871,7 @@ int32_t parseFromPattern(
870871
return -1;
871872
}
872873
cur += size;
873-
if (!date.weekOfMonthDateFormat) {
874-
date.weekDateFormat = true;
875-
}
874+
date.hasDayOfWeek = true;
876875
date.dayOfYearFormat = false;
877876
if (!date.hasYear) {
878877
date.hasYear = true;
@@ -1095,6 +1094,7 @@ int32_t parseFromPattern(
10951094
return -1;
10961095
}
10971096
date.year = number;
1097+
date.hasWeek = true;
10981098
date.weekDateFormat = true;
10991099
date.dayOfYearFormat = false;
11001100
date.centuryFormat = false;
@@ -1107,6 +1107,7 @@ int32_t parseFromPattern(
11071107
return -1;
11081108
}
11091109
date.week = number;
1110+
date.hasWeek = true;
11101111
date.weekDateFormat = true;
11111112
date.dayOfYearFormat = false;
11121113
date.weekOfMonthDateFormat = false;
@@ -1637,6 +1638,10 @@ Expected<DateTimeResult> DateTimeFormatter::parse(
16371638

16381639
// Convert the parsed date/time into a timestamp.
16391640
Expected<int64_t> daysSinceEpoch;
1641+
1642+
// Ensure you use week date format only when you have year and at least week.
1643+
date.weekDateFormat = date.hasYear && date.hasWeek;
1644+
16401645
if (date.weekDateFormat) {
16411646
daysSinceEpoch =
16421647
util::daysSinceEpochFromWeekDate(date.year, date.week, date.dayOfWeek);

velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4530,6 +4530,12 @@ TEST_F(DateTimeFunctionsTest, dateParse) {
45304530
EXPECT_EQ(
45314531
Timestamp(-66600, 0), dateParse("1969-12-31+11:00", "%Y-%m-%d+%H:%i"));
45324532

4533+
setQueryTimeZone("America/Los_Angeles");
4534+
// Tests if it uses weekdateformat if %v not present but %a is present.
4535+
EXPECT_EQ(
4536+
Timestamp(1730707200, 0),
4537+
dateParse("04-Nov-2024 (Mon)", "%d-%b-%Y (%a)"));
4538+
45334539
VELOX_ASSERT_THROW(dateParse("", "%y+"), "Invalid date format: ''");
45344540
VELOX_ASSERT_THROW(dateParse("1", "%y+"), "Invalid date format: '1'");
45354541
VELOX_ASSERT_THROW(dateParse("116", "%y+"), "Invalid date format: '116'");

0 commit comments

Comments
 (0)