huan233usc commented on code in PR #17161:
URL: https://github.com/apache/iceberg/pull/17161#discussion_r3993843788
##########
parquet/src/main/java/org/apache/iceberg/data/parquet/BaseParquetWriter.java:
##########
@@ -268,8 +268,11 @@ public Optional<ParquetValueWriter<?>> visit(
@Override
public Optional<ParquetValueWriter<?>> visit(
LogicalTypeAnnotation.GeometryLogicalTypeAnnotation geometryType) {
- // geometry values are pure WKB stored in a BINARY column
- return Optional.of(ParquetValueWriters.geospatial(desc));
+ // geometry values are pure WKB stored in a BINARY column; the writer
also scans the
+ // coordinates to produce a bounding box (the Parquet footer cannot).
The concrete CRS is
+ // immaterial here: ParquetMetrics rewrites the bound's type from the
table schema before
+ // serialization, and the serialization is keyed on the type id, which
ignores the CRS.
+ return Optional.of(ParquetValueWriters.geometry(desc,
Types.GeometryType.crs84()));
Review Comment:
Thanks, agreed that the WKB walk should be avoided when bounds are not
retained. Doing that requires threading the selected `MetricsConfig` into the
value-writer configuration; the generic writer factory currently receives only
the Iceberg schema and Parquet type. I’ll keep this PR focused and handle that
plumbing in a follow-up (also listed in the PR scope).
--
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]
