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 { ... }

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

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?

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.

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!

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

bfa00de

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 added 2 commits May 12, 2026 00:10

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

8358f07

Refactored RS_FromPath after manual reviewing

2c52e4a

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

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

2756654

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

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

0861b1b

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

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

41974dd


		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!

paleolimbot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

paleolimbot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

zhangfengcdt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Kontinuation commented May 11, 2026 •

edited

Loading