diff --git a/velox/type/Timestamp.h b/velox/type/Timestamp.h index a342867e0d54..b676ad40f961 100644 --- a/velox/type/Timestamp.h +++ b/velox/type/Timestamp.h @@ -158,13 +158,12 @@ struct Timestamp { int64_t toMillis() const { // 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(). + // 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::min() || - result > std::numeric_limits::max()) { + if (result < INT64_MIN || result > INT64_MAX) { VELOX_USER_FAIL( "Could not convert Timestamp({}, {}) to milliseconds", seconds_, @@ -183,19 +182,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 a negative seconds*1000000 does not fit in + // int64_t, but seconds*1000000 + nanos does. An example is + // Timestamp(-9223372036855, 224'192'000). + + // If the final result does not fit in int64_t we throw. + __int128_t result = static_cast<__int128_t>(seconds_) * 1'000'000 + + static_cast(nanos_ / 1'000); + if (result < INT64_MIN || result > INT64_MAX) { VELOX_USER_FAIL( - "Could not convert Timestamp({}, {}) to microseconds, {}", + "Could not convert Timestamp({}, {}) to microseconds", seconds_, - nanos_, - e.what()); + nanos_); } + return result; } Timestamp toPrecision(const TimestampPrecision& precision) const { diff --git a/velox/type/tests/TimestampTest.cpp b/velox/type/tests/TimestampTest.cpp index 9e344cf99934..f5cfdc9b7f0e 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(-9223372036855, 224'192'000).toMicros()); + ASSERT_NO_THROW(Timestamp(9223372036854, 775'807'000).toMicros()); } TEST(TimestampTest, toAppend) {