Fix toMillis should work for all presto timestamps (facebookincubator…

…#7506)

Summary:
Pull Request resolved: facebookincubator#7506

Presto timestamps are in the range TimeStamp::minMillis() to TimeStamp::maxMillis().
moreover minMillis has a comment:
```
// The minimum Timestamp that toMillis() method will not overflow.
// Used to calculate the minimum value of the Presto timestamp.
```
Before this diff  TimeStamp::minMillis().toMillis throws because the computation
overflows. However in that case we multiply negative number then add positive number
so the final result still fits in int64_t.
It is important the toMillis does not throw when it should not, since it is used in
functions like inPredicate() and other functions that do not expect it to throw
for some values. Throwing will show up as incorrect results compared to presto.
This diff fixes that .
I wonder if we shall throw a runtime error in toMillis() instead.

found while investigating facebookincubator#7161

Reviewed By: kagamiori

Differential Revision: D51177715

fbshipit-source-id: 5ec2d4d9c5448f7951a795ffb7a8c5c36fdee15f

velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp

@@ -858,7 +858,7 @@ TEST_F(DateTimeFunctionsTest, minusTimestampIntervalDayTime) {
   EXPECT_EQ(-1000, minus(1, 2));
   VELOX_ASSERT_THROW(
       minus(Timestamp::kMinSeconds, Timestamp::kMaxSeconds),
-      "integer overflow");
+      "Could not convert Timestamp(-9223372036854776, 0) to milliseconds");
 }
 
 TEST_F(DateTimeFunctionsTest, dayOfMonthTimestampWithTimezone) {

velox/type/Timestamp.h

@@ -120,19 +120,21 @@ struct Timestamp {
   }
 
   int64_t toMillis() const {
-    // When an integer overflow occurs in the calculation,
-    // an exception will be thrown.
-    try {
-      return checkedPlus(
-          checkedMultiply(seconds_, (int64_t)1'000),
-          (int64_t)(nanos_ / 1'000'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*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.
+    __int128_t result =
+        (__int128_t)seconds_ * 1'000 + (int64_t)(nanos_ / 1'000'000);
+    if (result < std::numeric_limits<int64_t>::min() ||
+        result > std::numeric_limits<int64_t>::max()) {
       VELOX_USER_FAIL(
-          "Could not convert Timestamp({}, {}) to milliseconds, {}",
+          "Could not convert Timestamp({}, {}) to milliseconds",
           seconds_,
-          nanos_,
-          e.what());
+          nanos_);
     }
+    return result;
   }
 
   int64_t toMicros() const {

velox/type/tests/TimestampTest.cpp

@@ -116,6 +116,8 @@ TEST(TimestampTest, arithmeticOverflow) {
           "Could not convert Timestamp({}, {}) to nanoseconds",
           negativeSecond,
           0));
+  ASSERT_NO_THROW(Timestamp::minMillis().toMillis());
+  ASSERT_NO_THROW(Timestamp::maxMillis().toMillis());
 }
 
 TEST(TimestampTest, toAppend) {