Skip to content

Commit

Permalink
Fix timezone issue in date_diff (#8540)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #8540

In Presto Jave, timestamp with time zone in date_diff is converted to
timestamp as GMT (simply drop the timezone field). Velox needs to match
this behavior.
Presto code ref: https://github.com/prestodb/presto/blob/0.286-edge17/presto-main/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java#L500

Reviewed By: Yuhta

Differential Revision: D53105892

fbshipit-source-id: 5187d079cfbd1be82de4752544ca3752561ab03c
  • Loading branch information
zacw7 authored and facebook-github-bot committed Jan 29, 2024
1 parent f3b91e3 commit 0698b7e
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 9 deletions.
15 changes: 10 additions & 5 deletions velox/functions/prestosql/DateTimeFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,18 @@ struct TimestampWithTimezoneSupport {
VELOX_DEFINE_FUNCTION_TYPES(T);

// Convert timestampWithTimezone to a timestamp representing the moment at the
// zone in timestampWithTimezone.
// zone in timestampWithTimezone. If `asGMT` is set to true, return the GMT
// time at the same moment.
FOLLY_ALWAYS_INLINE
Timestamp toTimestamp(
const arg_type<TimestampWithTimezone>& timestampWithTimezone) {
const arg_type<TimestampWithTimezone>& timestampWithTimezone,
bool asGMT = false) {
const auto milliseconds = *timestampWithTimezone.template at<0>();
auto tz = *timestampWithTimezone.template at<1>();
Timestamp timestamp = Timestamp::fromMillis(milliseconds);
timestamp.toTimezone(*timestampWithTimezone.template at<1>());
if (!asGMT) {
timestamp.toTimezone(*timestampWithTimezone.template at<1>());
}

return timestamp;
}
Expand Down Expand Up @@ -1119,8 +1124,8 @@ struct DateDiffFunction : public TimestampWithTimezoneSupport<T> {
call(
result,
unitString,
this->toTimestamp(timestamp1),
this->toTimestamp(timestamp2));
this->toTimestamp(timestamp1, true),
this->toTimestamp(timestamp2, true));
}
};

Expand Down
8 changes: 4 additions & 4 deletions velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2502,23 +2502,23 @@ TEST_F(DateTimeFunctionsTest, dateDiffTimestampWithTimezone) {
// timestamp1: 1970-01-01 00:00:00.000 +00:00 (0)
// timestamp2: 2020-08-25 16:30:10.123 -08:00 (1'598'373'010'123)
EXPECT_EQ(
1598347810123,
1598373010123,
dateDiff(
"millisecond",
0,
"+00:00",
1'598'373'010'123,
"America/Los_Angeles"));
EXPECT_EQ(
1598347810,
1598373010,
dateDiff(
"second", 0, "+00:00", 1'598'373'010'123, "America/Los_Angeles"));
EXPECT_EQ(
26639130,
26639550,
dateDiff(
"minute", 0, "+00:00", 1'598'373'010'123, "America/Los_Angeles"));
EXPECT_EQ(
443985,
443992,
dateDiff("hour", 0, "+00:00", 1'598'373'010'123, "America/Los_Angeles"));
EXPECT_EQ(
18499,
Expand Down

0 comments on commit 0698b7e

Please sign in to comment.