Skip to content

ST_Contains/ST_Within pushdown: vortex.geo.contains kernel, DuckDB lowering#8720

Merged
HarukiMoriarty merged 2 commits into
nemo/geo-intersectsfrom
nemo/geo-contains
Jul 13, 2026
Merged

ST_Contains/ST_Within pushdown: vortex.geo.contains kernel, DuckDB lowering#8720
HarukiMoriarty merged 2 commits into
nemo/geo-intersectsfrom
nemo/geo-contains

Conversation

@HarukiMoriarty

Copy link
Copy Markdown
Contributor

Rationale for this change

Adds the third geo predicate kernel: OGC ST_Contains, following the GeoDistance/GeoIntersects pattern from #8704. One kernel serves both SQL spellings, since ST_Within(a, b) is ST_Contains(b, a).

What changes are included in this PR?

  • vortex-geo: GeoContains kernel (vortex.geo.contains): OGC containment, boundary excluded, delegating to geo::Contains. Containment is asymmetric, so the operand roles are kept in separate helpers and pinned by a test. Nullable operands are rejected like the other kernels.
  • vortex-duckdb: st_contains and st_within both lower to the kernel — within is contains with the operands swapped. Both get spatial override rows so their filters push through views.

@HarukiMoriarty HarukiMoriarty requested a review from myrrc July 10, 2026 19:41
@codspeed-hq

codspeed-hq Bot commented Jul 10, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 11.27%

⚡ 5 improved benchmarks
✅ 1659 untouched benchmarks
⏩ 42 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation encode_varbin[(1000, 8)] 155.5 µs 138.9 µs +11.98%
Simulation encode_varbin[(1000, 4)] 154.5 µs 138.7 µs +11.4%
Simulation encode_varbin[(1000, 2)] 154 µs 138.5 µs +11.18%
Simulation compare_int_constant 298.7 µs 268.7 µs +11.16%
Simulation encode_varbin[(1000, 32)] 160.3 µs 144.9 µs +10.64%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing nemo/geo-contains (31aedfb) with nemo/geo-intersects (89e67a0)

Open in CodSpeed

Footnotes

  1. 42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@HarukiMoriarty HarukiMoriarty added the changelog/feature A new feature label Jul 10, 2026

@myrrc myrrc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

vortex-geo gains a GeoContains kernel (vortex.geo.contains): OGC
containment, boundary excluded, delegating to geo::Contains. Containment
is not symmetric, so the constant dispatch keeps the container and
contained roles in two distinct helpers, and the asymmetry is pinned by
tests.

Operands are validated up front in return_dtype via the shared
validate_geometry_operands helper: each must be a native geometry type
and non-nullable, matching GeoDistance and GeoIntersects.

vortex-duckdb lowers both st_contains and st_within to the one kernel:
st_within(a, b) is st_contains(b, a), so the shared match arm swaps the
operands for the within spelling. Both functions get spatial override
rows so their filters push through views, and the e2e pushdown matrix
grows to five filters on both routes. SpatialBench itself only uses
ST_Within as join conditions, which stay with DuckDB's spatial join;
Q1-Q9 row counts validate unchanged with the new overrides.

Signed-off-by: Nemo Yu <zyu379@wisc.edu>
@HarukiMoriarty HarukiMoriarty merged commit 2364f0e into nemo/geo-intersects Jul 13, 2026
4 checks passed
@HarukiMoriarty HarukiMoriarty deleted the nemo/geo-contains branch July 13, 2026 15:22
HarukiMoriarty added a commit that referenced this pull request Jul 13, 2026
…wering (#8720)

## Rationale for this change

<!--
Why are you proposing this change, and what is its impact?
Is it part of a long term effort, or a bigger change?

If this PR is related to a tracked effort or an open issue, please link
to the relevant issue.
-->

Adds the third geo predicate kernel: OGC `ST_Contains`, following the
`GeoDistance`/`GeoIntersects` pattern from #8704. One kernel serves both
SQL spellings, since `ST_Within(a, b)` is `ST_Contains(b, a)`.

## What changes are included in this PR?

- `vortex-geo`: `GeoContains` kernel (`vortex.geo.contains`): OGC
containment, boundary excluded, delegating to `geo::Contains`.
Containment is asymmetric, so the operand roles are kept in separate
helpers and pinned by a test. Nullable operands are rejected like the
other kernels.
- `vortex-duckdb`: `st_contains` and `st_within` both lower to the
kernel — within is contains with the operands swapped. Both get spatial
override rows so their filters push through views.

Signed-off-by: Nemo Yu <zyu379@wisc.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants