xiangfu0 commented on code in PR #20060:
URL: https://github.com/apache/pinot/pull/20060#discussion_r4384443743
##########
pinot-plugins/pinot-input-format/pinot-parquet/src/main/java/org/apache/pinot/plugin/inputformat/parquet/ParquetNativeRecordExtractor.java:
##########
@@ -149,6 +148,11 @@ private Object extractValue(Group from, int fieldIndex,
Type fieldType, int inde
return intValue;
case INT64:
long longValue = from.getLong(fieldIndex, index);
+ if (logicalTypeAnnotation instanceof
LogicalTypeAnnotation.IntLogicalTypeAnnotation integerType
Review Comment:
Added the behavior note to the PR description: native ingestion now errors
on files containing UINT64 values above Long.MAX_VALUE instead of storing
wrapped negative values. Representable values keep their existing types.
##########
pinot-plugins/pinot-input-format/pinot-parquet/src/main/java/org/apache/pinot/plugin/inputformat/parquet/ParquetUtils.java:
##########
@@ -159,14 +159,37 @@ public static Timestamp convertLongToTimestamp(long
value, LogicalTypeAnnotation
}
}
- /// Converts a 12-byte INT96 timestamp to `Long` epoch nanos. INT96 stores
the timestamp split as bytes
- /// 0..7 (nanos within the day, long, little-endian) and bytes 8..11 (Julian
day number, int, little-endian).
- /// Nanos is INT96's natural unit since this physical encoding carries
nanosecond precision; pair with
- /// [TimestampUtils#fromNanosSinceEpoch] for a [Timestamp].
+ /// Converts a 12-byte INT96 timestamp to [Timestamp] without narrowing it
to signed epoch nanoseconds.
+ /// INT96 stores bytes 0..7 as nanoseconds within the day and bytes 8..11 as
the Julian day, little-endian.
+ /// This also preserves instants outside the approximately 1677–2262 range
of signed epoch nanoseconds.
+ public static Timestamp convertInt96ToTimestamp(byte[] int96Bytes) {
+ ByteBuffer buffer =
ByteBuffer.wrap(int96Bytes).order(ByteOrder.LITTLE_ENDIAN);
+ long days = buffer.getInt(8) - JULIAN_DAY_NUMBER_FOR_UNIX_EPOCH;
+ long nanosOfDay = buffer.getLong(0);
+ long epochSeconds = days * 86_400L + Math.floorDiv(nanosOfDay,
1_000_000_000L);
+ Timestamp timestamp = new Timestamp(epochSeconds * 1_000L);
+ timestamp.setNanos((int) Math.floorMod(nanosOfDay, 1_000_000_000L));
+ return timestamp;
+ }
+
+ /// Converts a 12-byte INT96 timestamp to signed `Long` epoch nanoseconds.
+ ///
+ /// @throws IllegalArgumentException if the exact value does not fit in a
signed `long`; logical extraction
+ /// should use [#convertInt96ToTimestamp(byte\[\])] instead
public static long convertInt96ToEpochNanos(byte[] int96Bytes) {
- ByteBuffer buf =
ByteBuffer.wrap(int96Bytes).order(ByteOrder.LITTLE_ENDIAN);
- long days = buf.getInt(8) - JULIAN_DAY_NUMBER_FOR_UNIX_EPOCH;
- long nanosOfDay = buf.getLong(0);
- return days * NANOS_PER_DAY + nanosOfDay;
+ ByteBuffer buffer =
ByteBuffer.wrap(int96Bytes).order(ByteOrder.LITTLE_ENDIAN);
+ long days = buffer.getInt(8) - JULIAN_DAY_NUMBER_FOR_UNIX_EPOCH;
+ long nanosOfDay = buffer.getLong(0);
+ // Evaluate the complete sum in 128 bits. At Long.MIN_VALUE, the day
product alone underflows, but adding
+ // the positive day fraction brings the sum back into range, so
multiplyExact followed by addExact is wrong.
+ long product = days * NANOS_PER_DAY;
+ long nanos = product + nanosOfDay;
+ long carry = Long.compareUnsigned(nanos, product) < 0 ? 1 : 0;
+ long high = Math.multiplyHigh(days, NANOS_PER_DAY) + (nanosOfDay >> 63) +
carry;
+ if (high != (nanos >> 63)) {
+ throw new IllegalArgumentException("INT96 timestamp " +
convertInt96ToTimestamp(int96Bytes).toInstant()
+ + " is outside the signed epoch-nanosecond interval and cannot be
returned as raw epoch nanoseconds");
+ }
+ return nanos;
}
}
Review Comment:
Added the inline explanation in e657ad301671c3de935189d077bebbe59e39d7f5:
unsigned low-word overflow supplies the carry, while nanosOfDay >> 63
sign-extends the addend into the high word. Formatting, checkstyle and license
checks pass; the change is comment-only.
--
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]
