From 966f563b83cfc3e402188c0a005f8947fdc6feef Mon Sep 17 00:00:00 2001
From: rexan <rexan@apache.org>
Date: Fri, 6 Dec 2024 05:53:05 -0500
Subject: [PATCH] Fix toMicros() should work for all spark timestamps

---
 velox/type/Timestamp.h             | 24 +++++++++++++-----------
 velox/type/tests/TimestampTest.cpp |  2 ++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/velox/type/Timestamp.h b/velox/type/Timestamp.h
index 88a31e72f62e7..7d38d43b83971 100644
--- a/velox/type/Timestamp.h
+++ b/velox/type/Timestamp.h
@@ -160,7 +160,7 @@ struct Timestamp {
     // there are cases such that seconds*1000 does not fit in int64_t,
     // but seconds*1000 + nanos does, an example is TimeStamp::minMillis().
 
-    // If the final result does not fit in int64_tw we throw.
+    // If the final result does not fit in int64_t we throw.
     __int128_t result =
         (__int128_t)seconds_ * 1'000 + (int64_t)(nanos_ / 1'000'000);
     if (result < std::numeric_limits<int64_t>::min() ||
@@ -183,19 +183,21 @@ struct Timestamp {
 
   // Keep it in header for getting inlined.
   int64_t toMicros() const {
-    // When an integer overflow occurs in the calculation,
-    // an exception will be thrown.
-    try {
-      return checkedPlus(
-          checkedMultiply(seconds_, (int64_t)1'000'000),
-          (int64_t)(nanos_ / 1'000));
-    } catch (const std::exception& e) {
+    // We use int128_t to make sure the computation does not overflows since
+    // there are cases such that seconds*1000000 does not fit in int64_t,
+    // but seconds*1000000 + nanos does, an example is TimeStamp::minMillis().
+
+    // If the final result does not fit in int64_t we throw.
+    __int128_t result =
+        (__int128_t)seconds_ * 1'000'000 + (int64_t)(nanos_ / 1'000);
+    if (result < std::numeric_limits<int64_t>::min() ||
+        result > std::numeric_limits<int64_t>::max()) {
       VELOX_USER_FAIL(
-          "Could not convert Timestamp({}, {}) to microseconds, {}",
+          "Could not convert Timestamp({}, {}) to microseconds",
           seconds_,
-          nanos_,
-          e.what());
+          nanos_);
     }
+    return result;
   }
 
   /// Exports the current timestamp as a std::chrono::time_point of millisecond
diff --git a/velox/type/tests/TimestampTest.cpp b/velox/type/tests/TimestampTest.cpp
index 9e344cf999349..7645479c7235f 100644
--- a/velox/type/tests/TimestampTest.cpp
+++ b/velox/type/tests/TimestampTest.cpp
@@ -154,6 +154,8 @@ TEST(TimestampTest, arithmeticOverflow) {
           0));
   ASSERT_NO_THROW(Timestamp::minMillis().toMillis());
   ASSERT_NO_THROW(Timestamp::maxMillis().toMillis());
+  ASSERT_NO_THROW(Timestamp::minMillis().toMicros());
+  ASSERT_NO_THROW(Timestamp::maxMillis().toMicros());
 }
 
 TEST(TimestampTest, toAppend) {