Skip to content

Commit 229b3bd

Browse files
feat: support ST_DWithin pushdown in vortex (#8625)
## Summary <!-- 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. --> Insert non-throwing geo predicate `vortex_dwithin` in DuckDB, which later pushdown into vortex, call `distance` scalar function on scanning, significantly improve Q1/Q3 performance in SpatialBench. ## What changes are included in this PR? <!-- No need to duplicate information from the previous section, but if you're touching many parts of the code base, its worth explicitly noting the important changes or how they are tested. --> 1. adding new geo predicate `vortex_dwithin` in DuckDB, which is non-throwing and can be pushdown. 2. adding SQL rewrite so for geo native type, `ST_dwithin` is text rewritten into `vortex_dwithin`. <!-- Are there any user-facing changes that might require documentation updates Is any public API changed? --> ## Performance ``` ┏━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Query ┃ duckdb:parquet (base) ┃ duckdb:vortex ┃ duckdb:vortex-native ┃ ┡━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━┩ │ 1 │ 179.6ms │ 132.1ms (0.74x) │ 29.0ms (0.16x) │ │ 2 │ 268.1ms │ 255.3ms (0.95x) │ 312.5ms (1.17x) │ │ 3 │ 247.8ms │ 216.4ms (0.87x) │ 140.2ms (0.57x) │ │ 4 │ 199.6ms │ 146.9ms (0.74x) │ 144.5ms (0.72x) │ │ 5 │ 3.40s │ 3.43s (1.01x) │ 3.03s (0.89x) │ │ 6 │ 528.1ms │ 359.2ms (0.68x) │ 455.4ms (0.86x) │ │ 7 │ 984.0ms │ 983.2ms (1.00x) │ 887.7ms (0.90x) │ │ 8 │ 1.08s │ 945.4ms (0.87x) │ 997.6ms (0.92x) │ │ 9 │ 34.2ms │ 33.5ms (0.98x) │ 43.6ms (1.27x) │ └───────┴───────────────────────┴─────────────────┴──────────────────────┘ ``` 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 `geo` crate for calculation. Q3 is also potentially can be benefit from manually calculation. --------- Signed-off-by: Nemo Yu <zyu379@wisc.edu>
1 parent ee2cd67 commit 229b3bd

11 files changed

Lines changed: 536 additions & 36 deletions

File tree

‎benchmarks/duckdb-bench/src/lib.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ impl DuckClient {
7878
for stmt in &statements {
7979
self.connection().query(stmt)?;
8080
}
81+
// After `LOAD spatial`, shadow `ST_DWithin` so radius filters push. No-op without it.
82+
self.db
83+
.as_ref()
84+
.vortex_expect("DuckClient database accessed after close")
85+
.register_st_dwithin_override()?;
8186
self.init_sql = statements;
8287
Ok(())
8388
}
@@ -127,6 +132,11 @@ impl DuckClient {
127132
.vortex_expect("connection just opened")
128133
.query(stmt)?;
129134
}
135+
// Re-shadow `ST_DWithin` against the fresh instance.
136+
self.db
137+
.as_ref()
138+
.vortex_expect("database just opened")
139+
.register_st_dwithin_override()?;
130140

131141
Ok(())
132142
}

‎vortex-duckdb/cpp/expr.cpp‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,19 @@
1111
#include "duckdb/planner/expression/bound_operator_expression.hpp"
1212
#include "duckdb/planner/expression/bound_conjunction_expression.hpp"
1313

14+
#include "duckdb/catalog/catalog.hpp"
15+
#include "duckdb/catalog/catalog_entry/scalar_function_catalog_entry.hpp"
16+
#include "duckdb/common/error_data.hpp"
17+
#include "duckdb/logging/logger.hpp"
18+
#include "duckdb/main/capi/capi_internal.hpp"
19+
#include "duckdb/main/client_context.hpp"
20+
#include "duckdb/main/connection.hpp"
21+
#include "duckdb/main/database_manager.hpp"
22+
#include "duckdb/parser/parsed_data/create_scalar_function_info.hpp"
23+
#include "duckdb/transaction/meta_transaction.hpp"
24+
25+
#include <exception>
26+
1427
using namespace duckdb;
1528

1629
extern "C" const char *duckdb_vx_sfunc_name(duckdb_vx_sfunc ffi_func) {
@@ -21,6 +34,52 @@ extern "C" const char *duckdb_vx_sfunc_name(duckdb_vx_sfunc ffi_func) {
2134
return func->name.c_str();
2235
}
2336

37+
extern "C" duckdb_state duckdb_vx_register_st_dwithin_override(duckdb_database ffi_db) {
38+
if (!ffi_db) {
39+
return DuckDBError;
40+
}
41+
const DatabaseWrapper &wrapper = *reinterpret_cast<DatabaseWrapper *>(ffi_db);
42+
DatabaseInstance &db = *wrapper.database->instance;
43+
try {
44+
Connection conn(db);
45+
ClientContext &context = *conn.context;
46+
context.RunFunctionInTransaction([&]() {
47+
auto &system = Catalog::GetSystemCatalog(context);
48+
auto entry = system.GetEntry<ScalarFunctionCatalogEntry>(context,
49+
DEFAULT_SCHEMA,
50+
"st_dwithin",
51+
OnEntryNotFound::RETURN_NULL);
52+
if (!entry) {
53+
// No `spatial` loaded, so there is no `ST_DWithin` to override.
54+
return;
55+
}
56+
ScalarFunctionSet set("st_dwithin");
57+
for (const auto &overload : entry->functions.functions) {
58+
ScalarFunction copy = overload;
59+
// Keep the radius as children[2]; spatial's bind folds it into private bind data.
60+
copy.bind = nullptr;
61+
set.AddFunction(copy);
62+
}
63+
CreateScalarFunctionInfo info(std::move(set));
64+
info.on_conflict = OnCreateConflict::REPLACE_ON_CONFLICT;
65+
// `internal` entries are only accepted by the system catalog.
66+
info.internal = false;
67+
// The user catalog binds ahead of the system catalog, shadowing spatial's entry;
68+
// `RestoreStDWithin` rebinds unpushed calls through the original.
69+
auto &catalog = Catalog::GetCatalog(context, DatabaseManager::GetDefaultDatabase(context));
70+
// Durable catalogs require the modified mark; scalar function entries are never
71+
// persisted, so this is metadata-only.
72+
MetaTransaction::Get(context).ModifyDatabase(catalog.GetAttached(), DatabaseModificationType());
73+
catalog.CreateFunction(context, info);
74+
});
75+
} catch (const std::exception &e) {
76+
ErrorData data(e);
77+
DUCKDB_LOG_ERROR(db, "Failed to register the ST_DWithin override:\t" + data.Message());
78+
return DuckDBError;
79+
}
80+
return DuckDBSuccess;
81+
}
82+
2483
extern "C" const char *duckdb_vx_expr_to_string(duckdb_vx_expr ffi_expr) {
2584
if (!ffi_expr) {
2685
return nullptr;

‎vortex-duckdb/cpp/include/expr.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ typedef struct duckdb_vx_sfunc_ *duckdb_vx_sfunc;
1313

1414
const char *duckdb_vx_sfunc_name(duckdb_vx_sfunc ffi_func);
1515

16+
/// Shadow `ST_DWithin` with a copy that keeps the radius as the third argument, so
17+
/// radius filters can push into Vortex scans. See `RestoreStDWithin` in
18+
/// scalar_fn_pushdown.hpp for the override/restore example.
19+
duckdb_state duckdb_vx_register_st_dwithin_override(duckdb_database ffi_db);
20+
1621
typedef struct duckdb_vx_expr_ *duckdb_vx_expr;
1722

1823
/// Return the string representation of the expression. Must be freed with `duckdb_free`.

‎vortex-duckdb/cpp/include/scalar_fn_pushdown.hpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,21 @@ using Projections = unordered_map<TableIndex, LogicalProjection &>;
7777

7878
LogicalOperatorPtr TryPushdownScalarFunctions(ClientContext &context, LogicalOperatorPtr plan);
7979

80+
/*
81+
* We override spatial's `ST_DWithin` with a copy that keeps the radius as a plain third
82+
* argument (see expr.h); the original folds it into bind data at bind time. We need the
83+
* radius visible to push the filter into a Vortex scan, but spatial's join optimizer only
84+
* recognizes the folded 2-argument form. So this pass rebinds join conditions through
85+
* spatial's original function and leaves filters alone:
86+
*
87+
* FILTER st_dwithin(t.geom, 'POINT(0 0)', 10.0) -- untouched, pushed into the scan
88+
* JOIN ON st_dwithin(a.geom, b.geom, 10.0) -- rebound: st_dwithin(a.geom, b.geom)
89+
* with the radius in bind data
90+
*
91+
* Runs in the pre-optimize hook, before any extension's optimizer pass.
92+
*/
93+
void RestoreStDWithin(ClientContext &context, LogicalOperator &plan);
94+
8095
/**
8196
* Collect fn(col) expressions i.e. expressions where a single function (not
8297
* a function chain) wraps a single bound column. If "col" is used without

‎vortex-duckdb/cpp/scalar_fn_pushdown.cpp‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33
#include "duckdb/catalog/catalog.hpp"
4+
#include "duckdb/catalog/catalog_entry/scalar_function_catalog_entry.hpp"
5+
#include "duckdb/function/function_binder.hpp"
46
#include "duckdb/planner/operator/logical_projection.hpp"
57
#include "scalar_fn_pushdown.hpp"
68
#include "table_function.hpp"
@@ -231,3 +233,67 @@ ScalarFnReplace::ScalarFnReplace(Analyses &analyses, const Projections &projecti
231233
TableColumnStorageIndex GetAnalysis::StorageIndex(TableColumnScanIndex idx) const {
232234
return get.GetColumnIds()[idx].GetPrimaryIndex();
233235
}
236+
237+
namespace {
238+
239+
// See RestoreStDWithin: rebinding through spatial's own entry lets spatial build its own bind
240+
// data, so nothing here depends on its internals.
241+
class StDWithinRestore final : public LogicalOperatorVisitor {
242+
public:
243+
explicit StDWithinRestore(ClientContext &context) : context(context) {
244+
}
245+
246+
// Restore join conditions, filters must keep the radius visible so
247+
// DuckDB's filter pushdown can offer them to Vortex scans.
248+
void VisitOperator(LogicalOperator &op) override {
249+
using enum LogicalOperatorType;
250+
switch (op.type) {
251+
case LOGICAL_COMPARISON_JOIN:
252+
case LOGICAL_ANY_JOIN:
253+
case LOGICAL_DELIM_JOIN:
254+
case LOGICAL_ASOF_JOIN:
255+
VisitOperatorExpressions(op);
256+
break;
257+
default:
258+
break;
259+
}
260+
VisitOperatorChildren(op);
261+
}
262+
263+
ExpressionPtr VisitReplace(BoundFunctionExpression &expr, ExpressionPtr *) override {
264+
if (expr.children.size() != 3 || expr.function.name != "st_dwithin") {
265+
return nullptr; // Not the override's shape: keep it and descend into its children.
266+
}
267+
// The system catalog holds spatial's original; the user-catalog override cannot shadow
268+
// this lookup.
269+
auto original = Catalog::GetSystemCatalog(context).GetEntry<ScalarFunctionCatalogEntry>(
270+
context,
271+
DEFAULT_SCHEMA,
272+
"st_dwithin",
273+
OnEntryNotFound::RETURN_NULL);
274+
if (!original) {
275+
return nullptr;
276+
}
277+
vector<ExpressionPtr> children;
278+
children.reserve(expr.children.size());
279+
for (const auto &child : expr.children) {
280+
children.push_back(child->Copy());
281+
}
282+
ErrorData error;
283+
FunctionBinder binder(context);
284+
auto bound = binder.BindScalarFunction(*original, std::move(children), error);
285+
if (!bound) {
286+
return nullptr; // No matching overload: keep the executable 3-argument form.
287+
}
288+
return bound;
289+
}
290+
291+
private:
292+
ClientContext &context;
293+
};
294+
295+
} // namespace
296+
297+
void RestoreStDWithin(ClientContext &context, LogicalOperator &plan) {
298+
StDWithinRestore(context).VisitOperator(plan);
299+
}

‎vortex-duckdb/cpp/vortex_duckdb.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,13 @@ static void VortexOptimizeFunction(OptimizerExtensionInput &input, unique_ptr<Lo
271271
plan = TryPushdownScalarFunctions(input.context, std::move(plan));
272272
}
273273

274+
static void VortexPreOptimizeFunction(OptimizerExtensionInput &input, unique_ptr<LogicalOperator> &plan) {
275+
RestoreStDWithin(input.context, *plan);
276+
}
277+
274278
struct VortexOptimizerExtension final : OptimizerExtension {
275-
inline VortexOptimizerExtension() : OptimizerExtension(VortexOptimizeFunction, nullptr, {}) {
279+
inline VortexOptimizerExtension()
280+
: OptimizerExtension(VortexOptimizeFunction, VortexPreOptimizeFunction, {}) {
276281
}
277282
};
278283

0 commit comments

Comments
 (0)