ST_Intersects pushdown: vortex.geo.intersects kernel, DuckDB lowering, spatial function overrides

benchmarks/duckdb-bench/src/lib.rs

@@ -78,11 +78,12 @@ impl DuckClient {
         for stmt in &statements {
             self.connection().query(stmt)?;
         }
-        // After `LOAD spatial`, shadow `ST_DWithin` so radius filters push. No-op without it.
+        // After `LOAD spatial`, shadow the overridden spatial functions so that their filters
+        // push down. No-op without it.
         self.db
             .as_ref()
             .vortex_expect("DuckClient database accessed after close")
-            .register_st_dwithin_override()?;
+            .register_spatial_overrides()?;
         self.init_sql = statements;
         Ok(())
     }
@@ -132,11 +133,11 @@ impl DuckClient {
                 .vortex_expect("connection just opened")
                 .query(stmt)?;
         }
-        // Re-shadow `ST_DWithin` against the fresh instance.
+        // Re-shadow the overridden spatial functions against the fresh instance.
         self.db
             .as_ref()
             .vortex_expect("database just opened")
-            .register_st_dwithin_override()?;
+            .register_spatial_overrides()?;
 
         Ok(())
     }

vortex-duckdb/build.rs

@@ -27,12 +27,13 @@ const DEFAULT_DUCKDB_VERSION: &str = "1.5.3";
 
 const BUILD_ARTIFACTS: [&str; 3] = ["libduckdb.dylib", "libduckdb.so", "libduckdb_static.a"];
 
-const SOURCE_FILES: [&str; 10] = [
+const SOURCE_FILES: [&str; 11] = [
     "cpp/vortex_duckdb.cpp",
     "cpp/copy_function.cpp",
     "cpp/expr.cpp",
     "cpp/optimizer.cpp",
     "cpp/scalar_fn_pushdown.cpp",
+    "cpp/spatial_overrides.cpp",
     "cpp/cast_pushdown.cpp",
     "cpp/aggregate_fn_pushdown.cpp",
     "cpp/table_filter.cpp",
@@ -178,9 +179,10 @@ const DUCKDB_C_API_FUNCTIONS: [&str; 133] = [
     "duckdb_vector_size",
 ];
 
-const DUCKDB_C_API_HEADERS: [&str; 6] = [
+const DUCKDB_C_API_HEADERS: [&str; 7] = [
     "cpp/include/vortex_duckdb.h",
     "cpp/include/expr.h",
+    "cpp/include/spatial_overrides.h",
     "cpp/include/table_filter.h",
     "cpp/include/vector.h",
     "cpp/include/copy_function.h",

vortex-duckdb/cpp/expr.cpp

@@ -14,19 +14,6 @@
 #include "duckdb/planner/expression/bound_operator_expression.hpp"
 #include "duckdb/planner/expression/bound_conjunction_expression.hpp"
 
-#include "duckdb/catalog/catalog.hpp"
-#include "duckdb/catalog/catalog_entry/scalar_function_catalog_entry.hpp"
-#include "duckdb/common/error_data.hpp"
-#include "duckdb/logging/logger.hpp"
-#include "duckdb/main/capi/capi_internal.hpp"
-#include "duckdb/main/client_context.hpp"
-#include "duckdb/main/connection.hpp"
-#include "duckdb/main/database_manager.hpp"
-#include "duckdb/parser/parsed_data/create_scalar_function_info.hpp"
-#include "duckdb/transaction/meta_transaction.hpp"
-
-#include <exception>
-
 using namespace duckdb;
 
 extern "C" const char *duckdb_vx_sfunc_name(duckdb_vx_sfunc ffi_func) {
@@ -37,52 +24,6 @@ extern "C" const char *duckdb_vx_sfunc_name(duckdb_vx_sfunc ffi_func) {
     return func->name.c_str();
 }
 
-extern "C" duckdb_state duckdb_vx_register_st_dwithin_override(duckdb_database ffi_db) {
-    if (!ffi_db) {
-        return DuckDBError;
-    }
-    const DatabaseWrapper &wrapper = *reinterpret_cast<DatabaseWrapper *>(ffi_db);
-    DatabaseInstance &db = *wrapper.database->instance;
-    try {
-        Connection conn(db);
-        ClientContext &context = *conn.context;
-        context.RunFunctionInTransaction([&]() {
-            auto &system = Catalog::GetSystemCatalog(context);
-            auto entry = system.GetEntry<ScalarFunctionCatalogEntry>(context,
-                                                                     DEFAULT_SCHEMA,
-                                                                     "st_dwithin",
-                                                                     OnEntryNotFound::RETURN_NULL);
-            if (!entry) {
-                // No `spatial` loaded, so there is no `ST_DWithin` to override.
-                return;
-            }
-            ScalarFunctionSet set("st_dwithin");
-            for (const auto &overload : entry->functions.functions) {
-                ScalarFunction copy = overload;
-                // Keep the radius as children[2]; spatial's bind folds it into private bind data.
-                copy.bind = nullptr;
-                set.AddFunction(copy);
-            }
-            CreateScalarFunctionInfo info(std::move(set));
-            info.on_conflict = OnCreateConflict::REPLACE_ON_CONFLICT;
-            // `internal` entries are only accepted by the system catalog.
-            info.internal = false;
-            // The user catalog binds ahead of the system catalog, shadowing spatial's entry;
-            // `RestoreStDWithin` rebinds unpushed calls through the original.
-            auto &catalog = Catalog::GetCatalog(context, DatabaseManager::GetDefaultDatabase(context));
-            // Durable catalogs require the modified mark; scalar function entries are never
-            // persisted, so this is metadata-only.
-            MetaTransaction::Get(context).ModifyDatabase(catalog.GetAttached(), DatabaseModificationType());
-            catalog.CreateFunction(context, info);
-        });
-    } catch (const std::exception &e) {
-        ErrorData data(e);
-        DUCKDB_LOG_ERROR(db, "Failed to register the ST_DWithin override:\t" + data.Message());
-        return DuckDBError;
-    }
-    return DuckDBSuccess;
-}
-
 extern "C" const char *duckdb_vx_agg_func_name(duckdb_vx_agg_func ffi) {
     D_ASSERT(ffi);
     return reinterpret_cast<AggregateFunction *>(ffi)->name.c_str();

vortex-duckdb/cpp/include/expr.h

@@ -14,11 +14,6 @@ typedef struct duckdb_vx_agg_func_ *duckdb_vx_agg_func;
 
 const char *duckdb_vx_sfunc_name(duckdb_vx_sfunc ffi_func);
 
-/// Shadow `ST_DWithin` with a copy that keeps the radius as the third argument, so
-/// radius filters can push into Vortex scans. See `RestoreStDWithin` in
-/// scalar_fn_pushdown.hpp for the override/restore example.
-duckdb_state duckdb_vx_register_st_dwithin_override(duckdb_database ffi_db);
-
 typedef struct duckdb_vx_expr_ *duckdb_vx_expr;
 
 const char *duckdb_vx_agg_func_name(duckdb_vx_agg_func func);

vortex-duckdb/cpp/include/scalar_fn_pushdown.hpp

@@ -35,5 +35,3 @@ struct ScalarFnReplace final : LogicalOperatorVisitor {
     ExpressionPtr VisitReplace(BoundColumnRefExpression &expr, ExpressionPtr *ptr) override;
     ExpressionPtr VisitReplace(BoundFunctionExpression &expr, ExpressionPtr *ptr) override;
 };
-
-void RestoreStDWithin(ClientContext &context, LogicalOperator &plan);

vortex-duckdb/cpp/include/spatial_overrides.h

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: Copyright the Vortex contributors
+
+#pragma once
+
+#include "duckdb.h"
+
+#ifdef __cplusplus /* If compiled as C++, use C ABI */
+extern "C" {
+#endif
+
+/// Shadow the spatial functions that block filter pushdown with pushable copies; see
+/// `SPATIAL_OVERRIDES` in spatial_overrides.cpp for the list. Call after `LOAD spatial`;
+/// does nothing when spatial is not loaded.
+duckdb_state duckdb_vx_register_spatial_overrides(duckdb_database ffi_db);
+
+#ifdef __cplusplus
+}
+#endif

vortex-duckdb/cpp/include/spatial_overrides.hpp

@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: Copyright the Vortex contributors
+#pragma once
+#include "duckdb/main/client_context.hpp"
+#include "duckdb/planner/expression/bound_function_expression.hpp"
+#include "duckdb/planner/logical_operator_visitor.hpp"
+
+using namespace duckdb;
+
+/*
+ * Vortex shadows some spatial (`ST_*`) scalar functions with tweaked copies, because as
+ * spatial registers them their filters can never push into Vortex scans.
+ *
+ * `duckdb_vx_register_spatial_overrides` (spatial_overrides.h) installs the copies after
+ * `LOAD spatial`. `RestoreSpatialOverrides` below hands join conditions back to spatial's
+ * originals: Vortex never pushes join conditions, and spatial's join machinery expects its
+ * own functions.
+ */
+
+// Rebinds overridden spatial calls in join conditions back to spatial's original, so spatial's
+// own machinery handles joins. Filters are left untouched: they keep the override and push to
+// Vortex scans.
+struct SpatialOverrideRestore final : LogicalOperatorVisitor {
+    ClientContext &context;
+
+    explicit SpatialOverrideRestore(ClientContext &context);
+    void VisitOperator(LogicalOperator &op) override;
+    unique_ptr<Expression> VisitReplace(BoundFunctionExpression &expr, unique_ptr<Expression> *ptr) override;
+};
+
+/// Rebind overridden spatial calls in join conditions to spatial's originals.
+void RestoreSpatialOverrides(ClientContext &context, LogicalOperator &plan);

vortex-duckdb/cpp/scalar_fn_pushdown.cpp

@@ -2,9 +2,6 @@
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 #include "scalar_fn_pushdown.hpp"
 
-#include "duckdb/catalog/catalog.hpp"
-#include "duckdb/catalog/catalog_entry/scalar_function_catalog_entry.hpp"
-#include "duckdb/function/function_binder.hpp"
 #include "duckdb/planner/operator/logical_projection.hpp"
 #include "duckdb/planner/operator/logical_get.hpp"
 
@@ -122,67 +119,3 @@ ScalarFnCollect::ScalarFnCollect(Analyses &analyses, const Projections &projecti
 ScalarFnReplace::ScalarFnReplace(Analyses &analyses, const Projections &projections)
     : analyses(analyses), projections(projections) {
 }
-
-namespace {
-
-// See RestoreStDWithin: rebinding through spatial's own entry lets spatial build its own bind
-// data, so nothing here depends on its internals.
-class StDWithinRestore final : public LogicalOperatorVisitor {
-public:
-    explicit StDWithinRestore(ClientContext &context) : context(context) {
-    }
-
-    // Restore join conditions, filters must keep the radius visible so
-    // DuckDB's filter pushdown can offer them to Vortex scans.
-    void VisitOperator(LogicalOperator &op) override {
-        using enum LogicalOperatorType;
-        switch (op.type) {
-        case LOGICAL_COMPARISON_JOIN:
-        case LOGICAL_ANY_JOIN:
-        case LOGICAL_DELIM_JOIN:
-        case LOGICAL_ASOF_JOIN:
-            VisitOperatorExpressions(op);
-            break;
-        default:
-            break;
-        }
-        VisitOperatorChildren(op);
-    }
-
-    ExpressionPtr VisitReplace(BoundFunctionExpression &expr, ExpressionPtr *) override {
-        if (expr.children.size() != 3 || expr.function.name != "st_dwithin") {
-            return nullptr; // Not the override's shape: keep it and descend into its children.
-        }
-        // The system catalog holds spatial's original; the user-catalog override cannot shadow
-        // this lookup.
-        auto original = Catalog::GetSystemCatalog(context).GetEntry<ScalarFunctionCatalogEntry>(
-            context,
-            DEFAULT_SCHEMA,
-            "st_dwithin",
-            OnEntryNotFound::RETURN_NULL);
-        if (!original) {
-            return nullptr;
-        }
-        vector<ExpressionPtr> children;
-        children.reserve(expr.children.size());
-        for (const auto &child : expr.children) {
-            children.push_back(child->Copy());
-        }
-        ErrorData error;
-        FunctionBinder binder(context);
-        auto bound = binder.BindScalarFunction(*original, std::move(children), error);
-        if (!bound) {
-            return nullptr; // No matching overload: keep the executable 3-argument form.
-        }
-        return bound;
-    }
-
-private:
-    ClientContext &context;
-};
-
-} // namespace
-
-void RestoreStDWithin(ClientContext &context, LogicalOperator &plan) {
-    StDWithinRestore(context).VisitOperator(plan);
-}