rdblue commented on code in PR #3610:
URL: https://github.com/apache/parquet-java/pull/3610#discussion_r3562464778
##########
parquet-hadoop/src/test/java/org/apache/parquet/format/converter/TestParquetMetadataConverter.java:
##########
@@ -1171,17 +1171,14 @@ public void testMissingValuesFromStats() {
@Test
public void testSkippedV2Stats() {
+ // INTERVAL has an undefined column order, so its stats are skipped.
testSkippedV2Stats(
Types.optional(PrimitiveTypeName.FIXED_LEN_BYTE_ARRAY)
.length(12)
.as(OriginalType.INTERVAL)
.named(""),
new BigInteger("12345678"),
new BigInteger("12345679"));
- testSkippedV2Stats(
- Types.optional(PrimitiveTypeName.INT96).named(""),
- new BigInteger("-75687987"),
- new BigInteger("45367657"));
Review Comment:
There are other tests that also need to be updated. For example,
`testV2StatsEqualMinMax` tests cases where the min and max are equal and stats
are preserved. This also uses `BigInteger` to pass the values, which is
suspicious.
I think this should update all of the INT96 cases to use `NanoTime`. Looking
at the code path, I don't think that NanoTime is plumbed to work since I don't
see a `Statistics` implementation for it. Maybe this should actually pass a
Binary produced by NanoTime instead.
--
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]
