feat: support ST_DWithin pushdown in vortex#8625
Conversation
Signed-off-by: Nemo Yu <zyu379@wisc.edu>
| match format { | ||
| Format::VortexNative => strip_wkb_wrappers(query), | ||
| // Native geometry is `GEOMETRY`: drop `ST_GeomFromWKB(..)`, route pushable `ST_DWithin`. | ||
| Format::VortexNative => route_pushable_dwithin(&strip_wkb_wrappers(query)), |
There was a problem hiding this comment.
I think CREATE MACRO in init.sql is an easier approach than rewriting the query manually.
There was a problem hiding this comment.
Here the queries call ST_GeomFromWKB/ST_DWithin, which the spatial extension already defines, and you can't shadow those: CREATE MACRO ST_DWithin(...) -> Catalog Error: Macro Function with name "ST_DWithin" already exists (same for ST_GeomFromWKB, both tested).
| } | ||
| const DatabaseWrapper &wrapper = *reinterpret_cast<DatabaseWrapper *>(ffi_db); | ||
| try { | ||
| Connection conn(*wrapper.database->instance); |
There was a problem hiding this comment.
Why do we need a transaction here? Can we register the function in the global catalog without it?
There was a problem hiding this comment.
It looks like we cannot not. DuckDB's catalog is MVCC, so CreateFunction needs an active transaction (it throws ActiveTransaction called without active transaction otherwise). A fresh Connection only begins a transaction per query, and we're calling the catalog API directly, so RunFunctionInTransaction is just the "begin → run → commit" wrapper to supply one.
I confirmed by removing it: registration aborts and queries then fail with vortex_dwithin does not exist.
- Replace non-ASCII characters in comments with ASCII. - Document why catalog registration needs RunFunctionInTransaction. - Reference the FFI function in register_geo_aliases doc. Signed-off-by: Nemo Yu <zyu379@wisc.edu>
Merging this PR will improve performance by 12.06%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[128] |
244.4 ns | 215.3 ns | +13.55% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[1024] |
304.7 ns | 275.6 ns | +10.58% |
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-native-pushdown (94674e0) with nemo/duckdb-native-geometry (69a4f90)
Footnotes
-
4 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. ↩
2 participants
Summary
Insert non-throwing geo predicate
vortex_dwithinin DuckDB, which later pushdown into vortex, calldistancescalar function on scanning, significantly improve Q1/Q3 performance in SpatialBench.What changes are included in this PR?
vortex_dwithinin DuckDB, which is non-throwing and can be pushdown.ST_dwithinis text rewritten intovortex_dwithin.Performance
Takeaways: Q1 and Q3 is significantly improved due to the single table geo predicate is pushdown into vortex scan. Q1 is more benefit from the manually fast path without going through
geocrate for calculation. Q3 is also potentially can be benefit from manually calculation.