feat(rust/sedona-raster-gdal): add RS_FromPath by Kontinuation · Pull Request #831 · apache/sedona-db

Kontinuation · 2026-05-11T02:06:24Z

Summary

add RS_FromPath for constructing out-db rasters from GDAL-openable paths
wire sedona-raster-gdal and SedonaContext minimally for this standalone child PR
keep other GDAL-backed raster functions out of scope for follow-up PRs

Testing

cargo test -p sedona-raster-gdal rs_from_path --no-default-features
cargo clippy -p sedona-raster-gdal --no-default-features -- -D warnings
cargo test -p sedona-raster-gdal -p sedona --lib
cargo clippy -p sedona-raster-gdal -p sedona --lib -- -D warnings

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new GDAL-backed raster UDF that constructs out-of-db rasters from GDAL-openable paths, and wires it into SedonaContext for availability.

Changes:

Introduced RS_FromPath-style UDF (rs_frompath) backed by GDAL, plus a Criterion benchmark.
Added out-db raster construction helper (append_as_outdb_raster) and centralized nodata (de)serialization helpers.
Registered GDAL UDFs during SedonaContext initialization and added SQL reference docs.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
rust/sedona/src/context.rs	Registers all GDAL UDFs into `SedonaContext` during initialization.
rust/sedona-raster-gdal/src/utils.rs	Adds out-db raster builder path, refactors nodata conversion usage, expands tests.
rust/sedona-raster-gdal/src/rs_frompath.rs	Implements the path-based raster UDF and its unit tests.
rust/sedona-raster-gdal/src/lib.rs	Exposes the new UDF and provides `all_gdal_udfs()`.
rust/sedona-raster-gdal/src/gdal_dataset_provider.rs	Refactors nodata setting to shared helper.
rust/sedona-raster-gdal/src/gdal_common.rs	Adds shared helpers for reading/setting band nodata bytes.
rust/sedona-raster-gdal/benches/rs_frompath.rs	Adds benchmarks for the new UDF using test fixtures.
rust/sedona-raster-gdal/Cargo.toml	Adds deps needed for the UDF + registers the benchmark target.
docs/reference/sql/rs_frompath.qmd	Adds SQL reference documentation for RS_FromPath.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jiayuasu · 2026-05-13T05:29:12Z

@james-willis please review too

paleolimbot

Thank you for working on this!

paleolimbot · 2026-05-13T13:51:41Z

+            let len = path_array.len();
+            let mut builder = RasterBuilder::new(len);
+            for i in 0..len {
+                if path_array.is_null(i) {
+                    builder.append_null()?;
+                } else {
+                    let path = path_array.value(i);
+                    append_as_outdb_raster(gdal, path, &mut builder)?;
+                }
+            }


This works, although our iteration elsewhere is generally for item_opt in path_array { ... }

Switched to for path_opt in path_array

paleolimbot · 2026-05-13T13:52:34Z

+
+            match &args[0] {
+                ColumnarValue::Scalar(_) => {
+                    let scalar = datafusion_common::ScalarValue::try_from_array(&result, 0)?;
+                    Ok(ColumnarValue::Scalar(scalar))
+                }
+                ColumnarValue::Array(_) => Ok(ColumnarValue::Array(result)),
+            }


Suggested change

match &args[0] {

ColumnarValue::Scalar(_) => {

let scalar = datafusion_common::ScalarValue::try_from_array(&result, 0)?;

Ok(ColumnarValue::Scalar(scalar))

}

ColumnarValue::Array(_) => Ok(ColumnarValue::Array(result)),

}

executor.finish(result)

paleolimbot · 2026-05-13T13:54:00Z

+
+            let paths = args[0].cast_to(&DataType::Utf8, None)?.into_array(1)?;


Suggested change

let paths = args[0].cast_to(&DataType::Utf8, None)?.into_array(1)?;

let executor = WkbBytesExecutor::new(arg_types, args);

let paths = args[0].cast_to(&DataType::Utf8, None)?.into_array(executor.num_iterations())?;

The rest of this loop suggests this can handle both scalars and columns but I think as written this will fail for anything except a scalar or one row table

Switched to use WkbBytesExecutor. It looks a bit strange: we use the executor for purposes other than iterating over the input data, but it still make the code shorter.

paleolimbot · 2026-05-13T13:59:46Z

+    fn assert_raster_dimensions(
+        result: &ColumnarValue,
+        expected_len: usize,
+        width: u64,
+        height: u64,
+    ) {
+        match result {


Not your problem for this PR, but we really need support for rasters in the ScalarUdfTester so that these tests can be more compact.

In the meantime this could be moved to sedona_testing::rasters which has a few asserters along these lines.

paleolimbot · 2026-05-13T14:03:30Z

+/// Append a raster source path as a single out-db raster to the provided [`RasterBuilder`].
+pub fn append_as_outdb_raster(gdal: &Gdal, path: &str, builder: &mut RasterBuilder) -> Result<()> {
+    let gdal_path = normalize_outdb_source_path(path);
+    let dataset = gdal
+        .open_ex_with_options(
+            &gdal_path,
+            DatasetOptions {
+                open_flags: GDAL_OF_RASTER | GDAL_OF_READONLY,
+                ..Default::default()
+            },
+        )


I am guessing this does blocking IO. Should we make any attempt to mitigate this or implement RS_FromPath as an async UDF to get the auto batch size adjustment from DataFusion for these types of functions?

paleolimbot · 2026-05-13T14:05:01Z

 result_large_err = "allow"

 [dependencies]
+arrow = { workspace = true }


Can we use a targeted subcrate for whatever uses this import?

Depend on arrow-array, arrow-buffer and arrow-schema.

paleolimbot · 2026-05-13T14:08:58Z

        );

+        for udf in sedona_raster_gdal::all_gdal_udfs() {
+            out.ctx.register_udf(udf.into());


Can this use the same pattern as the other crates that provide scalar functions? (e.g., out.register_scalar_kernels(sedona_raster_gdal::register::scalar_kernels().into_iter())?;). I suppose if we make it an async UDF we'll need the pattern you have here.

Now we have sedona_raster_gdal::register::default_function_set, so this loop was replaced by out.register_function_set(sedona_raster_gdal::register::default_function_set());.

zhangfengcdt

We may want to align the registration the same way as other UDFs via scalar_kernels() from a register module inside sedona-raster-gdal as @paleolimbot already points out, otherwise, LTGM!

Kontinuation · 2026-05-31T07:47:26Z

I tried to implement RS_FromPath as a DataFusion async UDF but was blocked by a DataFusion bug: apache/datafusion#22662. I have submitted a patch for fixing this: apache/datafusion#22663.

I'd like to leave RS_FromPath as synchronous UDF for now until the fix got accepted and released by the upstream. The async version of RS_FromPath can be found in https://github.com/Kontinuation/sedona-db/tree/pr-c-rs-frompath-async. We extended SedonaScalarUDF to support async function and made RS_FromPath async by dispatching GDAL calls to tokio worker threads.

paleolimbot

I tried to implement RS_FromPath as a DataFusion async UDF but was blocked by a DataFusion bug:

Thank you for trying and for fixing that!

I'd like to leave RS_FromPath as synchronous UDF for now until the fix got accepted and released by the upstream.

Sounds good to me. If an async context is truly helpful we can store an Arc<Runtime> in the UDF and instantiate/register from the Cli/Python where we have access to it, but no need to do that here.

paleolimbot · 2026-06-02T02:38:19Z

A note that this has some overlap with #886 but I also think it's good to have this merge and we can draw the best from both approaches as we go.

github-actions Bot requested a review from prantogg May 11, 2026 02:06

This was referenced May 11, 2026

feat(rust/sedona-raster-gdal): add RS_MetaData #833

Draft

feat(rust/sedona-raster-gdal): add RS_MetaData Kontinuation/sedona-db#3

Closed

Kontinuation requested a review from Copilot May 12, 2026 16:55

Copilot AI reviewed May 12, 2026

View reviewed changes

Comment thread rust/sedona-raster-gdal/src/rs_frompath.rs Outdated

Comment thread rust/sedona-raster-gdal/src/utils.rs

Comment thread rust/sedona/src/context.rs Outdated

Comment thread rust/sedona-raster-gdal/src/rs_frompath.rs Outdated

Comment thread rust/sedona-raster-gdal/src/gdal_common.rs

Copilot started reviewing on behalf of Kontinuation May 12, 2026 17:21 View session

Kontinuation marked this pull request as ready for review May 13, 2026 02:53

jiayuasu requested review from paleolimbot and zhangfengcdt May 13, 2026 05:29

paleolimbot reviewed May 13, 2026

View reviewed changes

zhangfengcdt approved these changes May 13, 2026

View reviewed changes

Kontinuation mentioned this pull request May 31, 2026

AsyncFuncExpr drops async UDF return field metadata apache/datafusion#22662

Open

kontinuation added 7 commits May 31, 2026 15:37

feat(rust/sedona-raster-gdal): add RS_FromPath

908093c

fix(rust/sedona-raster-gdal): align RS_FromPath with the single-path API

c7decc3

Refactored RS_FromPath after manual reviewing

73927fa

fix(rust/sedona-raster-gdal): cover RS_FromPath edge cases

cfe08e1

fix(docs/reference/sql): use checked-in raster for RS_FromPath

7af8c93

fix(rust/sedona-raster-gdal): address review follow-ups

ebb7f15

Fix compilation and tests after rebasing

2b03dd8

Kontinuation force-pushed the pr-c-rs-frompath branch from 41974dd to 2b03dd8 Compare May 31, 2026 07:43

fix(rust/sedona-raster-gdal): use executor shape handling in RS_FromPath

57edaa8

paleolimbot approved these changes Jun 1, 2026

View reviewed changes

paleolimbot mentioned this pull request Jun 2, 2026

feat(rust): Lazy Raster Loading Support for Zarr and GDAL #886

Merged

2 tasks

Kontinuation merged commit badd49f into apache:main Jun 2, 2026
17 checks passed


		let paths = args[0].cast_to(&DataType::Utf8, None)?.into_array(1)?;

-            let paths = args[0].cast_to(&DataType::Utf8, None)?.into_array(1)?;
+            let executor = WkbBytesExecutor::new(arg_types, args);
+            let paths = args[0].cast_to(&DataType::Utf8, None)?.into_array(executor.num_iterations())?;

Conversation

Kontinuation commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jiayuasu commented May 13, 2026

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhangfengcdt left a comment

Choose a reason for hiding this comment

Uh oh!

Kontinuation commented May 31, 2026

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

paleolimbot commented Jun 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Kontinuation commented May 11, 2026 •

edited

Loading