divjotarora commented on code in PR #3680:
URL: https://github.com/apache/parquet-java/pull/3680#discussion_r3689876260
##########
parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveStringifier.java:
##########
@@ -266,6 +267,41 @@ Instant getInstant(long value) {
}
}
+ /**
+ * Stringifier implementation for timestamps that handles both int64 and
FLBA(12) carriers.
+ * Values outside of Instant's supported range render as raw integers rather
than human-readable
+ * timestamps.
+ */
+ private abstract static class TimestampStringifier extends DateStringifier {
+ private TimestampStringifier(String name, String format) {
+ super(name, format);
+ }
+
+ @Override
+ public String stringify(Binary value) {
+ if (value == null) {
+ return BINARY_NULL;
+ }
+ byte[] littleEndian = value.getBytesUnsafe();
+ byte[] bigEndian = new byte[littleEndian.length];
+ for (int i = 0; i < littleEndian.length; ++i) {
+ bigEndian[i] = littleEndian[littleEndian.length - 1 - i];
+ }
+ try {
+ BigInteger units = new BigInteger(bigEndian);
+ try {
+ return toFormattedString(getInstant(units));
+ } catch (ArithmeticException | DateTimeException e) {
+ return units.toString();
+ }
+ } catch (NumberFormatException e) {
+ return BINARY_INVALID;
Review Comment:
The `BigInteger` ctor throws if the array is empty, but that's not possible
as it's guaranteed to be size 12 higher up. Removed the try/catch
##########
parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveStringifier.java:
##########
@@ -274,56 +310,96 @@ Instant getInstant(int value) {
;
};
+ // Converts a count of time units since epoch, held as a BigInteger (the
96-bit FLBA(12) carrier),
+ // into an Instant. The 96-bit count does not fit in a long, but the
whole-second count does, so
Review Comment:
Good catch, changed the comment to
> Converts a count of time units since epoch, held as a BigInteger (the
96-bit FLBA(12) carrier),
into an Instant. Throws ArithmeticException if the sections portion
overflows long.
##########
parquet-column/src/test/java/org/apache/parquet/internal/column/columnindex/TestBinaryTruncator.java:
##########
@@ -91,6 +93,17 @@ public void testContractNonStringTypes() {
testTruncator(Types.required(INT96).named("test_int96"), false);
}
+ @Test
+ public void testFlba12Timestamp() {
+ BinaryTruncator truncator =
BinaryTruncator.getTruncator(Types.required(FIXED_LEN_BYTE_ARRAY)
+ .length(12)
+ .as(LogicalTypeAnnotation.timestampType(true, TimeUnit.NANOS))
+ .named("test_fixed_timestamp"));
+ Binary value = Binary.fromConstantByteArray(new byte[] {0, 0, 0, 0, 0, 0,
0, 0, 1, 2, 3, 4});
Review Comment:
No specific reason. I changed it to `bytes[i] = i` to make it look less
intentional.
##########
parquet-column/src/test/java/org/apache/parquet/schema/TestPrimitiveStringifier.java:
##########
@@ -417,6 +446,18 @@ private Binary toBinary(int... bytes) {
return Binary.fromConstantByteArray(array);
}
+ // Encodes a unit count as the extended-precision timestamp carrier: a
12-byte signed
Review Comment:
Added an override that directly accepts a `BigInteger` + tests that use it
for values larger than the int64 range.
##########
parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveStringifier.java:
##########
@@ -266,6 +267,41 @@ Instant getInstant(long value) {
}
}
+ /**
+ * Stringifier implementation for timestamps that handles both int64 and
FLBA(12) carriers.
+ * Values outside of Instant's supported range render as raw integers rather
than human-readable
+ * timestamps.
+ */
+ private abstract static class TimestampStringifier extends DateStringifier {
+ private TimestampStringifier(String name, String format) {
+ super(name, format);
+ }
+
+ @Override
+ public String stringify(Binary value) {
+ if (value == null) {
+ return BINARY_NULL;
+ }
+ byte[] littleEndian = value.getBytesUnsafe();
Review Comment:
Added a general `reverse()` function to `BytesUtils`, this can be used for
bidirectional LE <--> BE swaps, but let me know if you'd prefer `toBigEndian`
specifically.
##########
parquet-column/src/test/java/org/apache/parquet/schema/TestPrimitiveStringifier.java:
##########
@@ -417,6 +446,18 @@ private Binary toBinary(int... bytes) {
return Binary.fromConstantByteArray(array);
}
+ // Encodes a unit count as the extended-precision timestamp carrier: a
12-byte signed
+ // two's-complement little-endian value (sign-extended, so negatives fill
the high bytes with
+ // 0xFF).
+ private Binary flba12(long units) {
+ byte[] le = new byte[12];
Review Comment:
It's about the same amount of code, but probably conceptually easier to
read. Done.
--
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]
