Port a upstream patch facebookincubator#6204 to fix timezone issue

velox/functions/lib/DateTimeFormatter.cpp

@@ -214,27 +214,6 @@ std::string padContent(
   }
 }
 
-void validateTimePoint(const std::chrono::time_point<
-                       std::chrono::system_clock,
-                       std::chrono::milliseconds>& timePoint) {
-  // Due to the limit of std::chrono we can only represent time in
-  // [-32767-01-01, 32767-12-31] date range
-  const auto minTimePoint = date::sys_days{
-      date::year_month_day(date::year::min(), date::month(1), date::day(1))};
-  const auto maxTimePoint = date::sys_days{
-      date::year_month_day(date::year::max(), date::month(12), date::day(31))};
-  if (timePoint < minTimePoint || timePoint > maxTimePoint) {
-    VELOX_USER_FAIL(
-        "Cannot format time out of range of [{}-{}-{}, {}-{}-{}]",
-        (int)date::year::min(),
-        "01",
-        "01",
-        (int)date::year::max(),
-        "12",
-        "31");
-  }
-}
-
 size_t countOccurence(const std::string_view& base, const std::string& target) {
   int occurrences = 0;
   std::string::size_type pos = 0;
@@ -952,10 +931,11 @@ void parseFromPattern(
 std::string DateTimeFormatter::format(
     const Timestamp& timestamp,
     const date::time_zone* timezone) const {
-  const std::chrono::
-      time_point<std::chrono::system_clock, std::chrono::milliseconds>
-          timePoint(std::chrono::milliseconds(timestamp.toMillis()));
-  validateTimePoint(timePoint);
+  Timestamp t = timestamp;
+  if (timezone != nullptr) {
+    t.toTimezone(*timezone);
+  }
+  const auto timePoint = t.toTimePoint();
   const auto daysTimePoint = date::floor<date::days>(timePoint);
 
   const auto durationInTheDay = date::make_time(timePoint - daysTimePoint);

velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp

@@ -696,7 +696,7 @@ TEST_F(DateTimeFunctionsTest, hour) {
 
   EXPECT_EQ(std::nullopt, hour(std::nullopt));
   EXPECT_EQ(13, hour(Timestamp(0, 0)));
-  EXPECT_EQ(12, hour(Timestamp(-1, Timestamp::kMaxNanos)));
+  EXPECT_EQ(13, hour(Timestamp(-1, Timestamp::kMaxNanos)));
   // Disabled for now because the TZ for Pacific/Apia in 2096 varies between
   // systems.
   // EXPECT_EQ(21, hour(Timestamp(4000000000, 0)));
@@ -2529,12 +2529,12 @@ TEST_F(DateTimeFunctionsTest, formatDateTime) {
 
   // Multi-specifier and literal formats
   EXPECT_EQ(
-      "AD 19 1970 4 Thu 1970 1 1 1 AM 2 2 2 2 33 11 5 Asia/Kolkata",
+      "AD 19 1970 4 Thu 1970 1 1 1 AM 8 8 8 8 3 11 5 Asia/Kolkata",
       formatDatetime(
           fromTimestampString("1970-01-01 02:33:11.5"),
           "G C Y e E y D M d a K h H k m s S zzzz"));
   EXPECT_EQ(
-      "AD 19 1970 4 asdfghjklzxcvbnmqwertyuiop Thu ' 1970 1 1 1 AM 2 2 2 2 33 11 5 1234567890\\\"!@#$%^&*()-+`~{}[];:,./ Asia/Kolkata",
+      "AD 19 1970 4 asdfghjklzxcvbnmqwertyuiop Thu ' 1970 1 1 1 AM 8 8 8 8 3 11 5 1234567890\\\"!@#$%^&*()-+`~{}[];:,./ Asia/Kolkata",
       formatDatetime(
           fromTimestampString("1970-01-01 02:33:11.5"),
           "G C Y e 'asdfghjklzxcvbnmqwertyuiop' E '' y D M d a K h H k m s S 1234567890\\\"!@#$%^&*()-+`~{}[];:,./ zzzz"));
@@ -2787,21 +2787,43 @@ TEST_F(DateTimeFunctionsTest, dateFormat) {
   EXPECT_EQ("z", dateFormat(fromTimestampString("1970-01-01"), "%z"));
   EXPECT_EQ("g", dateFormat(fromTimestampString("1970-01-01"), "%g"));
 
-  // With timezone
+  // With timezone. Indian Standard Time (IST) UTC+5:30.
   setQueryTimeZone("Asia/Kolkata");
+
   EXPECT_EQ(
       "1970-01-01", dateFormat(fromTimestampString("1970-01-01"), "%Y-%m-%d"));
   EXPECT_EQ(
-      "2000-02-29 12:00:00 AM",
+      "2000-02-29 05:30:00 AM",
       dateFormat(
           fromTimestampString("2000-02-29 00:00:00.987"), "%Y-%m-%d %r"));
   EXPECT_EQ(
-      "2000-02-29 00:00:00.987000",
+      "2000-02-29 05:30:00.987000",
       dateFormat(
           fromTimestampString("2000-02-29 00:00:00.987"),
           "%Y-%m-%d %H:%i:%s.%f"));
   EXPECT_EQ(
-      "-2000-02-29 00:00:00.987000",
+      "-2000-02-29 05:53:29.987000",
+      dateFormat(
+          fromTimestampString("-2000-02-29 00:00:00.987"),
+          "%Y-%m-%d %H:%i:%s.%f"));
+
+  // Same timestamps with a different timezone. Pacific Daylight Time (North
+  // America) PDT UTC-8:00.
+  setQueryTimeZone("America/Los_Angeles");
+
+  EXPECT_EQ(
+      "1969-12-31", dateFormat(fromTimestampString("1970-01-01"), "%Y-%m-%d"));
+  EXPECT_EQ(
+      "2000-02-28 04:00:00 PM",
+      dateFormat(
+          fromTimestampString("2000-02-29 00:00:00.987"), "%Y-%m-%d %r"));
+  EXPECT_EQ(
+      "2000-02-28 16:00:00.987000",
+      dateFormat(
+          fromTimestampString("2000-02-29 00:00:00.987"),
+          "%Y-%m-%d %H:%i:%s.%f"));
+  EXPECT_EQ(
+      "-2000-02-28 16:07:03.987000",
       dateFormat(
           fromTimestampString("-2000-02-29 00:00:00.987"),
           "%Y-%m-%d %H:%i:%s.%f"));

velox/type/Timestamp.cpp

@@ -69,9 +69,40 @@ void Timestamp::toGMT(int16_t tzID) {
   }
 }
 
+namespace {
+void validateTimePoint(const std::chrono::time_point<
+                       std::chrono::system_clock,
+                       std::chrono::milliseconds>& timePoint) {
+  // Due to the limit of std::chrono we can only represent time in
+  // [-32767-01-01, 32767-12-31] date range
+  const auto minTimePoint = date::sys_days{
+      date::year_month_day(date::year::min(), date::month(1), date::day(1))};
+  const auto maxTimePoint = date::sys_days{
+      date::year_month_day(date::year::max(), date::month(12), date::day(31))};
+  if (timePoint < minTimePoint || timePoint > maxTimePoint) {
+    VELOX_USER_FAIL(
+        "Timestamp is outside of supported range of [{}-{}-{}, {}-{}-{}]",
+        (int)date::year::min(),
+        "01",
+        "01",
+        (int)date::year::max(),
+        "12",
+        "31");
+  }
+}
+} // namespace
+
+std::chrono::time_point<std::chrono::system_clock, std::chrono::milliseconds>
+Timestamp::toTimePoint() const {
+  auto tp = std::chrono::
+      time_point<std::chrono::system_clock, std::chrono::milliseconds>(
+          std::chrono::milliseconds(toMillis()));
+  validateTimePoint(tp);
+  return tp;
+}
+
 void Timestamp::toTimezone(const date::time_zone& zone) {
-  auto tp = std::chrono::time_point<std::chrono::system_clock>(
-      std::chrono::seconds(seconds_));
+  auto tp = toTimePoint();
   auto epoch = zone.to_local(tp).time_since_epoch();
   seconds_ = std::chrono::duration_cast<std::chrono::seconds>(epoch).count();
 }

velox/type/Timestamp.h

@@ -117,6 +117,11 @@ struct Timestamp {
     }
   }
 
+  /// Due to the limit of std::chrono, throws if timestamp is outside of
+  /// [-32767-01-01, 32767-12-31] range.
+  std::chrono::time_point<std::chrono::system_clock, std::chrono::milliseconds>
+  toTimePoint() const;
+
   static Timestamp fromMillis(int64_t millis) {
     if (millis >= 0 || millis % 1'000 == 0) {
       return Timestamp(millis / 1'000, (millis % 1'000) * 1'000'000);

velox/type/tests/TimestampTest.cpp

@@ -17,6 +17,7 @@
 #include <gtest/gtest.h>
 
 #include "velox/common/base/tests/GTestUtils.h"
+#include "velox/external/date/tz.h"
 #include "velox/type/Timestamp.h"
 
 namespace facebook::velox {
@@ -158,5 +159,47 @@ TEST(TimestampTest, toString) {
   EXPECT_EQ("-292275055-05-16T16:47:04.000000000", kMin.toString());
   EXPECT_EQ("292278994-08-17T07:12:55.999999999", kMax.toString());
 }
+
+TEST(TimestampTest, increaseOperator) {
+  auto ts = Timestamp(0, 999999998);
+  EXPECT_EQ("1970-01-01T00:00:00.999999998", ts.toString());
+  ++ts;
+  EXPECT_EQ("1970-01-01T00:00:00.999999999", ts.toString());
+  ++ts;
+  EXPECT_EQ("1970-01-01T00:00:01.000000000", ts.toString());
+  ++ts;
+  EXPECT_EQ("1970-01-01T00:00:01.000000001", ts.toString());
+  ++ts;
+  EXPECT_EQ("1970-01-01T00:00:01.000000002", ts.toString());
+
+  auto kMax = Timestamp(Timestamp::kMaxSeconds, Timestamp::kMaxNanos);
+  VELOX_ASSERT_THROW(++kMax, "Timestamp nanos out of range");
+}
+
+TEST(TimestampTest, decreaseOperator) {
+  auto ts = Timestamp(0, 2);
+  EXPECT_EQ("1970-01-01T00:00:00.000000002", ts.toString());
+  --ts;
+  EXPECT_EQ("1970-01-01T00:00:00.000000001", ts.toString());
+  --ts;
+  EXPECT_EQ("1970-01-01T00:00:00.000000000", ts.toString());
+  --ts;
+  EXPECT_EQ("1969-12-31T23:59:59.999999999", ts.toString());
+  --ts;
+  EXPECT_EQ("1969-12-31T23:59:59.999999998", ts.toString());
+
+  auto kMin = Timestamp(Timestamp::kMinSeconds, 0);
+  VELOX_ASSERT_THROW(--kMin, "Timestamp nanos out of range");
+}
+
+TEST(TimestampTest, outOfRange) {
+  auto* timezone = date::locate_zone("GMT");
+  Timestamp t(-3217830796800, 0);
+
+  VELOX_ASSERT_THROW(
+      t.toTimePoint(), "Timestamp is outside of supported range");
+  VELOX_ASSERT_THROW(
+      t.toTimezone(*timezone), "Timestamp is outside of supported range");
+}
 } // namespace
 } // namespace facebook::velox