rdblue commented on code in PR #17560:
URL: https://github.com/apache/iceberg/pull/17560#discussion_r3786491593
##########
data/src/test/java/org/apache/iceberg/parquet/TestParquetMetrics.java:
##########
@@ -107,4 +123,96 @@ public int splitCount(InputFile inputFile) throws
IOException {
public boolean supportsSmallRowGroups() {
return true;
}
+
+ @TestTemplate
+ public void testMetricsForNullStructWithFloatingAndGeoLeaves() throws
IOException {
+ // float, double, geometry and geography are the only types whose writers
report metrics, so
+ // they are the ones whose nested null counts could be dropped when a
struct is null. Null
+ // counts are only tracked for optional fields.
+ StructType struct =
+ StructType.of(
+ optional(2, "optDouble", DoubleType.get()),
+ optional(3, "optFloat", FloatType.get()),
+ optional(4, "geom", GeometryType.crs84()),
+ optional(5, "optLong", LongType.get()));
+ Schema schema = new Schema(optional(1, "struct", struct));
+
+ Record inner = GenericRecord.create(struct);
+ inner.setField("optDouble", 1.5D);
+ inner.setField("optFloat", 2.5F);
+ inner.setField("geom", wkbPoint(30, 10));
+ inner.setField("optLong", 10L);
+ Record withStruct = GenericRecord.create(schema);
+ withStruct.setField("struct", inner);
+ Record nullStruct = GenericRecord.create(schema);
+ nullStruct.setField("struct", null);
+
+ Metrics metrics = getMetrics(schema, withStruct, nullStruct, nullStruct);
+
+ assertThat(metrics.recordCount()).isEqualTo(3L);
+ // each leaf has one value from the populated struct and two nulls from
the null structs
+ assertCounts(2, 3L, 2L, 0L, metrics);
+ assertCounts(3, 3L, 2L, 0L, metrics);
+ assertCounts(4, 3L, 2L, metrics);
+ // a type without writer metrics was already correct via the footer;
included as a control
+ assertCounts(5, 3L, 2L, metrics);
+
+ // the counts also reach a data file built from these metrics
+ DataFile dataFile =
+ DataFiles.builder(PartitionSpec.unpartitioned())
+ .withPath("/path/to/file.parquet")
+ .withFileSizeInBytes(1024)
+ .withFormat(FileFormat.PARQUET)
+ .withMetrics(metrics)
+ .build();
+ assertThat(dataFile.nullValueCounts()).containsEntry(2,
2L).containsEntry(3, 2L);
+ }
+
+ @TestTemplate
+ public void testMetricsForRequiredNestedFieldInNullStruct() throws
IOException {
Review Comment:
These tests duplicate a lot of what is tested in `TestParquetValueWriters`.
I think the responsibility of this class is to validate the conversion of
`FieldMetrics` to Parquet metrics, not to validate by type. I think one nested
test is sufficient.
--
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]
