elphastori commented on code in PR #18272:
URL: https://github.com/apache/iceberg/pull/18272#discussion_r4383715908


##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetUtil.java:
##########
@@ -173,15 +174,35 @@ public static boolean isIntType(PrimitiveType 
primitiveType) {
   }
 
   /**
-   * Method to read timestamp (parquet Int96) from bytebuffer. Read 12 bytes 
in byteBuffer: 8 bytes
-   * (time of day nanos) + 4 bytes(julianDay)
+   * Reads 12 INT96 bytes in the buffer's byte order as microseconds since the 
Unix epoch.
+   *
+   * <p>Submicrosecond precision is truncated.
+   *
+   * @throws ArithmeticException if the timestamp overflows a long
    */
   public static long extractTimestampInt96(ByteBuffer buffer) {
-    // 8 bytes (time of day nanos)
     long timeOfDayNanos = buffer.getLong();
-    // 4 bytes(julianDay)
     int julianDay = buffer.getInt();
-    return TimeUnit.DAYS.toMicros(julianDay - UNIX_EPOCH_JULIAN)
-        + TimeUnit.NANOSECONDS.toMicros(timeOfDayNanos);
+    return timestampInt96(julianDay, timeOfDayNanos / 1000, MICROS_PER_DAY);
+  }
+
+  /**
+   * Reads 12 INT96 bytes in the buffer's byte order as nanoseconds since the 
Unix epoch.
+   *
+   * @throws ArithmeticException if the timestamp overflows a long
+   */
+  public static long extractTimestampInt96Nanos(ByteBuffer buffer) {
+    long timeOfDayNanos = buffer.getLong();
+    int julianDay = buffer.getInt();
+    return timestampInt96(julianDay, timeOfDayNanos, NANOS_PER_DAY);
+  }
+
+  private static long timestampInt96(int julianDay, long timeOfDay, long 
ticksPerDay) {

Review Comment:
   Thanks for working on this.
   
   A non-blocking suggestion: An out-of-range value now fails with 
`ArithmeticException: long overflow` which doesn't say which column or value 
caused it. The error can also look like an Iceberg bug rather than a value the 
column type can’t hold.
   
   Could `extractTimestampInt96` and `extractTimestampInt96Nanos` catch the 
exception and rethrow one that names the value and the unit? For example: 
`INT96 timestamp 0001-01-01T00:00:00Z is out of range for nanoseconds (long 
overflow)`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to