Skip to content

refactor(rust/sedona-functions): centralize bounding functions - #1216

Open
paleolimbot wants to merge 3 commits into
apache:mainfrom
paleolimbot:codex/issue-1037-bounder-functions
Open

refactor(rust/sedona-functions): centralize bounding functions#1216
paleolimbot wants to merge 3 commits into
apache:mainfrom
paleolimbot:codex/issue-1037-bounder-functions

Conversation

@paleolimbot

@paleolimbot paleolimbot commented Sep 2, 2026

Copy link
Copy Markdown
Member

This PR makes bounding functions for geography generic, implemented in sedona-functions using the bounder present in the config options. This lets geography bounding occur based on runtime injection of a bounder, rather than compile time features present in the sedona build.

I checked the bounding benchmarks for sedona-functions and didn't find any regressions.

Closes #1037.

@paleolimbot
paleolimbot marked this pull request as ready for review September 2, 2026 22:40
SedonaType::Arrow(DataType::Null) => Edges::Planar,
_ => return sedona_internal_err!("Expected geometry or geography, got {arg_type:?}"),
};
let factory = config_options

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we register the bounder in new_from_context() too? With the sedona/s2geography feature enabled, this test now fails:

#[tokio::test]
async fn geography_bounds_default_context() {
    let ctx = sedona::context::SedonaContext::new();
    ctx.ctx
        .sql("SELECT ST_XMin(ST_GeogFromText('POINT (1 2)'))")
        .await.unwrap()
        .collect().await.unwrap();
}

The error is ST_XMin() requires a bounder for Spherical edges, which is not registered in this session. new_from_context(SessionContext::new()) has the same problem. new_local_interactive() works because it installs the bounder.

);

let mut bounder = T::default();
let mut bounder = bounder_for_arg_type(&arg_types[0], config_options, "ST_Envelope")?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is geography support meant to survive a native UDF export/import roundtrip? I reproduced a failure at the Rust FFI boundary; the equivalent Python example is:

import sedonadb
from sedonadb.udf import sedona_native_scalar_udf

con = sedonadb.connect()
capsules = con.funcs.st_envelope.__sedonadb_scalar_udf__()
con.register(sedona_native_scalar_udf(capsules, name="rt_envelope"))

con.sql("SELECT ST_Envelope(ST_GeogFromText('POINT (1 2)'))").to_arrow_table()
con.sql("SELECT rt_envelope(ST_GeogFromText('POINT (1 2)'))").to_arrow_table()

The direct call works, but the imported call reports ST_Envelope() requires a bounder for Spherical edges, which is not registered in this session. The C wrapper invokes the exported kernel with None for ConfigOptions, so it cannot find the session's bounder. ST_XMin has the same issue; its previous S2 kernel worked through this roundtrip.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rust/sedona-functions: Move geography bounding functions into sedona-functions

2 participants