You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sedona.apache.org by ja...@apache.org on 2026/09/29 22:07:13 UTC
(sedona-db) branch main updated: fix(rust/sedona-raster-functions): name the band and its source file in RS_Value's out-of-range error (#1358)
This is an automated email from the ASF dual-hosted git repository.
james-willis pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/sedona-db.git
The following commit(s) were added to refs/heads/main by this push:
new 5ff399ad5 fix(rust/sedona-raster-functions): name the band and its source file in RS_Value's out-of-range error (#1358)
5ff399ad5 is described below
commit 5ff399ad53cc1b2a187a6c0a6b36bae011a036da
Author: James Willis <ja...@users.noreply.github.com>
AuthorDate: Tue Sep 29 15:07:07 2026 -0700
fix(rust/sedona-raster-functions): name the band and its source file in RS_Value's out-of-range error (#1358)
---
python/sedonadb/tests/functions/test_rs_value.py | 26 +++++
rust/sedona-raster-functions/src/rs_value.rs | 7 +-
rust/sedona-raster-functions/src/sampling.rs | 120 ++++++++++++++++++++++-
3 files changed, 149 insertions(+), 4 deletions(-)
diff --git a/python/sedonadb/tests/functions/test_rs_value.py b/python/sedonadb/tests/functions/test_rs_value.py
index 993be2a11..8131d3541 100644
--- a/python/sedonadb/tests/functions/test_rs_value.py
+++ b/python/sedonadb/tests/functions/test_rs_value.py
@@ -27,8 +27,11 @@ discriminate flooring from int-truncation in the world-to-pixel math (they
differ only for negative fractional indices).
"""
+import re
+
import pyarrow as pa
import pytest
+import sedonadb
from sedonadb.raster_testing import (
_is_nodata,
@@ -243,6 +246,29 @@ def test_rs_values_matches_comparators(con, tmp_path, dtype):
assert got == expected, f"band {band}"
+def test_out_of_range_band_error_names_the_band(con, tmp_path):
+ """An out-of-range band is reported by the 1-based number the query
+ passed, not the 0-based index it maps to (band 3 of 2 used to read
+ "Band index 2"), and names the file the raster was read from."""
+ tiff = _write_fixture(tmp_path, "uint8", nodata=None)
+ x, y = pixel_center(0, 0)
+ message = f"Band 3 is out of range: {re.escape(str(tiff))} has 2 bands"
+ with pytest.raises(sedonadb._lib.SedonaError, match=message):
+ _sedonadb_value(con, tiff, x, y, band=3)
+ with pytest.raises(sedonadb._lib.SedonaError, match=message):
+ _sedonadb_values(con, tiff, [(x, y)], band=3)
+
+
+def test_out_of_range_band_error_without_a_file(con):
+ """A raster with no source file (built in memory) is "this raster"."""
+ sql = "SELECT RS_Value(RS_Example(), ST_Point(74.58, 110.57, 'OGC:CRS84'), 4)"
+ with pytest.raises(
+ sedonadb._lib.SedonaError,
+ match="Band 4 is out of range: this raster has 3 bands",
+ ):
+ con.sql(sql).to_arrow_table()
+
+
@pytest.mark.parametrize("dtype", ["uint8", "float64"])
def test_rs_band_nodata_matches_comparators(con, tmp_path, dtype):
"""The band nodata reads back exactly on every band, and a band without
diff --git a/rust/sedona-raster-functions/src/rs_value.rs b/rust/sedona-raster-functions/src/rs_value.rs
index ad5ac9192..52a184a9a 100644
--- a/rust/sedona-raster-functions/src/rs_value.rs
+++ b/rust/sedona-raster-functions/src/rs_value.rs
@@ -649,9 +649,14 @@ mod tests {
#[test]
fn band_out_of_range_errors() {
+ // The error names the 1-based band the caller asked for, not the
+ // accessor's 0-based index (which read "Band index 1" here).
let spec = RasterSpec::d2(2, 1).band_values(&[1u8, 2]);
let err = sample(spec, 0, 0, 2).unwrap_err().to_string();
- assert!(err.contains("RS_Value"), "unexpected error: {err}");
+ assert!(
+ err.contains("RS_Value: Band 2 is out of range: this raster has 1 bands"),
+ "unexpected error: {err}"
+ );
}
#[test]
diff --git a/rust/sedona-raster-functions/src/sampling.rs b/rust/sedona-raster-functions/src/sampling.rs
index f56187a7b..6965a1933 100644
--- a/rust/sedona-raster-functions/src/sampling.rs
+++ b/rust/sedona-raster-functions/src/sampling.rs
@@ -35,7 +35,9 @@ use sedona_geometry::error::SedonaGeometryError;
use sedona_geometry::transform::{CrsEngine, CrsTransform, visit_point_coords};
use sedona_raster::error::RasterResultExt;
use sedona_raster::geo_transform::GeoTransformEx;
-use sedona_raster::traits::{BandRef, NdBuffer, RasterRef, nodata_bytes_to_f64_lossless};
+use sedona_raster::traits::{
+ BandRef, NdBuffer, RasterRef, nodata_bytes_to_f64_lossless, split_outdb_band_fragment,
+};
use sedona_schema::crs::CrsRef;
use sedona_schema::datatypes::SedonaType;
use wkb::reader::read_wkb;
@@ -52,8 +54,9 @@ pub(crate) fn int32_array_arg(arg: &ColumnarValue, num_iterations: usize) -> Res
/// Resolve the **1-based** `band_num` to a [`BandRef`], mapping it onto the
/// raster's 0-based [`RasterRef::band`] accessor. Band 0 is rejected as not
-/// 1-based (callers clamp negative inputs to 0 for exactly this signal); an
-/// out-of-range band surfaces the accessor's own error. `func` names the
+/// 1-based (callers clamp negative inputs to 0 for exactly this signal). An
+/// out-of-range band is checked here rather than left to the accessor, whose
+/// error names the 0-based index the caller never wrote. `func` names the
/// calling UDF for the error message.
pub(crate) fn resolve_band<'a>(
func: &str,
@@ -63,11 +66,50 @@ pub(crate) fn resolve_band<'a>(
let index = band_num.checked_sub(1).ok_or_else(|| {
exec_datafusion_err!("{func}: Invalid band number {band_num}: band numbers must be 1-based")
})?;
+ let num_bands = raster.num_bands();
+ if index >= num_bands {
+ return exec_err!(
+ "{func}: Band {band_num} is out of range: {} has {num_bands} bands",
+ describe_raster(raster)
+ );
+ }
raster
.band(index)
.map_err(|e| exec_datafusion_err!("{func}: {e}"))
}
+/// How an error message names `raster`: the file its bands were read from,
+/// when every band carries the same source URI, otherwise `"this raster"`.
+///
+/// A band's URI loses its `#band=N` fragment, so the bands of one file compare
+/// equal, and a URL loses its query string, which can hold a signed URL's
+/// credentials that do not belong in an error message.
+pub(crate) fn describe_raster(raster: &dyn RasterRef) -> String {
+ let mut source = None;
+ for index in 0..raster.num_bands() {
+ let Some(uri) = raster.band_outdb_uri(index) else {
+ return "this raster".to_string();
+ };
+ let uri = source_of(uri);
+ match source {
+ None => source = Some(uri),
+ Some(ref seen) if *seen == uri => {}
+ Some(_) => return "this raster".to_string(),
+ }
+ }
+ source.unwrap_or_else(|| "this raster".to_string())
+}
+
+/// A band URI without its `#band=N` fragment and, for a `scheme://` URL, its
+/// query string.
+fn source_of(uri: &str) -> String {
+ let uri = split_outdb_band_fragment(uri).map_or_else(|_| uri.to_string(), |(base, _)| base);
+ match uri.split_once('?') {
+ Some((base, _)) if uri.contains("://") => base.to_string(),
+ _ => uri,
+ }
+}
+
/// Advance the optional band-number iterator one row, yielding the 1-based band
/// to sample. A missing band argument defaults to band 1; a NULL band element
/// returns `None`, which the caller propagates to a NULL result. Band 0 and
@@ -283,3 +325,75 @@ pub(crate) fn read_pixel(
Ok(Some(value))
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use sedona_raster::array::RasterStructArray;
+ use sedona_schema::raster::BandDataType;
+ use sedona_testing::raster_spec::RasterSpec;
+
+ fn describe(spec: RasterSpec) -> String {
+ let array = spec.build();
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ describe_raster(&rasters.get(0).unwrap())
+ }
+
+ fn outdb(spec: RasterSpec, uri: &str) -> RasterSpec {
+ spec.band(BandDataType::UInt8).outdb(uri, Some("geotiff"))
+ }
+
+ #[test]
+ fn names_the_file_every_band_was_read_from() {
+ let spec = outdb(
+ outdb(RasterSpec::d2(2, 2), "s3://bucket/foofy.tif#band=1"),
+ "s3://bucket/foofy.tif#band=2",
+ );
+ assert_eq!(describe(spec), "s3://bucket/foofy.tif");
+ }
+
+ #[test]
+ fn drops_a_urls_query_string() {
+ let spec = outdb(
+ RasterSpec::d2(2, 2),
+ "https://host/foofy.tif?X-Amz-Signature=secret#band=1",
+ );
+ assert_eq!(describe(spec), "https://host/foofy.tif");
+ // A local path keeps a literal '?'.
+ let spec = outdb(RasterSpec::d2(2, 2), "/data/what?.tif");
+ assert_eq!(describe(spec), "/data/what?.tif");
+ }
+
+ #[test]
+ fn falls_back_without_a_single_source() {
+ // In-memory bands carry no URI.
+ let spec = RasterSpec::d2(2, 2).band_values(&[0u8; 4]);
+ assert_eq!(describe(spec), "this raster");
+ // Bands read from different files.
+ let spec = outdb(
+ outdb(RasterSpec::d2(2, 2), "s3://bucket/a.tif"),
+ "s3://bucket/b.tif",
+ );
+ assert_eq!(describe(spec), "this raster");
+ // One band from a file, one in memory.
+ let spec = outdb(RasterSpec::d2(2, 2), "s3://bucket/a.tif").band_values(&[0u8; 4]);
+ assert_eq!(describe(spec), "this raster");
+ // No bands at all.
+ assert_eq!(describe(RasterSpec::d2(2, 2)), "this raster");
+ }
+
+ #[test]
+ fn out_of_range_band_names_the_file() {
+ let array = outdb(RasterSpec::d2(2, 2), "/data/foofy.tif").build();
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let raster = rasters.get(0).unwrap();
+ let err = resolve_band("RS_Value", &raster, 3)
+ .err()
+ .unwrap()
+ .to_string();
+ assert!(
+ err.contains("RS_Value: Band 3 is out of range: /data/foofy.tif has 1 bands"),
+ "{err}"
+ );
+ }
+}