ST_Intersects pushdown: vortex.geo.intersects kernel, DuckDB lowering, spatial function overrides#8704
Merged
Merged
Conversation
Merging this PR will improve performance by 18.1%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
myrrc
requested changes
Jul 10, 2026
Contributor
|
Let's add a sqllogictest which checks our queries don't break if we override the functions |
HarukiMoriarty
force-pushed
the
nemo/geo-intersects
branch
from
32dd2cc to
8cfcef6
Compare
…, spatial overrides
vortex-geo gains a GeoIntersects kernel (OGC semantics: not disjoint,
boundary contact counts; the function id is kept stable so a future
stats-pruning rewrite can key on it). Kernels stay thin
materialize-and-delegate wrappers over geo: GeoDistance's columnar point
fast paths are removed as well, since geo computes row by row and
hand-rolled columnar paths belong in a future columnar geometry compute
library (noted in scalar_fn/mod.rs).
vortex-duckdb lowers two-argument st_intersects like st_distance, and
shadows the spatial functions whose registrations block filter pushdown:
st_intersects is marked fallible, which DuckDB will not push through a
view's projection, and st_dwithin folds its radius into bind data. The
overrides live in their own translation unit
(cpp/spatial_overrides.{hpp,cpp}) as a (name, arity, tweak) table shared
by registration and the join-condition restore pass, keeping
scalar_fn_pushdown.* free of spatial code. e2e tests cover every lowered
geo filter on a direct file scan and through a view.
Signed-off-by: Nemo Yu <zyu379@wisc.edu>
HarukiMoriarty
force-pushed
the
nemo/geo-intersects
branch
from
8cfcef6 to
25a839e
Compare
myrrc
approved these changes
Jul 10, 2026
The C declaration moves to spatial_overrides.h, which bindgen consumes alongside the other C API headers, and SpatialOverrideRestore is declared in spatial_overrides.hpp with its methods implemented in the .cpp, matching scalar_fn_pushdown's layout. Signed-off-by: Nemo Yu <zyu379@wisc.edu>
Geometry arrays are never nullable, so GeoIntersects and GeoDistance validate their operand dtypes in return_dtype, and the DuckDB lowering no longer pushes nullable native geometry columns. The column length checks use vortex_ensure_eq. Signed-off-by: Nemo Yu <zyu379@wisc.edu>
connortsui20
requested changes
Jul 13, 2026
`return_dtype` for GeoDistance and GeoIntersects only rejected nullable operands, so a non-geometry column (e.g. i32) passed planning and failed later inside the kernel. Extract a shared `validate_geometry_operands` helper in the extension module that checks each operand is a native geometry type and non-nullable, and call it from both `return_dtype`s. Addresses review feedback on PR #8704. Signed-off-by: Nemo Yu <zyu379@wisc.edu>
# Conflicts: # vortex-geo/src/extension/mod.rs
connortsui20
approved these changes
Jul 13, 2026
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>
…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>
HarukiMoriarty
force-pushed
the
nemo/geo-intersects
branch
from
2364f0e to
1bc431c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
ST_Intersectsover native geometry columns currently evaluates inside DuckDB, exporting every row asGEOMETRYjust to be tested. This adds a nativevortex.geo.intersectskernel and pushes the filter into the scan: on SpatialBench SF1 (6M points vs a literal polygon), 406ms (DuckDB filter) / 36.6ms (SPATIAL_JOIN) -> 12.4ms pushed. Part of the native geospatial lane, following theST_Distance/ST_DWithinpushdown.What changes are included in this PR?
vortex-geo:GeoIntersectskernel.vortex-duckdb: lowerst_intersects(native column, GEOMETRY literal)likest_distance.vortex-duckdb: shadow spatial'sST_Intersectswith a non-fallible copy — DuckDB won't push can-throw filters through the projection every view-registered table has. TheST_DWithinoverride generalizes into a table-driven registry (spatial_overrides.hpp) shared by registration and the join-condition restore pass.Note: drop
GeoDistance's columnar point fast paths.geois row-oriented, so kernels materialize rows regardless; hand-rolled columnar paths belong in a future columnar geometry compute library (pushed Q1: 5.4ms -> 12.6ms, still far ahead of unpushed).