wirybeaver commented on code in PR #3342:
URL: https://github.com/apache/iceberg-rust/pull/3342#discussion_r4419926351


##########
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:
   Updated in 8327373c. The TODO is implemented using Parquet geospatial 
statistics, including geography bounds supplied by a custom accumulator. I 
simplified aggregation to a single column-chunk entry point and retained 
conservative handling of missing row-group bounds. The review also caught a 
related correctness issue: spatial bounds could make scalar metrics evaluators 
prune files or incorrectly claim all rows matched. Both evaluators now ignore 
spatial bounds for scalar comparisons, with a regression check in the 
writer/manifest/read-back test. The srid:0 Arrow metadata correction is 
included as well.



-- 
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]

Reply via email to