Skip to content

feat: N-dimensional raster type extension (Phase 1) #749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
james-willis wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: N-dimensional raster type extension (Phase 1) #749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dad6f8c
refactor(sedona-schema): canonical N-D raster schema
james-willis May 4, 2026
d69069a
feat(sedona-raster): N-D trait surface and BandRef::is_2d
james-willis May 4, 2026
5bebc6e
refactor(sedona-raster, sedona-raster-functions, sedona-testing): N-D…
james-willis May 4, 2026
7a1c7e7
feat(raster-gdal): port loader to canonical N-D schema
james-willis May 4, 2026
f8da8e0
fix(raster-gdal): keep Cow::Owned band bytes alive for GDAL MEM dataset
james-willis May 6, 2026
9e396ce
feat(sedona-raster): error on OutDb byte access via BandRef accessors
james-willis May 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 24 additions & 15 deletions rust/sedona-raster-functions/src/executor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,13 +358,16 @@ impl<'a, 'b> RasterExecutor<'a, 'b> {
arr0.len()
);
}

// Hoist the RasterStructArray so its lifetime covers the loop.
let scalar_arr1;
let r1 = match sv1 {
ScalarValue::Struct(arc_struct) => {
let arr1 = RasterStructArray::new(arc_struct.as_ref());
if arr1.is_null(0) {
scalar_arr1 = RasterStructArray::new(arc_struct.as_ref());
if scalar_arr1.is_null(0) {
None
} else {
Some(arr1.get(0)?)
Some(scalar_arr1.get(0)?)
}
}
ScalarValue::Null => None,
Expand Down Expand Up @@ -395,13 +398,16 @@ impl<'a, 'b> RasterExecutor<'a, 'b> {
arr1.len()
);
}

// Hoist the RasterStructArray so its lifetime covers the loop.
let scalar_arr0;
let r0 = match sv0 {
ScalarValue::Struct(arc_struct) => {
let arr0 = RasterStructArray::new(arc_struct.as_ref());
if arr0.is_null(0) {
scalar_arr0 = RasterStructArray::new(arc_struct.as_ref());
if scalar_arr0.is_null(0) {
None
} else {
Some(arr0.get(0)?)
Some(scalar_arr0.get(0)?)
}
}
ScalarValue::Null => None,
Expand All @@ -421,27 +427,30 @@ impl<'a, 'b> RasterExecutor<'a, 'b> {
Ok(())
}
(ColumnarValue::Scalar(sv0), ColumnarValue::Scalar(sv1)) => {
// Hoist both RasterStructArrays so their lifetimes cover the loop.
let scalar_arr0;
let r0 = match sv0 {
ScalarValue::Struct(arc_struct) => {
let arr0 = RasterStructArray::new(arc_struct.as_ref());
if arr0.is_null(0) {
scalar_arr0 = RasterStructArray::new(arc_struct.as_ref());
if scalar_arr0.is_null(0) {
None
} else {
Some(arr0.get(0)?)
Some(scalar_arr0.get(0)?)
}
}
ScalarValue::Null => None,
_ => {
return sedona_internal_err!("Expected Struct scalar for raster");
}
};
let scalar_arr1;
let r1 = match sv1 {
ScalarValue::Struct(arc_struct) => {
let arr1 = RasterStructArray::new(arc_struct.as_ref());
if arr1.is_null(0) {
scalar_arr1 = RasterStructArray::new(arc_struct.as_ref());
if scalar_arr1.is_null(0) {
None
} else {
Some(arr1.get(0)?)
Some(scalar_arr1.get(0)?)
}
}
ScalarValue::Null => None,
Expand Down Expand Up @@ -724,7 +733,7 @@ mod tests {
match raster_opt {
None => builder.append_null(),
Some(raster) => {
let width = raster.metadata().width();
let width = raster.width().unwrap();
Comment on lines -727 to +736
Copy link
Copy Markdown
Member

@paleolimbot paleolimbot May 12, 2026

Choose a reason for hiding this comment

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

Probably width() should return a Result so non-test uses can do raster.width()? instead of unwrapping

builder.append_value(width);
}
}
Expand Down Expand Up @@ -766,7 +775,7 @@ mod tests {
match raster_opt {
None => builder.append_null(),
Some(raster) => {
let width = raster.metadata().width();
let width = raster.width().unwrap();
builder.append_value(width);
}
}
Expand Down Expand Up @@ -803,7 +812,7 @@ mod tests {
match raster_opt {
None => builder.append_null(),
Some(raster) => {
let width = raster.metadata().width();
let width = raster.width().unwrap();
builder.append_value(width);
}
}
Expand Down
69 changes: 27 additions & 42 deletions rust/sedona-raster-functions/src/rs_band_accessors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use datafusion_common::cast::as_int32_array;
use datafusion_common::error::Result;
use datafusion_expr::{ColumnarValue, Volatility};
use sedona_expr::scalar_udf::{SedonaScalarKernel, SedonaScalarUDF};
use sedona_raster::traits::RasterRef;
use sedona_raster::traits::{nodata_bytes_to_f64, RasterRef};
use sedona_schema::{datatypes::SedonaType, matchers::ArgMatcher};

// ===========================================================================
Expand Down Expand Up @@ -120,14 +120,15 @@ fn get_pixel_type(
Ok(())
}
Some(raster) => {
let num_bands = raster.bands().len();
let num_bands = raster.num_bands();
if band_index < 1 || band_index > num_bands as i32 {
builder.append_null();
return Ok(());
}
let band = raster.bands().band(band_index as usize)?;
let dt = band.metadata().data_type()?;
builder.append_value(dt.pixel_type_name());
match raster.band_data_type((band_index - 1) as usize) {
Some(dt) => builder.append_value(dt.pixel_type_name()),
None => builder.append_null(),
Copy link
Copy Markdown
Contributor Author

@james-willis james-willis May 5, 2026

Choose a reason for hiding this comment

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

not totally sure if we should handle this branch or throw. should generally be unreachable - we don't expect the arrow to contain datatype values that don't exist.

Copy link
Copy Markdown
Member

@paleolimbot paleolimbot May 12, 2026

Choose a reason for hiding this comment

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

sedona_internal_err!() is a good choice for violations of internal assumptions anywhere you can return a Result<> (better to know and get an issue report than a silent null)

}
Ok(())
}
}
Expand Down Expand Up @@ -224,16 +225,19 @@ fn get_nodata_value(
Ok(())
}
Some(raster) => {
let num_bands = raster.bands().len();
let num_bands = raster.num_bands();
if band_index < 1 || band_index > num_bands as i32 {
builder.append_null();
return Ok(());
}
let band = raster.bands().band(band_index as usize)?;
let band_meta = band.metadata();
match band_meta.nodata_value_as_f64()? {
None => builder.append_null(),
Some(val) => builder.append_value(val),
let idx = (band_index - 1) as usize;
match (raster.band_nodata(idx), raster.band_data_type(idx)) {
(Some(bytes), Some(dt)) => {
let val = nodata_bytes_to_f64(bytes, &dt)
.map_err(datafusion_common::DataFusionError::from)?;
Comment on lines +236 to +237
Copy link
Copy Markdown
Member

@paleolimbot paleolimbot May 12, 2026

Choose a reason for hiding this comment

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

Can you make sure this is the version that errors for i64 and u64?

builder.append_value(val);
}
_ => builder.append_null(),
}
Ok(())
}
Expand All @@ -246,30 +250,27 @@ mod tests {
use arrow_array::{Array, Float64Array, Int32Array, Int64Array, StringArray, StructArray};
use datafusion_expr::ScalarUDF;
use sedona_raster::builder::RasterBuilder;
use sedona_raster::traits::{BandMetadata, RasterMetadata};
use sedona_schema::datatypes::RASTER;
use sedona_schema::raster::{BandDataType, StorageType};
use sedona_schema::raster::BandDataType;
use sedona_testing::compare::assert_array_equal;
use sedona_testing::rasters::generate_test_rasters;
use sedona_testing::testers::ScalarUdfTester;

/// Build a single-row raster StructArray with custom metadata and band metadata.
/// Build a single-row raster StructArray with custom parameters.
fn build_custom_raster(
meta: &RasterMetadata,
band_meta: &BandMetadata,
width: u64,
height: u64,
data_type: BandDataType,
nodata: Option<&[u8]>,
data: &[u8],
crs: Option<&str>,
) -> StructArray {
let mut builder = RasterBuilder::new(1);
builder.start_raster(meta, crs).expect("start raster");
builder
.start_band(BandMetadata {
datatype: band_meta.datatype,
nodata_value: band_meta.nodata_value.clone(),
storage_type: band_meta.storage_type,
outdb_url: band_meta.outdb_url.clone(),
outdb_band_id: band_meta.outdb_band_id,
})
.start_raster_2d(width, height, 0.0, 0.0, 1.0, -1.0, 0.0, 0.0, crs)
.expect("start raster");
builder
.start_band_2d(data_type, nodata)
.expect("start band");
builder.band_data_writer().append_value(data);
builder.finish_band().expect("finish band");
Expand Down Expand Up @@ -401,25 +402,9 @@ mod tests {
#[test]
fn udf_bandnodatavalue_no_nodata() {
// Create a raster without nodata
let meta = RasterMetadata {
width: 2,
height: 2,
upperleft_x: 0.0,
upperleft_y: 0.0,
scale_x: 1.0,
scale_y: -1.0,
skew_x: 0.0,
skew_y: 0.0,
};
let band_meta = BandMetadata {
datatype: BandDataType::UInt8,
nodata_value: None,
storage_type: StorageType::InDb,
outdb_url: None,
outdb_band_id: None,
};
let data = vec![1u8, 2, 3, 4];
let rasters = build_custom_raster(&meta, &band_meta, &data, Some("OGC:CRS84"));
let rasters =
build_custom_raster(2, 2, BandDataType::UInt8, None, &data, Some("OGC:CRS84"));

let udf: ScalarUDF = rs_bandnodatavalue_udf().into();
let tester = ScalarUdfTester::new(udf, vec![RASTER]);
Expand Down
Loading
Loading