paleolimbot commented on code in PR #3342:
URL: https://github.com/apache/iceberg-rust/pull/3342#discussion_r4414965795
##########
crates/iceberg/src/writer/file_writer/parquet_writer.rs:
##########
@@ -371,6 +371,12 @@ impl MinMaxColAggregator {
));
};
+ if matches!(ty, PrimitiveType::Geometry(_) |
PrimitiveType::Geography(_)) {
+ // Statistics are optional, not setting them is fine.
+ // TODO: Implement bounds for geospatial types.
+ return Ok(());
Review Comment:
This is a very big TODO...geometry and geography written without statistics
is a bit of a regression (particularly if other bits of the table have them).
The Parquet stats handling is pretty well defined in the parquet crate, this
is possibly easy to do and is the same for Geometry and Geography. (Statistics
aren't written by default for Geography, but this can be plugged in at runtime
like we do in SedonaDB). Given that this PR is already very small, I think
adding here may be a good idea while we're all looking at it.
--
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]
