Fix date_add() semantic on daylight savings boundaries (#10188)

Summary:
Pull Request resolved: #10188

date_add() used to apply the timestamp deltas based on the local
timestamp, which makes the conversion linear, but does not follow Presto's
semantic. Changing it to follow Presto semantic. Applying the same
rules to other timestamp and interval arithmetic functions.

More context on: #10163

Reviewed By: kagamiori, mbasmanova

Differential Revision: D58479195

fbshipit-source-id: 0509df1fd7836e92e2b4492a5956ac4b24a71a1b

velox/functions/prestosql/DateTimeFunctions.h

@@ -1118,7 +1118,7 @@ struct DateTruncFunction : public TimestampWithTimezoneSupport<T> {
         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<T> {
       const arg_type<Varchar>& unitString,
       const int64_t value,
       const arg_type<TimestampWithTimezone>& 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();
   }
 };

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(

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<std::string> unit,
+                            std::optional<int32_t> value,
+                            std::optional<std::string> timestampString) {
+    return evaluateOnce<std::string>(
+        "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) {

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));
 }