Skip to content

Conversation

@Kontinuation
Copy link
Member

This patch pushes all spatial predicates when reading GeoParquet files, including distance predicate and all spatial relation operators such as ST_Contains, ST_Within, etc. To implement this, we have added the following methods to BoundingBox:

  • contains: Check for bounding box containment
  • expand_by: Expand the bounding box by the specified distance

The interval types also have corresponding functions implemented to work with 1-D plain and wrapping-around intervals.

Currently GeoParquet predicate pushdown only work with planar geometries correctly. This limitation already exists prior to developing this patch. I suggest that we submit another PR to disable spatial predicate pushdown for GeoParquet files containing spherical geography objects.

@Kontinuation Kontinuation requested a review from Copilot September 9, 2025 13:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This patch enhances GeoParquet predicate pushdown capabilities by adding support for all spatial predicates including distance predicates and spatial relation operators. The key improvements include expanding bounding box functionality with containment checking and distance-based expansion, implementing spatial predicate parsing utilities, and extending the spatial filter system to handle additional spatial operations.

  • Adds bounding box containment and expansion methods to support spatial predicate evaluation
  • Implements comprehensive spatial predicate pushdown for functions like ST_Contains, ST_Within, ST_DWithin, etc.
  • Introduces utility functions for parsing distance predicates from physical expressions

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
rust/sedona-geometry/src/interval.rs Adds containment checking and expansion methods to interval types
rust/sedona-geometry/src/bounding_box.rs Implements contains and expand_by methods for bounding box operations
rust/sedona-expr/src/utils.rs New utility module for parsing distance predicates from expressions
rust/sedona-expr/src/spatial_filter.rs Extends spatial filter support to handle all spatial relation operators and distance predicates
rust/sedona-spatial-join/src/optimizer.rs Refactors distance predicate matching to use new utility functions
rust/sedona-spatial-join/src/operand_evaluator.rs Adds type casting for distance values
rust/sedona-geoparquet/src/format.rs Fixes typo and updates test cases for expanded predicate support
rust/sedona-geoparquet/Cargo.toml Adds rstest dependency for parameterized tests
rust/sedona-expr/src/lib.rs Exports new utils module
rust/sedona-expr/Cargo.toml Adds rstest dependency for parameterized tests

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Kontinuation Kontinuation marked this pull request as ready for review September 9, 2025 13:53
Copy link
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! (Particularly for handling the wraparound case 😬 )

@paleolimbot
Copy link
Member

I suggest that we submit another PR to disable spatial predicate pushdown for GeoParquet files containing spherical geography objects.

Agreed. This requires porting spherical bounding to Rust (which we probably need anyway for Rust Parquet to properly support geography), or injecting it from the session somehow to avoid depending on s2 C++.

@jiayuasu jiayuasu merged commit af678b1 into apache:main Sep 9, 2025
5 checks passed
Kontinuation added a commit that referenced this pull request Sep 12, 2025
…phy types (#57)

As mentioned in #39, our GeoParquet data pruning optimization and spatial join optimization does not handle geography types correctly, so we'd better disable them to avoid generating incorrect query results.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants