diff --git a/velox/functions/prestosql/DateTimeFunctions.h b/velox/functions/prestosql/DateTimeFunctions.h index e1c5b3b29aa9..d639d12fe68b 100644 --- a/velox/functions/prestosql/DateTimeFunctions.h +++ b/velox/functions/prestosql/DateTimeFunctions.h @@ -1118,7 +1118,7 @@ struct DateTruncFunction : public TimestampWithTimezoneSupport { Timestamp::fromMillis(Timestamp::calendarUtcToEpoch(dateTime) * 1000); timestamp.toGMT(unpackZoneKeyId(timestampWithTimezone)); - result = pack(timestamp.toMillis(), unpackZoneKeyId(timestampWithTimezone)); + result = pack(timestamp, unpackZoneKeyId(timestampWithTimezone)); } private: @@ -1207,9 +1207,8 @@ struct DateAddFunction : public TimestampWithTimezoneSupport { const arg_type& unitString, const int64_t value, const arg_type& timestampWithTimezone) { - const auto unit = unit_.has_value() - ? unit_.value() - : fromDateTimeUnitString(unitString, true /*throwIfInvalid*/).value(); + const auto unit = unit_.value_or( + fromDateTimeUnitString(unitString, true /*throwIfInvalid*/).value()); if (value != (int32_t)value) { VELOX_UNSUPPORTED("integer overflow"); @@ -1443,7 +1442,7 @@ struct FromIso8601Timestamp { tzID = sessionTzID_; } ts.toGMT(tzID); - result = pack(ts.toMillis(), tzID); + result = pack(ts, tzID); return Status::OK(); } @@ -1614,7 +1613,7 @@ struct ParseDateTimeFunction { ? dateTimeResult->timezoneId : sessionTzID_.value_or(0); dateTimeResult->timestamp.toGMT(timezoneId); - result = pack(dateTimeResult->timestamp.toMillis(), timezoneId); + result = pack(dateTimeResult->timestamp, timezoneId); return Status::OK(); } }; diff --git a/velox/functions/prestosql/DateTimeImpl.h b/velox/functions/prestosql/DateTimeImpl.h index 7a8430623492..0654c865abb0 100644 --- a/velox/functions/prestosql/DateTimeImpl.h +++ b/velox/functions/prestosql/DateTimeImpl.h @@ -204,13 +204,8 @@ FOLLY_ALWAYS_INLINE int64_t addToTimestampWithTimezone( const DateTimeUnit unit, const int32_t value) { auto timestamp = unpackTimestampUtc(timestampWithTimezone); - auto tzID = unpackZoneKeyId(timestampWithTimezone); - timestamp.toTimezone(tzID); - - auto finalTimestamp = addToTimestamp(timestamp, unit, value); - - finalTimestamp.toGMT(tzID); - return pack(finalTimestamp.toMillis(), tzID); + auto finalTimeStamp = addToTimestamp(timestamp, unit, (int32_t)value); + return pack(finalTimeStamp, unpackZoneKeyId(timestampWithTimezone)); } FOLLY_ALWAYS_INLINE int64_t diffTimestamp( diff --git a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp index 0396b9f8fd57..07afeafb234a 100644 --- a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp +++ b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp @@ -1089,8 +1089,17 @@ TEST_F(DateTimeFunctionsTest, timestampWithTimeZonePlusIntervalDayTime) { "2024-10-03 01:51:00.000 America/Los_Angeles", test("2024-10-03 01:50 America/Los_Angeles", 1 * kMillisInMinute)); - // TODO Add tests for transitioning to/from DST once - // https://github.com/facebookincubator/velox/issues/10163 is fixed. + // Testing daylight saving transitions. + + // At the beginning there is a 1 hour gap. + EXPECT_EQ( + "2024-03-10 01:30:00.000 America/Los_Angeles", + test("2024-03-10 03:30 America/Los_Angeles", -1 * kMillisInHour)); + + // At the end there is a 1 hour duplication. + EXPECT_EQ( + "2024-11-03 01:30:00.000 America/Los_Angeles", + test("2024-11-03 01:30 America/Los_Angeles", 1 * kMillisInHour)); } TEST_F(DateTimeFunctionsTest, minusTimestamp) { @@ -2403,6 +2412,31 @@ TEST_F(DateTimeFunctionsTest, dateAddTimestampWithTimeZone) { "year", 3, "1972-05-20+23:01:02+14:00", "1975-05-20+23:01:02+14:00"); evaluateDateAddFromStrings( "year", 3, "1968-02-20+23:01:02+14:00", "1971-02-20+23:01:02+14:00"); + + // Tests date_add() on daylight saving transition boundaries. + // + // Presto's semantic is to apply the delta to GMT, which means that at times + // the observed delta may not be linear, in cases when it hits a daylight + // savings boundary. + auto dateAddAndCast = [&](std::optional unit, + std::optional value, + std::optional timestampString) { + return evaluateOnce( + "cast(date_add(c0, c1, cast(c2 as timestamp with time zone)) as VARCHAR)", + unit, + value, + timestampString); + }; + + EXPECT_EQ( + "2024-03-10 03:50:00.000 America/Los_Angeles", + dateAddAndCast("hour", 1, "2024-03-10 01:50:00 America/Los_Angeles")); + EXPECT_EQ( + "2024-11-03 01:50:00.000 America/Los_Angeles", + dateAddAndCast("hour", 1, "2024-11-03 01:50:00 America/Los_Angeles")); + EXPECT_EQ( + "2024-11-03 00:50:00.000 America/Los_Angeles", + dateAddAndCast("hour", -1, "2024-11-03 01:50:00 America/Los_Angeles")); } TEST_F(DateTimeFunctionsTest, dateDiffDate) { diff --git a/velox/functions/prestosql/types/TimestampWithTimeZoneType.h b/velox/functions/prestosql/types/TimestampWithTimeZoneType.h index bbcc0b2b3431..85f0460c9df0 100644 --- a/velox/functions/prestosql/types/TimestampWithTimeZoneType.h +++ b/velox/functions/prestosql/types/TimestampWithTimeZoneType.h @@ -142,6 +142,10 @@ inline int64_t pack(int64_t millisUtc, int16_t timeZoneKey) { return (millisUtc << kMillisShift) | (timeZoneKey & kTimezoneMask); } +inline int64_t pack(const Timestamp& timestamp, int16_t timeZoneKey) { + return pack(timestamp.toMillis(), timeZoneKey); +} + inline Timestamp unpackTimestampUtc(int64_t dateTimeWithTimeZone) { return Timestamp::fromMillis(unpackMillisUtc(dateTimeWithTimeZone)); }