From 0698b7e51758dbf4a268fd3e0fd1afb0a763ba4f Mon Sep 17 00:00:00 2001 From: Zac Wen Date: Mon, 29 Jan 2024 08:55:08 -0800 Subject: [PATCH] Fix timezone issue in date_diff (#8540) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/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 --- velox/functions/prestosql/DateTimeFunctions.h | 15 ++++++++++----- .../prestosql/tests/DateTimeFunctionsTest.cpp | 8 ++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/velox/functions/prestosql/DateTimeFunctions.h b/velox/functions/prestosql/DateTimeFunctions.h index 421838e4adff..a141aa050322 100644 --- a/velox/functions/prestosql/DateTimeFunctions.h +++ b/velox/functions/prestosql/DateTimeFunctions.h @@ -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) { + const arg_type& 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; } @@ -1119,8 +1124,8 @@ struct DateDiffFunction : public TimestampWithTimezoneSupport { call( result, unitString, - this->toTimestamp(timestamp1), - this->toTimestamp(timestamp2)); + this->toTimestamp(timestamp1, true), + this->toTimestamp(timestamp2, true)); } }; diff --git a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp index 5f75818151d4..2b991032eec4 100644 --- a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp +++ b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp @@ -2502,7 +2502,7 @@ 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, @@ -2510,15 +2510,15 @@ TEST_F(DateTimeFunctionsTest, dateDiffTimestampWithTimezone) { 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,