szehon-ho commented on code in PR #17161:
URL: https://github.com/apache/iceberg/pull/17161#discussion_r3899700011
##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetValueWriters.java:
##########
@@ -362,6 +368,30 @@ public Stream<FieldMetrics<?>> metrics() {
}
}
+ private static class GeometryWriter extends PrimitiveWriter<ByteBuffer> {
+ private final GeometryFieldMetrics.Builder metricsBuilder;
+
+ private GeometryWriter(ColumnDescriptor desc,
org.apache.iceberg.types.Type geometryType) {
+ super(desc);
+ this.metricsBuilder =
+ new GeometryFieldMetrics.Builder(
+ desc.getPrimitiveType().getId().intValue(), geometryType);
+ }
+
+ @Override
+ public void write(int repetitionLevel, ByteBuffer buffer) {
+ // Accumulate the bounding box before writing, so it reads the buffer's
coordinates while the
+ // position is intact (the scanner reads a duplicate and leaves this
buffer untouched).
+ metricsBuilder.addValue(buffer);
Review Comment:
Please preserve the existing average WKB size metric while adding bounds.
#17333 already made `GeospatialWriter` populate `avg_value_size_in_bytes`, but
this writer replaces it with a builder that never records or returns the size,
so generic Parquet geometry writes lose that metric. Could
`GeometryFieldMetrics.Builder` accumulate the WKB size and carry the average
into the returned `FieldMetrics`, and keep the existing `TestMetrics` assertion?
--
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]
