address comment

facebookincubator · Dec 12, 2024 · 86b1e46 · 86b1e46
1 parent 09a1ee4
commit 86b1e46
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 5 deletions.
diff --git a/velox/type/Timestamp.h b/velox/type/Timestamp.h
@@ -158,7 +158,7 @@ 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_t we throw.
     __int128_t result =
@@ -183,8 +183,9 @@ struct Timestamp {
   // Keep it in header for getting inlined.
   int64_t toMicros() const {
     // 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().
+    // 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 =

diff --git a/velox/type/tests/TimestampTest.cpp b/velox/type/tests/TimestampTest.cpp
@@ -154,8 +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());
+  ASSERT_NO_THROW(Timestamp(-9223372036855, 224'192'000).toMicros());
+  ASSERT_NO_THROW(Timestamp(9223372036854, 775'807'000).toMicros());
 }
 
 TEST(TimestampTest, toAppend) {