Skip to content

Commit 38edff1

Browse files
committed
fix(rust/sedona-raster-functions): RS_SetBandNoDataValue clears nodata on null
A null nodata argument previously nulled the whole raster row. It now flows through as None and clears the addressed band's nodata via Override::Clear, rebuilding the raster with that band carrying no nodata sentinel. A null raster or band still yields a null raster. set_band_nodata takes Option<f64>; Some(v) sets Override::Set(packed bytes), None clears, and other bands keep their own nodata (Override::Keep).
1 parent ffc15ec commit 38edff1

1 file changed

Lines changed: 61 additions & 18 deletions

File tree

rust/sedona-raster-functions/src/rs_set_band_nodata.rs

Lines changed: 61 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
//! ```
2424
//!
2525
//! The setter companion to the `RS_BandNoDataValue` getter. `nodata` is a double
26-
//! packed into the band's native data type. A null raster, band, or nodata value
27-
//! yields a null raster (matching `RS_SetCRS`/`RS_SetSRID`).
26+
//! packed into the band's native data type. A null raster or band yields a null
27+
//! raster (matching `RS_SetCRS`/`RS_SetSRID`); a null `nodata` value instead
28+
//! **clears** the addressed band's nodata — the raster is rebuilt with that band
29+
//! carrying no nodata sentinel, rather than nulling the whole row.
2830
//!
2931
//! An out-of-range band index is an error — unlike the getter, which returns
3032
//! NULL for a missing band. The asymmetry is deliberate: this op rewrites the
@@ -49,7 +51,7 @@ use datafusion_common::exec_err;
4951
use datafusion_expr::{ColumnarValue, Volatility};
5052
use sedona_expr::scalar_udf::{SedonaScalarKernel, SedonaScalarUDF};
5153
use sedona_raster::builder::{RasterBuilder, RasterOverrides};
52-
use sedona_raster::traits::{nodata_f64_to_bytes, BandOverrides, RasterRef};
54+
use sedona_raster::traits::{nodata_f64_to_bytes, BandOverrides, Override, RasterRef};
5355
use sedona_schema::datatypes::SedonaType;
5456
use sedona_schema::matchers::ArgMatcher;
5557

@@ -132,19 +134,24 @@ impl SedonaScalarKernel for RsSetBandNoDataValue {
132134
Some(bands) => Some(bands.value(i)),
133135
None => None,
134136
};
135-
if value_values.is_null(i) {
136-
return null_out(&mut builder);
137-
}
138-
set_band_nodata(&mut builder, raster, band, value_values.value(i))
137+
// A null `nodata` value is not a null output: it flows through as
138+
// `None` and clears the addressed band's nodata.
139+
let value = if value_values.is_null(i) {
140+
None
141+
} else {
142+
Some(value_values.value(i))
143+
};
144+
set_band_nodata(&mut builder, raster, band, value)
139145
})?;
140146

141147
executor.finish(Arc::new(builder.finish()?))
142148
}
143149
}
144150

145151
/// Copy `raster` into `builder`, overriding the addressed (1-based) band's
146-
/// nodata with `value` packed into that band's data type. Every other band is
147-
/// copied with its data shared and metadata inherited.
152+
/// nodata: `Some(value)` packs `value` into that band's data type, `None`
153+
/// clears the band's nodata. Every other band is copied with its data shared
154+
/// and metadata (including its own nodata) inherited.
148155
///
149156
/// `band` is `None` for the 2-argument form (no band given): it defaults to band
150157
/// 1 only when the raster is single-band, and errors on a multiband raster so a
@@ -153,7 +160,7 @@ fn set_band_nodata(
153160
builder: &mut RasterBuilder,
154161
raster: &dyn RasterRef,
155162
band: Option<i64>,
156-
value: f64,
163+
value: Option<f64>,
157164
) -> Result<()> {
158165
let num_bands = raster.num_bands();
159166
let band = match band {
@@ -178,16 +185,28 @@ fn set_band_nodata(
178185

179186
for band_idx in 0..num_bands {
180187
let band_ref = raster.band(band_idx)?;
181-
// Override the nodata only on the addressed band; others inherit.
182-
let nodata_bytes = if band_idx + 1 == band as usize {
183-
Some(nodata_f64_to_bytes(value, &band_ref.data_type())?)
188+
let addressed = band_idx + 1 == band as usize;
189+
// Pack the addressed band's new nodata bytes into a scratch binding the
190+
// override borrows from. A null value leaves this `None` so the override
191+
// clears the band's nodata rather than setting it.
192+
let new_nodata: Option<Vec<u8>> = match value {
193+
Some(v) if addressed => Some(nodata_f64_to_bytes(v, &band_ref.data_type())?),
194+
_ => None,
195+
};
196+
// Only the addressed band's nodata is touched (`Set` or `Clear`); every
197+
// other band keeps its own.
198+
let nodata = if addressed {
199+
match new_nodata.as_deref() {
200+
Some(bytes) => Override::Set(bytes),
201+
None => Override::Clear,
202+
}
184203
} else {
185-
None
204+
Override::Keep
186205
};
187206
band_ref.copy_into(
188207
builder,
189208
BandOverrides {
190-
nodata: nodata_bytes.as_deref(),
209+
nodata,
191210
..Default::default()
192211
},
193212
)?;
@@ -282,11 +301,35 @@ mod tests {
282301
}
283302

284303
#[test]
285-
fn null_value_nulls_raster() {
304+
fn null_value_clears_band_nodata() {
305+
// A null nodata value clears the addressed band's nodata rather than
306+
// nulling the whole raster: the single band starts with nodata 5 and
307+
// ends with none, every other field preserved.
308+
let one_band = RasterSpec::d2(2, 1).band_values(&[1u8, 2]).nodata(5u8);
286309
let result = tester_2arg()
287-
.invoke_array_scalar(Arc::new(two_band().build()), ScalarValue::Float64(None))
310+
.invoke_array_scalar(Arc::new(one_band.build()), ScalarValue::Float64(None))
288311
.unwrap();
289-
assert_rasters_equal(&result, &[None]);
312+
let expected = RasterSpec::d2(2, 1).band_values(&[1u8, 2]);
313+
assert_rasters_equal(&result, &[Some(expected)]);
314+
}
315+
316+
#[test]
317+
fn null_value_clears_only_the_addressed_band_nodata() {
318+
// 3-arg form on a multiband raster: clearing band 2's nodata leaves
319+
// band 1's nodata untouched.
320+
let input = RasterSpec::d2(2, 1)
321+
.band_values(&[1u8, 2])
322+
.nodata(7u8)
323+
.band_values(&[3u8, 4])
324+
.nodata(8u8);
325+
let result = tester_3arg()
326+
.invoke_array_scalar_scalar(Arc::new(input.build()), 2_i32, ScalarValue::Float64(None))
327+
.unwrap();
328+
let expected = RasterSpec::d2(2, 1)
329+
.band_values(&[1u8, 2])
330+
.nodata(7u8)
331+
.band_values(&[3u8, 4]);
332+
assert_rasters_equal(&result, &[Some(expected)]);
290333
}
291334

292335
#[test]

0 commit comments

Comments
 (0)