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/08/12 20:04:59 UTC
(sedona-db) branch main updated: feat(rust/sedona-raster): persist and read non-identity band views (#1113)
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 1245541bb feat(rust/sedona-raster): persist and read non-identity band views (#1113)
1245541bb is described below
commit 1245541bb3e24bb52c9f2e4df4b9fa23dead20cd
Author: James Willis <ja...@users.noreply.github.com>
AuthorDate: Wed Aug 12 13:04:53 2026 -0700
feat(rust/sedona-raster): persist and read non-identity band views (#1113)
---
Cargo.lock | 1 +
rust/sedona-raster/Cargo.toml | 1 +
rust/sedona-raster/src/array.rs | 732 ++++++++++++++++++++++++++++++---
rust/sedona-raster/src/builder.rs | 719 +++++++++++++++++++++++++++++++-
rust/sedona-raster/src/traits.rs | 123 ++++--
rust/sedona-raster/src/view_entries.rs | 64 +++
6 files changed, 1542 insertions(+), 98 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index d9eb662c0..5883b3146 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -6136,6 +6136,7 @@ dependencies = [
"approx",
"arrow-array",
"arrow-buffer",
+ "arrow-data",
"arrow-ipc",
"arrow-schema",
"async-trait",
diff --git a/rust/sedona-raster/Cargo.toml b/rust/sedona-raster/Cargo.toml
index f6b692429..a00022c32 100644
--- a/rust/sedona-raster/Cargo.toml
+++ b/rust/sedona-raster/Cargo.toml
@@ -42,5 +42,6 @@ sedona-schema = { workspace = true }
[dev-dependencies]
sedona-testing = { workspace = true }
approx = { workspace = true }
+arrow-data = { workspace = true }
arrow-ipc = { workspace = true }
tokio = { workspace = true, features = ["macros", "rt"] }
diff --git a/rust/sedona-raster/src/array.rs b/rust/sedona-raster/src/array.rs
index 482df92cd..ff90b55ed 100644
--- a/rust/sedona-raster/src/array.rs
+++ b/rust/sedona-raster/src/array.rs
@@ -27,14 +27,16 @@ use datafusion_common::cast::{
use crate::builder::RasterBuilder;
use crate::traits::{BandRef, NdBuffer, RasterRef};
-use crate::view_entries::ViewEntry;
-use sedona_schema::raster::{band_indices, raster_indices, BandDataType};
+use crate::view_entries::{ViewEntries, ViewEntry};
+use sedona_schema::raster::{band_indices, band_view_indices, raster_indices, BandDataType};
/// Arrow-backed implementation of BandRef for a single band within a raster.
///
-/// Today this handles only the canonical identity view: `view_entries` is
-/// synthesised from `source_shape`, `visible_shape == source_shape`,
-/// and `byte_strides` are plain C-order strides with `byte_offset = 0`.
+/// View-derived layout (`visible_shape`, `byte_strides`, `byte_offset`) is
+/// composed once at construction from the band's stored view (identity when
+/// the view row is null, otherwise the persisted `ViewEntry` list) and reused
+/// by every accessor. Source-shape and dim-name slices are borrowed directly
+/// from the underlying Arrow buffers.
struct BandRefImpl<'a> {
dim_names_list: &'a ListArray,
dim_names_values: &'a StringArray,
@@ -48,13 +50,17 @@ struct BandRefImpl<'a> {
band_row: usize,
/// Resolved at construction so accessors don't re-decode the discriminant.
data_type: BandDataType,
- /// Per-visible-axis view, length = ndim. Always identity today.
- view_entries: Vec<ViewEntry>,
- /// Visible shape, length = ndim. Equals `source_shape` today.
+ /// Per-visible-axis view, length = ndim.
+ view_entries: ViewEntries,
+ /// Visible shape (`[v.steps for v in view_entries]`), length = ndim.
visible_shape: Vec<i64>,
- /// Byte strides per visible axis. C-order over `source_shape` today.
+ /// Byte strides per visible axis. May be 0 (broadcast) or negative
+ /// (reverse iteration) for a non-identity view.
byte_strides: Vec<i64>,
/// Byte offset into `data` of the visible region's `[0,...,0]` element.
+ /// Non-negative by construction — `RasterRefImpl::band` composes it from
+ /// non-negative `start × source_stride` terms and rejects a negative or
+ /// overflowing result before the band is built.
byte_offset: u64,
}
@@ -82,7 +88,7 @@ impl<'a> BandRef for BandRefImpl<'a> {
}
fn view(&self) -> &[ViewEntry] {
- &self.view_entries
+ self.view_entries.as_slice()
}
fn data_type(&self) -> BandDataType {
@@ -116,9 +122,13 @@ impl<'a> BandRef for BandRefImpl<'a> {
fn is_indb(&self) -> bool {
// A 0-element visible region (any visible dim is 0) holds no readable
// bytes — trivially fully in-RAM — so it's InDb, not the OutDb
- // empty-`data` sentinel. Otherwise the discriminator is buffer presence.
- self.shape().iter().product::<i64>() == 0
- || !self.data_array.value(self.band_row).is_empty()
+ // empty-`data` sentinel. Test emptiness with `contains(&0)` rather than
+ // `Π shape`: a broadcast axis can push the visible element count past
+ // i64::MAX (e.g. a huge `time` axis), and the product would
+ // overflow-panic in debug / wrap in release. For non-negative shapes
+ // (guaranteed by `validate`) the two are equivalent. Otherwise the
+ // discriminator is buffer presence.
+ self.shape().contains(&0) || !self.data_array.value(self.band_row).is_empty()
}
fn nd_buffer(&self) -> Result<NdBuffer<'_>, ArrowError> {
@@ -177,6 +187,10 @@ pub struct RasterRefImpl<'a> {
band_datatype_array: &'a UInt32Array,
band_nodata_array: &'a BinaryArray,
band_view_list: &'a ListArray,
+ band_view_source_axis: &'a Int64Array,
+ band_view_start: &'a Int64Array,
+ band_view_step: &'a Int64Array,
+ band_view_steps: &'a Int64Array,
band_outdb_uri_array: &'a StringArray,
band_outdb_format_array: &'a StringViewArray,
band_data_array: &'a BinaryViewArray,
@@ -192,6 +206,202 @@ impl<'a> RasterRefImpl<'a> {
Some(self.crs_array.value(self.raster_index))
}
}
+
+ /// Read the band's stored view-entry list. Identity is encoded exclusively
+ /// as a NULL row and is synthesised as the canonical identity over
+ /// `source_shape`; a non-null row is decoded from the four parallel view
+ /// columns. An empty (non-null, zero-length) row is malformed and is
+ /// rejected downstream by [`ViewEntries::validate`].
+ fn read_band_view_entries(
+ &self,
+ band_row: usize,
+ source_shape: &[i64],
+ ) -> Result<ViewEntries, ArrowError> {
+ if self.band_view_list.is_null(band_row) {
+ return Ok(ViewEntries::identity_for_shape(source_shape));
+ }
+ let v_start = self.band_view_list.value_offsets()[band_row] as usize;
+ let v_end = self.band_view_list.value_offsets()[band_row + 1] as usize;
+ // The list offsets are authored by whoever wrote the Arrow array — for a
+ // view round-tripped in from another engine over IPC/FFI (validation is
+ // skipped on import), the four parallel child columns may be shorter
+ // than the offsets claim, or carry a null in a field the schema declares
+ // non-null. Either would panic or silently misread `.value(i)` below, so
+ // validate the child arrays against the offsets before indexing.
+ for (name, arr) in [
+ ("source_axis", self.band_view_source_axis),
+ ("start", self.band_view_start),
+ ("step", self.band_view_step),
+ ("steps", self.band_view_steps),
+ ] {
+ if v_end > arr.len() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "band {band_row}: view '{name}' child array has {} elements but the \
+ view list addresses up to {v_end}",
+ arr.len()
+ )));
+ }
+ if arr.null_count() > 0 && (v_start..v_end).any(|i| arr.is_null(i)) {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "band {band_row}: view '{name}' child array has a null in \
+ [{v_start}, {v_end}); view fields must be non-null"
+ )));
+ }
+ }
+ Ok(ViewEntries::new(
+ (v_start..v_end)
+ .map(|i| ViewEntry {
+ source_axis: self.band_view_source_axis.value(i),
+ start: self.band_view_start.value(i),
+ step: self.band_view_step.value(i),
+ steps: self.band_view_steps.value(i),
+ })
+ .collect(),
+ ))
+ }
+}
+
+/// Compose a validated view against a source shape into C-order byte strides
+/// and a byte offset.
+///
+/// C-order source strides are dtype-scaled cumulative products of
+/// `source_shape`; each visible axis's byte stride is `view.step ×
+/// src_stride` and its offset contribution is `view.start × src_stride`. All
+/// arithmetic is checked: even after [`ViewEntries::validate`], the cumulative
+/// byte product can overflow `i64` for cosmically large shapes, and a corrupt
+/// `source_shape` whose product wraps would otherwise silently pass the
+/// downstream bound check. The returned `byte_offset` is non-negative by
+/// construction (`start >= 0`, `src_stride > 0`); the defensive sign check
+/// guards a future refactor before we cross the `i64` → `u64` boundary in
+/// `band()`.
+fn compose_byte_strides(
+ band_row: usize,
+ source_shape: &[i64],
+ view_entries: &ViewEntries,
+ dtype_byte_size: usize,
+) -> Result<(Vec<i64>, i64), ArrowError> {
+ let overflow_err = |msg: &str| {
+ ArrowError::ExternalError(Box::new(sedona_common::sedona_internal_datafusion_err!(
+ "band {band_row}: {msg}"
+ )))
+ };
+
+ let dtype_size = dtype_byte_size as i64;
+
+ let mut source_strides_bytes = vec![0i64; source_shape.len()];
+ source_strides_bytes[source_shape.len() - 1] = dtype_size;
+ for k in (0..source_shape.len() - 1).rev() {
+ source_strides_bytes[k] = source_strides_bytes[k + 1]
+ .checked_mul(source_shape[k + 1])
+ .ok_or_else(|| overflow_err("source-stride product overflows i64"))?;
+ }
+
+ let mut byte_strides = vec![0i64; view_entries.len()];
+ let mut byte_offset: i64 = 0;
+ for (k, v) in view_entries.iter().enumerate() {
+ let src_stride = source_strides_bytes[v.source_axis as usize];
+ byte_strides[k] = v
+ .step
+ .checked_mul(src_stride)
+ .ok_or_else(|| overflow_err("view step × source-stride overflows i64"))?;
+ let start_off = v
+ .start
+ .checked_mul(src_stride)
+ .ok_or_else(|| overflow_err("view start × source-stride overflows i64"))?;
+ byte_offset = byte_offset
+ .checked_add(start_off)
+ .ok_or_else(|| overflow_err("view offset accumulation overflows i64"))?;
+ }
+
+ if byte_offset < 0 {
+ return Err(overflow_err("composed byte_offset is negative"));
+ }
+
+ Ok((byte_strides, byte_offset))
+}
+
+/// Verify that every byte the view can address lies within `buffer_len` and
+/// that every stride × index product (and their accumulations) fits in i64.
+///
+/// **Load-bearing**: this is the *only* bound check between the view's
+/// byte-stride description and the data buffer. Stride-aware consumers walk
+/// the buffer with plain-arithmetic indexing and rely on this precheck having
+/// proven every addressed byte is in range. Two corruption modes it catches:
+///
+/// 1. A writer that lies about `source_shape` (Arrow column shorter than the
+/// view promises).
+/// 2. A composed view whose stride × index product or accumulated offset
+/// overflows i64 even though `validate` accepted the per-entry bounds.
+///
+/// Empty visible regions (any axis with `steps == 0`) address no bytes and
+/// skip the check.
+fn check_view_buffer_bounds(
+ buffer_len: usize,
+ visible_shape: &[i64],
+ byte_strides: &[i64],
+ byte_offset: i64,
+ dtype_size: usize,
+) -> Result<(), ArrowError> {
+ if visible_shape.contains(&0) {
+ // An empty visible region addresses no elements, but the composed
+ // `byte_offset` (from a large `start` on a non-empty axis) must still
+ // stay within the buffer so the `NdBuffer.offset <= buffer.len()`
+ // invariant always holds.
+ let buffer_len_i64 = i64::try_from(buffer_len).map_err(|_| {
+ ArrowError::InvalidArgumentError(format!("buffer length {buffer_len} exceeds i64::MAX"))
+ })?;
+ if byte_offset > buffer_len_i64 {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "view byte offset {byte_offset} exceeds buffer length {buffer_len} \
+ for an empty visible region"
+ )));
+ }
+ return Ok(());
+ }
+ let mut min_offset = byte_offset;
+ let mut max_offset = byte_offset;
+ for (k, &stride) in byte_strides.iter().enumerate() {
+ // `validate` guarantees `steps >= 0`, so `visible_shape[k] - 1` is
+ // in-range for any non-empty axis.
+ let last_idx = visible_shape[k] - 1;
+ let contribution = last_idx.checked_mul(stride).ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "max addressable offset on axis {k} overflows i64"
+ ))
+ })?;
+ if contribution > 0 {
+ max_offset = max_offset.checked_add(contribution).ok_or_else(|| {
+ ArrowError::InvalidArgumentError(
+ "max addressable offset accumulation overflows i64".to_string(),
+ )
+ })?;
+ } else if contribution < 0 {
+ min_offset = min_offset.checked_add(contribution).ok_or_else(|| {
+ ArrowError::InvalidArgumentError(
+ "min addressable offset accumulation overflows i64".to_string(),
+ )
+ })?;
+ }
+ }
+ let last_byte = max_offset
+ .checked_add(dtype_size as i64 - 1)
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError("max addressable byte overflows i64".to_string())
+ })?;
+ if min_offset < 0 {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "view addresses out-of-bounds negative byte offset {min_offset}"
+ )));
+ }
+ let buffer_len_i64 = i64::try_from(buffer_len).map_err(|_| {
+ ArrowError::InvalidArgumentError(format!("buffer length {buffer_len} exceeds i64::MAX"))
+ })?;
+ if last_byte >= buffer_len_i64 {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "view addresses byte {last_byte} but buffer is only {buffer_len} bytes"
+ )));
+ }
+ Ok(())
}
impl<'a> RasterRef for RasterRefImpl<'a> {
@@ -234,43 +444,76 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
)))
})?;
- // Only the canonical identity view (null view row) is written today.
- // A non-null view row would require the view → byte-stride composition
- // path, which is not yet implemented. Surface it loudly here rather
- // than silently rejecting the band, so callers see the standardised
- // SedonaDB-internal-error framing.
- //
- // This rejection is also the guardrail keeping `RS_EnsureLoaded`
- // correct: it drops `view()` on rebuild, so it would corrupt a
- // viewed band. When this comes off (view composition), the loader
- // request/response must round-trip the view — tracked in
- // <https://github.com/apache/sedona-db/issues/897>.
- if !self.band_view_list.is_null(band_row) {
- return Err(ArrowError::ExternalError(Box::new(
- sedona_common::sedona_internal_datafusion_err!(
- "non-null view row at band {band_row}: view composition is not yet implemented"
- ),
- )));
- }
- let view_entries: Vec<ViewEntry> = source_shape
- .iter()
- .enumerate()
- .map(|(i, &s)| ViewEntry {
- source_axis: i as i64,
- start: 0,
- step: 1,
- steps: s,
- })
- .collect();
-
- let visible_shape: Vec<i64> = source_shape.to_vec();
+ // Read the band's view (identity when the row is null, otherwise the
+ // persisted ViewEntry list) and validate it against the source shape —
+ // a malformed or corrupt view surfaces loudly rather than mislocating
+ // bytes.
+ let view_entries = self.read_band_view_entries(band_row, source_shape)?;
+ view_entries.validate(source_shape).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_common::sedona_internal_datafusion_err!(
+ "band {band_row} has malformed view: {e}"
+ )))
+ })?;
- let dtype_size = data_type.byte_size() as i64;
- let mut byte_strides = vec![0i64; source_shape.len()];
- byte_strides[source_shape.len() - 1] = dtype_size;
- for k in (0..source_shape.len() - 1).rev() {
- byte_strides[k] = byte_strides[k + 1] * source_shape[k + 1];
- }
+ // The visible shape is view-derived and needed for every band. The
+ // InDb C-order byte-stride layout, by contrast, is only meaningful for
+ // InDb bands: an OutDb band (empty `data`, non-empty visible region)
+ // never dereferences its strides — `nd_buffer()` errors for it — so we
+ // skip composing them. That also lets an OutDb band whose described
+ // `source_shape` has a stride product exceeding i64 read its metadata
+ // without tripping the InDb source-stride overflow guard.
+ let visible_shape = view_entries.visible_shape();
+ let data_bytes = self.band_data_array.value(band_row);
+
+ // Mirror `BandRef::is_indb`: an empty visible region or non-empty data
+ // buffer is InDb; empty data with a non-empty visible region is OutDb.
+ let is_indb = visible_shape.contains(&0) || !data_bytes.is_empty();
+
+ let (byte_strides, byte_offset) = if is_indb {
+ // Compose the view onto the source's natural C-order byte strides to
+ // get per-axis byte strides (0 for broadcast, negative for reverse)
+ // and the byte offset of element [0,...,0].
+ let (byte_strides, byte_offset_i64) =
+ compose_byte_strides(band_row, source_shape, &view_entries, data_type.byte_size())?;
+
+ // Verify the data column is long enough to cover every byte the view
+ // can address. The view-machinery validation above doesn't know the
+ // actual `data` BinaryView length — a writer that lies about
+ // source_shape vs the bytes written would otherwise slip through and
+ // panic later when a consumer walks the strided buffer. Skipped when
+ // there are no bytes (an empty visible region).
+ if !data_bytes.is_empty() {
+ check_view_buffer_bounds(
+ data_bytes.len(),
+ &visible_shape,
+ &byte_strides,
+ byte_offset_i64,
+ data_type.byte_size(),
+ )
+ .map_err(|e| {
+ ArrowError::ExternalError(Box::new(
+ sedona_common::sedona_internal_datafusion_err!(
+ "band {band_row}: view-buffer bounds check failed: {e}"
+ ),
+ ))
+ })?;
+ }
+
+ // `compose_byte_strides` guarantees a non-negative offset; cross into
+ // `u64` for storage with a checked conversion that upholds that at
+ // the boundary.
+ let byte_offset = u64::try_from(byte_offset_i64).map_err(|_| {
+ ArrowError::ExternalError(Box::new(sedona_common::sedona_internal_datafusion_err!(
+ "band {band_row}: composed byte_offset {byte_offset_i64} is negative"
+ )))
+ })?;
+ (byte_strides, byte_offset)
+ } else {
+ // OutDb: the byte strides/offset are never read (`nd_buffer()`
+ // errors for OutDb bands), so leave them zeroed rather than compute
+ // an InDb layout for bytes that live elsewhere.
+ (vec![0i64; view_entries.len()], 0u64)
+ };
Ok(Box::new(BandRefImpl {
dim_names_list: self.band_dim_names_list,
@@ -286,7 +529,7 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
view_entries,
visible_shape,
byte_strides,
- byte_offset: 0,
+ byte_offset,
}))
}
@@ -408,6 +651,10 @@ pub struct RasterStructArray<'a> {
band_datatype_array: &'a UInt32Array,
band_nodata_array: &'a BinaryArray,
band_view_list: &'a ListArray,
+ band_view_source_axis: &'a Int64Array,
+ band_view_start: &'a Int64Array,
+ band_view_step: &'a Int64Array,
+ band_view_steps: &'a Int64Array,
band_outdb_uri_array: &'a StringArray,
band_outdb_format_array: &'a StringViewArray,
band_data_array: &'a BinaryViewArray,
@@ -454,6 +701,12 @@ impl<'a> RasterStructArray<'a> {
let band_datatype_array = as_uint32_array(bands_struct.column(band_indices::DATA_TYPE))?;
let band_nodata_array = as_binary_array(bands_struct.column(band_indices::NODATA))?;
let band_view_list = as_list_array(bands_struct.column(band_indices::VIEW))?;
+ let band_view_struct = as_struct_array(band_view_list.values())?;
+ let band_view_source_axis =
+ as_int64_array(band_view_struct.column(band_view_indices::SOURCE_AXIS))?;
+ let band_view_start = as_int64_array(band_view_struct.column(band_view_indices::START))?;
+ let band_view_step = as_int64_array(band_view_struct.column(band_view_indices::STEP))?;
+ let band_view_steps = as_int64_array(band_view_struct.column(band_view_indices::STEPS))?;
let band_outdb_uri_array = as_string_array(bands_struct.column(band_indices::OUTDB_URI))?;
let band_outdb_format_array =
as_string_view_array(bands_struct.column(band_indices::OUTDB_FORMAT))?;
@@ -477,6 +730,10 @@ impl<'a> RasterStructArray<'a> {
band_datatype_array,
band_nodata_array,
band_view_list,
+ band_view_source_axis,
+ band_view_start,
+ band_view_step,
+ band_view_steps,
band_outdb_uri_array,
band_outdb_format_array,
band_data_array,
@@ -520,6 +777,10 @@ impl<'a> RasterStructArray<'a> {
band_datatype_array: self.band_datatype_array,
band_nodata_array: self.band_nodata_array,
band_view_list: self.band_view_list,
+ band_view_source_axis: self.band_view_source_axis,
+ band_view_start: self.band_view_start,
+ band_view_step: self.band_view_step,
+ band_view_steps: self.band_view_steps,
band_outdb_uri_array: self.band_outdb_uri_array,
band_outdb_format_array: self.band_outdb_format_array,
band_data_array: self.band_data_array,
@@ -556,8 +817,8 @@ mod tests {
use crate::builder::{RasterBuilder, StartBandArgs};
use crate::traits::BandOverrides;
use arrow_array::{ArrayRef, ListArray, StructArray, UInt32Array};
- use arrow_buffer::{OffsetBuffer, ScalarBuffer};
- use arrow_schema::{DataType, Fields};
+ use arrow_buffer::{NullBuffer, OffsetBuffer, ScalarBuffer};
+ use arrow_schema::{DataType, Field, Fields};
use sedona_schema::raster::{band_indices, raster_indices, BandDataType, RasterSchema};
use sedona_testing::rasters::generate_test_rasters;
use std::sync::Arc;
@@ -907,6 +1168,218 @@ mod tests {
assert!(err.to_string().contains("empty source_shape"));
}
+ // ---- Read path: non-identity view decoding + corruption rejection ----
+
+ /// Build a single-raster, single-band StructArray carrying an explicit,
+ /// non-identity view (`start=1, step=2, steps=3` over an 8-byte source).
+ /// The corruption tests replace one band-level column of this baseline.
+ fn build_explicit_view_raster() -> StructArray {
+ let mut builder = RasterBuilder::new(1);
+ let transform = [0.0, 1.0, 0.0, 0.0, 0.0, -1.0];
+ builder
+ .start_raster_nd(&transform, &["x"], &[3], None)
+ .unwrap();
+ builder
+ .start_band(StartBandArgs {
+ view: Some(&[ViewEntry {
+ source_axis: 0,
+ start: 1,
+ step: 2,
+ steps: 3,
+ }]),
+ ..StartBandArgs::new(&["x"], &[8], BandDataType::UInt8)
+ })
+ .unwrap();
+ builder
+ .band_data_writer()
+ .append_value(vec![0u8, 1, 2, 3, 4, 5, 6, 7]);
+ builder.finish_band().unwrap();
+ builder.finish_raster().unwrap();
+ builder.finish().unwrap()
+ }
+
+ /// Rebuild the band view list from hand-rolled entries. `entries[i]`
+ /// supplies all four `(source_axis, start, step, steps)` values for band
+ /// row `i`; `nulls` controls per-row validity (`None` → every row non-null).
+ fn make_band_view_list(
+ entries: Vec<Vec<(i64, i64, i64, i64)>>,
+ nulls: Option<Vec<bool>>,
+ ) -> ArrayRef {
+ let mut offsets: Vec<i32> = vec![0];
+ let (mut sa, mut start, mut step, mut steps) = (vec![], vec![], vec![], vec![]);
+ for row in &entries {
+ for &(a, s, k, n) in row {
+ sa.push(a);
+ start.push(s);
+ step.push(k);
+ steps.push(n);
+ }
+ offsets.push(sa.len() as i32);
+ }
+ let view_struct_fields = Fields::from(vec![
+ Field::new("source_axis", DataType::Int64, false),
+ Field::new("start", DataType::Int64, false),
+ Field::new("step", DataType::Int64, false),
+ Field::new("steps", DataType::Int64, false),
+ ]);
+ let view_struct = StructArray::new(
+ view_struct_fields,
+ vec![
+ Arc::new(Int64Array::from(sa)) as ArrayRef,
+ Arc::new(Int64Array::from(start)) as ArrayRef,
+ Arc::new(Int64Array::from(step)) as ArrayRef,
+ Arc::new(Int64Array::from(steps)) as ArrayRef,
+ ],
+ None,
+ );
+ let DataType::List(view_field) = RasterSchema::view_type() else {
+ unreachable!()
+ };
+ Arc::new(ListArray::new(
+ view_field,
+ OffsetBuffer::new(ScalarBuffer::from(offsets)),
+ Arc::new(view_struct),
+ nulls.map(NullBuffer::from),
+ ))
+ }
+
+ /// Build a band source_shape list with hand-rolled i64 entries so tests can
+ /// inject values the builder's writer-side checks would refuse.
+ fn make_band_source_shape_list(rows: Vec<Vec<i64>>) -> ArrayRef {
+ let mut offsets: Vec<i32> = vec![0];
+ let mut values: Vec<i64> = vec![];
+ for row in &rows {
+ values.extend_from_slice(row);
+ offsets.push(values.len() as i32);
+ }
+ let DataType::List(field) = RasterSchema::source_shape_type() else {
+ unreachable!()
+ };
+ Arc::new(ListArray::new(
+ field,
+ OffsetBuffer::new(ScalarBuffer::from(offsets)),
+ Arc::new(Int64Array::from(values)),
+ None,
+ ))
+ }
+
+ #[test]
+ fn band_decodes_explicit_view_end_to_end() {
+ // The baseline non-identity fixture must decode to the right visible
+ // shape, strides, and offset — the read path's happy case for a
+ // persisted view read out of the Arrow columns.
+ let array = build_explicit_view_raster();
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let r = rasters.get(0).unwrap();
+ let band = r.band(0).unwrap();
+ assert_eq!(band.shape(), &[3]);
+ assert_eq!(band.raw_source_shape(), &[8]);
+ let buf = band.nd_buffer().unwrap();
+ assert_eq!(buf.strides, &[2]);
+ assert_eq!(buf.offset, 1);
+ assert!(!buf.is_contiguous());
+ }
+
+ #[test]
+ fn band_errors_when_view_length_mismatches_source_shape() {
+ // source_shape has 1 dim but the view encodes 2 entries.
+ let array = build_explicit_view_raster();
+ let bad_view = make_band_view_list(vec![vec![(0, 0, 1, 3), (0, 0, 1, 3)]], None);
+ let mutated = replace_band_column(&array, band_indices::VIEW, bad_view);
+ let rasters = RasterStructArray::try_new(&mutated).unwrap();
+ let err = rasters.get(0).unwrap().band(0).err().unwrap();
+ assert!(err.to_string().contains("malformed view"), "got: {err}");
+ }
+
+ #[test]
+ fn band_rejects_empty_non_null_view_row() {
+ // Identity is encoded exclusively as a NULL row; a non-null zero-length
+ // list is malformed and must error rather than fall back to identity.
+ let array = build_explicit_view_raster();
+ let empty_non_null_view = make_band_view_list(vec![vec![]], Some(vec![true]));
+ let mutated = replace_band_column(&array, band_indices::VIEW, empty_non_null_view);
+ let rasters = RasterStructArray::try_new(&mutated).unwrap();
+ let err = rasters.get(0).unwrap().band(0).err().unwrap();
+ assert!(err.to_string().contains("view length"), "got: {err}");
+ }
+
+ #[test]
+ fn band_errors_when_data_column_shorter_than_view() {
+ // Inflate source_shape to [16] and the view to steps=10 along it: the
+ // addressed byte range (0..10) jumps past the actual 8-byte data
+ // column, so the InDb bounds precheck must fire.
+ let array = build_explicit_view_raster();
+ let new_source_shape = make_band_source_shape_list(vec![vec![16i64]]);
+ let mutated_ss = replace_band_column(&array, band_indices::SOURCE_SHAPE, new_source_shape);
+ let new_view = make_band_view_list(vec![vec![(0, 0, 1, 10)]], None);
+ let mutated = replace_band_column(&mutated_ss, band_indices::VIEW, new_view);
+ let rasters = RasterStructArray::try_new(&mutated).unwrap();
+ let err = rasters.get(0).unwrap().band(0).err().unwrap();
+ assert!(err.to_string().contains("SedonaDB internal error"));
+ assert!(err.to_string().contains("view-buffer bounds check failed"));
+ }
+
+ #[test]
+ fn band_errors_when_source_stride_product_overflows() {
+ // dtype_size × Π source_shape[j>k] must not silently wrap. A 3-D source
+ // shape of [1, 1<<32, 1<<32] makes (1<<32) × (1<<32) = 1<<64 overflow
+ // i64 during the source-stride build.
+ let array = build_explicit_view_raster();
+ let new_source_shape =
+ make_band_source_shape_list(vec![vec![1i64, 1i64 << 32, 1i64 << 32]]);
+ let mutated_ss = replace_band_column(&array, band_indices::SOURCE_SHAPE, new_source_shape);
+ // steps=0 on the giant axes keeps validate's start/last checks trivial.
+ let new_view =
+ make_band_view_list(vec![vec![(0, 0, 1, 1), (1, 0, 1, 0), (2, 0, 1, 0)]], None);
+ let mutated = replace_band_column(&mutated_ss, band_indices::VIEW, new_view);
+ let rasters = RasterStructArray::try_new(&mutated).unwrap();
+ assert!(rasters.get(0).unwrap().band(0).is_err());
+ }
+
+ #[test]
+ fn fast_paths_return_columnar_values_when_band_view_is_corrupt() {
+ // band(i) validates the view and errors on a malformed one; the
+ // columnar fast paths read their fields directly without consulting the
+ // view. Pin the contract so a future reader doesn't accidentally couple
+ // them.
+ let mut builder = RasterBuilder::new(1);
+ let transform = [0.0, 1.0, 0.0, 0.0, 0.0, -1.0];
+ builder
+ .start_raster_nd(&transform, &["x"], &[3], None)
+ .unwrap();
+ builder
+ .start_band(StartBandArgs {
+ name: Some("a"),
+ view: Some(&[ViewEntry {
+ source_axis: 0,
+ start: 1,
+ step: 2,
+ steps: 3,
+ }]),
+ nodata: Some(&[0u8, 0, 0, 0]),
+ outdb_uri: Some("s3://bucket/a.tif"),
+ outdb_format: Some("GTiff"),
+ ..StartBandArgs::new(&["x"], &[8], BandDataType::UInt32)
+ })
+ .unwrap();
+ builder.band_data_writer().append_value(vec![0u8; 32]);
+ builder.finish_band().unwrap();
+ builder.finish_raster().unwrap();
+ let array = builder.finish().unwrap();
+
+ // Corrupt the view with a negative steps value.
+ let bad_view = make_band_view_list(vec![vec![(0, 0, 1, -1)]], None);
+ let mutated = replace_band_column(&array, band_indices::VIEW, bad_view);
+ let rasters = RasterStructArray::try_new(&mutated).unwrap();
+ let r = rasters.get(0).unwrap();
+
+ assert!(r.band(0).is_err());
+ assert_eq!(r.band_data_type(0), Some(BandDataType::UInt32));
+ assert_eq!(r.band_outdb_uri(0), Some("s3://bucket/a.tif"));
+ assert_eq!(r.band_outdb_format(0), Some("GTiff"));
+ assert_eq!(r.band_nodata(0), Some(&[0u8, 0, 0, 0][..]));
+ }
+
// direct fast-path tests
#[test]
@@ -1181,4 +1654,159 @@ mod tests {
"a 0-element band holds 0 bytes legitimately and must be InDb"
);
}
+
+ #[test]
+ fn is_indb_does_not_overflow_on_broadcast_axis_exceeding_i64_max() {
+ // A broadcast axis with huge `steps` can push the visible element count
+ // past i64::MAX. `is_indb()` must classify by buffer presence without
+ // computing `Π shape` (which would overflow-panic in debug / wrap in
+ // release), and `nd_buffer()` must not panic either.
+ //
+ // Visible shape = [4, 2^62] → product 2^64 overflows i64. The giant
+ // axis is a broadcast (step=0) over a size-1 source axis, so it
+ // addresses one byte per position and the 4-byte buffer suffices.
+ let big: i64 = 1 << 62;
+ let mut builder = RasterBuilder::new(1);
+ let transform = [0.0, 1.0, 0.0, 0.0, 0.0, -1.0];
+ builder
+ .start_raster_nd(&transform, &["y", "t"], &[4, big], None)
+ .unwrap();
+ builder
+ .start_band(StartBandArgs {
+ name: Some("broadcast_time"),
+ view: Some(&[
+ ViewEntry {
+ source_axis: 0,
+ start: 0,
+ step: 1,
+ steps: 4,
+ },
+ ViewEntry {
+ source_axis: 1,
+ start: 0,
+ step: 0,
+ steps: big,
+ },
+ ]),
+ ..StartBandArgs::new(&["y", "t"], &[4, 1], BandDataType::UInt8)
+ })
+ .unwrap();
+ builder.band_data_writer().append_value(vec![0u8, 1, 2, 3]);
+ builder.finish_band().unwrap();
+ builder.finish_raster().unwrap();
+ let array = builder.finish().unwrap();
+
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let raster = rasters.get(0).unwrap();
+ let band = raster.band(0).unwrap();
+ assert_eq!(band.shape(), &[4, big]);
+ assert!(
+ band.is_indb(),
+ "a band with bytes is InDb and must not overflow the product"
+ );
+ assert!(
+ band.nd_buffer().is_ok(),
+ "nd_buffer() must not panic on a broadcast axis"
+ );
+ }
+
+ #[test]
+ fn band_errors_when_view_child_arrays_shorter_than_offsets() {
+ // Round-tripping the view columns in from another engine over IPC/FFI
+ // (validation skipped on import) can yield a list whose offsets claim
+ // more entries than the child struct actually holds. Reading it must
+ // surface an error, not panic in `.value(i)`.
+ use arrow_data::ArrayData;
+ let array = build_explicit_view_raster();
+
+ // A valid length-1 view struct, but list offsets [0, 3] claiming three
+ // entries. Built unchecked because the safe constructors reject it.
+ let valid = make_band_view_list(vec![vec![(0, 0, 1, 3)]], None);
+ let valid_list = valid.as_any().downcast_ref::<ListArray>().unwrap();
+ let struct_values = valid_list.values().to_data();
+ let DataType::List(view_field) = RasterSchema::view_type() else {
+ unreachable!()
+ };
+ let over_offsets = arrow_buffer::Buffer::from_slice_ref([0i32, 3i32]);
+ // SAFETY: deliberately building an invalid array (offsets over-run the
+ // child) to exercise the read-path guard; nothing dereferences it
+ // beyond the guarded accessor under test.
+ let list_data = unsafe {
+ ArrayData::builder(DataType::List(view_field))
+ .len(1)
+ .add_buffer(over_offsets)
+ .add_child_data(struct_values)
+ .build_unchecked()
+ };
+ let bad_view: ArrayRef = Arc::new(ListArray::from(list_data));
+ let mutated = replace_band_column(&array, band_indices::VIEW, bad_view);
+ let rasters = RasterStructArray::try_new(&mutated).unwrap();
+ let err = rasters.get(0).unwrap().band(0).err().unwrap();
+ assert!(
+ err.to_string().contains("child array") && err.to_string().contains("addresses up to"),
+ "got: {err}"
+ );
+ }
+
+ #[test]
+ fn outdb_band_with_large_source_shape_reads_metadata() {
+ // An OutDb band never dereferences InDb byte strides, so a described
+ // source_shape whose C-order stride product overflows i64 must not
+ // block reading the band's metadata. Before the OutDb read fast-path,
+ // band() always composed strides and tripped the source-stride overflow
+ // guard even though the bytes live elsewhere.
+ //
+ // source_shape [1, 2^32, 2^32]: the C-order stride of axis 0 is
+ // 2^32 × 2^32 = 2^64, which overflows i64.
+ let big: i64 = 1 << 32;
+ let mut builder = RasterBuilder::new(1);
+ let transform = [0.0, 1.0, 0.0, 0.0, 0.0, -1.0];
+ builder
+ .start_raster_nd(&transform, &["y", "x"], &[big, big], None)
+ .unwrap();
+ builder
+ .start_band(StartBandArgs {
+ name: Some("external"),
+ nodata: Some(&[7u8]),
+ outdb_uri: Some("s3://bucket/huge.tif#band=1"),
+ outdb_format: Some("geotiff"),
+ ..StartBandArgs::new(&["z", "y", "x"], &[1, big, big], BandDataType::UInt8)
+ })
+ .unwrap();
+ builder.band_data_writer().append_value([]); // empty → OutDb
+ builder.finish_band().unwrap();
+ builder.finish_raster().unwrap();
+ let array = builder.finish().unwrap();
+
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let raster = rasters.get(0).unwrap();
+ let band = raster.band(0).unwrap();
+ assert!(
+ !band.is_indb(),
+ "empty data + non-empty visible region → OutDb"
+ );
+ assert_eq!(band.shape(), &[1, big, big]);
+ assert_eq!(band.nodata(), Some(&[7u8][..]));
+ assert_eq!(band.outdb_uri(), Some("s3://bucket/huge.tif#band=1"));
+ }
+
+ #[test]
+ fn band_errors_when_empty_region_offset_escapes_buffer() {
+ // An empty visible region (steps=0) addresses no elements, but a large
+ // `start` still composes a byte_offset. That offset must stay within
+ // the data buffer so the NdBuffer.offset invariant holds — a view whose
+ // offset runs past the buffer end must error even though it's empty.
+ let array = build_explicit_view_raster(); // source_shape [8], 8 data bytes
+ // start=100 with steps=0: validate skips the start bound for empty axes,
+ // so this composes byte_offset=100 over the 8-byte buffer.
+ let escaping_view = make_band_view_list(vec![vec![(0, 100, 1, 0)]], None);
+ let mutated = replace_band_column(&array, band_indices::VIEW, escaping_view);
+ let rasters = RasterStructArray::try_new(&mutated).unwrap();
+ let err = rasters.get(0).unwrap().band(0).err().unwrap();
+ assert!(
+ err.to_string().contains("view-buffer bounds check failed")
+ && err.to_string().contains("exceeds buffer length"),
+ "got: {err}"
+ );
+ }
}
diff --git a/rust/sedona-raster/src/builder.rs b/rust/sedona-raster/src/builder.rs
index 76782f9c8..6ce5d5283 100644
--- a/rust/sedona-raster/src/builder.rs
+++ b/rust/sedona-raster/src/builder.rs
@@ -29,7 +29,7 @@ use std::sync::Arc;
use sedona_schema::raster::{BandDataType, RasterSchema};
-use crate::traits::{BandOverrides, RasterRef};
+use crate::traits::{BandOverrides, BandRef, RasterRef};
use crate::view_entries::{ViewEntries, ViewEntry};
/// Raster-level metadata overrides for [`RasterBuilder::start_raster_from`] and
@@ -177,6 +177,22 @@ impl<'a> StartBandArgs<'a> {
}
}
+/// Arguments to [`RasterBuilder::with_view`]. Mirrors
+/// [`StartBandArgs`] minus the two fields `with_view` derives from
+/// `input` (`source_shape` from `input.raw_source_shape()`, `data_type` from
+/// `input.data_type()`) — accepting those from the caller would let them
+/// contradict `input`. `view` here is a *delta* composed against
+/// `input.view()`, not the absolute view stored on the band.
+pub struct WithViewArgs<'a> {
+ pub name: Option<&'a str>,
+ pub dim_names: &'a [&'a str],
+ pub input: &'a dyn BandRef,
+ pub view: &'a [ViewEntry],
+ pub nodata: Option<&'a [u8]>,
+ pub outdb_uri: Option<&'a str>,
+ pub outdb_format: Option<&'a str>,
+}
+
impl RasterBuilder {
/// Create a new raster builder with the specified capacity.
pub fn new(capacity: usize) -> Self {
@@ -368,10 +384,12 @@ impl RasterBuilder {
/// `view` is a per-axis window of offsets/steps over `source_shape`.
/// `None` is the canonical identity view — the whole source buffer in C
/// order — encoded as the identity null sentinel. A `Some` view is
- /// validated against `source_shape`; the band schema stores a view only as
- /// that identity null sentinel, so an identity `Some` view is stored the
- /// same as `None` and a non-identity view is rejected. View persistence is
- /// tracked in <https://github.com/apache/sedona-db/issues/897>.
+ /// validated against `source_shape`; an identity `Some` view is stored the
+ /// same as `None` (the null sentinel), while a non-identity view (a slice,
+ /// broadcast, permutation, or reverse) is persisted explicitly into the
+ /// band's four parallel view columns. In that case the `shape` column holds
+ /// the *source* shape and the visible shape is derived from the view on
+ /// read; the source bytes are carried over unchanged.
pub fn start_band(&mut self, args: StartBandArgs<'_>) -> Result<(), ArrowError> {
let StartBandArgs {
name,
@@ -384,10 +402,11 @@ impl RasterBuilder {
outdb_format,
} = args;
- // A caller-supplied view is validated against source_shape. The schema
- // stores views only as the identity null sentinel today, so a
- // non-identity view can't round-trip — reject it up front, before any
- // column append, rather than persisting mislocated bytes.
+ // A caller-supplied view is validated against source_shape. An identity
+ // view falls through to the null-sentinel storage path below (identical
+ // to `None`) so downstream readers agree on the canonical
+ // representation; a genuinely non-identity view is persisted explicitly
+ // into the band's four parallel view columns here.
if let Some(view) = view {
let ndim = dim_names.len();
if ndim == 0 {
@@ -406,13 +425,70 @@ impl RasterBuilder {
}
let view_entries = ViewEntries::new(view.to_vec());
view_entries.validate(source_shape)?;
+
if !view_entries.is_identity(source_shape) {
- return Err(ArrowError::InvalidArgumentError(
- "start_band: persisting a non-identity band view is not yet \
- supported (see https://github.com/apache/sedona-db/issues/897); \
- materialize the band (e.g. via RS_EnsureContiguous) first"
- .into(),
+ // -- Non-identity view: persist the explicit ViewEntry list. --
+ match name {
+ Some(n) => self.band_name.append_value(n),
+ None => self.band_name.append_null(),
+ }
+
+ for dn in dim_names {
+ self.band_dim_names_values.append_value(dn);
+ }
+ let next = *self.band_dim_names_offsets.last().unwrap() + ndim as i32;
+ self.band_dim_names_offsets.push(next);
+
+ // The `shape` column stores the *source* shape; the visible
+ // shape is derived from the view at read time.
+ for &s in source_shape {
+ self.band_shape_values.append_value(s);
+ }
+ let next = *self.band_shape_offsets.last().unwrap() + ndim as i32;
+ self.band_shape_offsets.push(next);
+
+ self.band_datatype.append_value(data_type as u32);
+
+ match nodata {
+ Some(b) => self.band_nodata.append_value(b),
+ None => self.band_nodata.append_null(),
+ }
+
+ // VIEW: one entry per visible axis written into the four
+ // parallel columns, offset advanced by the entry count,
+ // validity bit set — the mirror of the null-sentinel path below
+ // (validity false, offset unchanged).
+ for v in view {
+ self.band_view_source_axis_values
+ .append_value(v.source_axis);
+ self.band_view_start_values.append_value(v.start);
+ self.band_view_step_values.append_value(v.step);
+ self.band_view_steps_values.append_value(v.steps);
+ }
+ let next = *self.band_view_offsets.last().unwrap() + ndim as i32;
+ self.band_view_offsets.push(next);
+ self.band_view_validity.push(true);
+
+ match outdb_uri {
+ Some(uri) => self.band_outdb_uri.append_value(uri),
+ None => self.band_outdb_uri.append_null(),
+ }
+ match outdb_format {
+ Some(format) => self.band_outdb_format.append_value(format),
+ None => self.band_outdb_format.append_null(),
+ }
+
+ self.current_band_count += 1;
+ self.band_data_count_at_start = self.band_data.len();
+
+ // finish_raster compares the band's *visible* shape against
+ // spatial_shape.
+ self.current_raster_bands.push((
+ dim_names.iter().map(|s| s.to_string()).collect(),
+ view_entries.visible_shape(),
));
+
+ return Ok(());
}
}
@@ -487,6 +563,58 @@ impl RasterBuilder {
Ok(())
}
+ /// Build a band that is a new view into an existing band.
+ ///
+ /// The output band stores a view that is the composition of `input`'s
+ /// existing view with the supplied `view`. The supplied `view`'s
+ /// `source_axis` entries refer to `input`'s *visible* axes, not its
+ /// source axes — composition with `input.view()` translates them, so the
+ /// caller expresses the slice in the coordinates it sees.
+ ///
+ /// `dim_names` names the output's *visible* axes (`len() == view.len()`).
+ ///
+ /// Storage:
+ /// - **InDb input** → output is InDb. The input's source bytes are carried
+ /// over via [`BandRef::append_data_into`] (zero-copy when the impl shares
+ /// its backing `Buffer`); the composed view addresses the visible region
+ /// within them.
+ /// - **OutDb input** → output is OutDb. The data column stays empty and the
+ /// input's `outdb_uri` / `outdb_format` are inherited (unless overridden);
+ /// the composed view lives alongside the same external pointer and loading
+ /// is deferred to whoever reads the visible bytes.
+ ///
+ /// Identity-input shortcut: when `input` carries the identity view, the
+ /// composed view equals `view` verbatim.
+ pub fn with_view(&mut self, args: WithViewArgs) -> Result<(), ArrowError> {
+ let WithViewArgs {
+ name,
+ dim_names,
+ input,
+ view,
+ nodata,
+ outdb_uri,
+ outdb_format,
+ } = args;
+ // Delegate to `copy_into`, which composes `view` (a delta over the
+ // input's visible axes) onto the input's own view, carries the source
+ // bytes over, and inherits every field left unset here from the input
+ // via `.or_else(|| input.<field>())` — including `nodata`, which the
+ // earlier hand-rolled implementation forwarded verbatim and thereby
+ // dropped. `dim_names` and `view` are always supplied by this call, so
+ // they pass through as explicit overrides.
+ input.copy_into(
+ self,
+ BandOverrides {
+ name,
+ dim_names: Some(dim_names),
+ nodata,
+ outdb_uri,
+ outdb_format,
+ view: Some(view),
+ },
+ )
+ }
+
/// Convenience: start a 2D band with `dim_names=["y","x"]` and `shape=[height, width]`.
///
/// Must be called after `start_raster_2d` / `start_raster_2d` which sets
@@ -848,6 +976,16 @@ mod tests {
use arrow_schema::Schema;
use std::io::Cursor;
+ /// Terse [`ViewEntry`] constructor for the view-persistence tests.
+ fn ve(source_axis: i64, start: i64, step: i64, steps: i64) -> ViewEntry {
+ ViewEntry {
+ source_axis,
+ start,
+ step,
+ steps,
+ }
+ }
+
#[test]
fn test_iterator_basic_functionality() {
// Create a simple raster for testing using the correct API
@@ -1190,11 +1328,16 @@ mod tests {
#[test]
fn test_outdb_metadata_fields() {
- // Test creating raster with OutDb reference metadata
+ // Test creating raster with OutDb reference metadata.
+ //
+ // 10x10 UInt8 = 100 visible bytes, matching the InDb data buffer
+ // written below. `RasterRef::band()` now verifies the data column is
+ // long enough to cover the visible region, so the dimensions and the
+ // byte count must agree.
let mut builder = RasterBuilder::new(10);
builder
- .start_raster_2d(1024, 1024, 0.0, 0.0, 1.0, -1.0, 0.0, 0.0, None)
+ .start_raster_2d(10, 10, 0.0, 0.0, 1.0, -1.0, 0.0, 0.0, None)
.unwrap();
// Test InDb band (should have null OutDb fields)
@@ -1210,7 +1353,7 @@ mod tests {
builder
.start_band(StartBandArgs {
outdb_uri: Some("s3://mybucket/satellite_image.tif#band=2"),
- ..StartBandArgs::new(&["y", "x"], &[1024, 1024], BandDataType::Float32)
+ ..StartBandArgs::new(&["y", "x"], &[10, 10], BandDataType::Float32)
})
.unwrap();
// For OutDbRef, data field could be empty or contain metadata/thumbnail
@@ -1831,6 +1974,518 @@ mod tests {
assert_eq!(buf.as_contiguous().unwrap(), pixels.as_slice());
}
+ // ---- Non-identity view persistence: construct → finish → read back ----
+
+ /// Build a single-raster, single-band `UInt8` array carrying an explicit
+ /// `view` over `source_shape`, with `data` as the band's raw bytes. Uses
+ /// empty top-level spatial dims so `finish_raster` imposes no spatial-shape
+ /// constraint on the (arbitrary) view being exercised.
+ fn build_viewed_u8(
+ source_shape: &[i64],
+ dim_names: &[&str],
+ view: &[ViewEntry],
+ data: Vec<u8>,
+ ) -> StructArray {
+ let mut b = RasterBuilder::new(1);
+ b.start_raster_nd(&[0.0, 1.0, 0.0, 0.0, 0.0, -1.0], &[], &[], None)
+ .unwrap();
+ b.start_band(StartBandArgs {
+ view: Some(view),
+ ..StartBandArgs::new(dim_names, source_shape, BandDataType::UInt8)
+ })
+ .unwrap();
+ b.band_data_writer().append_value(data);
+ b.finish_band().unwrap();
+ b.finish_raster().unwrap();
+ b.finish().unwrap()
+ }
+
+ /// Walk an `NdBuffer`'s visible region in C-order and collect the single
+ /// `UInt8` byte at each visited position. The byte address is hand-computed
+ /// from the buffer's own shape/strides/offset, so it verifies exactly the
+ /// layout the reader composed — independent of `as_contiguous`, which
+ /// refuses strided views. Compare its output against a hand-computed
+ /// expectation, never against the buffer itself.
+ fn gather_u8(buf: &crate::traits::NdBuffer) -> Vec<u8> {
+ assert_eq!(buf.data_type, BandDataType::UInt8);
+ let n: i64 = buf.shape.iter().product();
+ let mut out = Vec::with_capacity(n.max(0) as usize);
+ let mut idx = vec![0i64; buf.shape.len()];
+ for _ in 0..n {
+ let mut pos = buf.offset as i64;
+ for (k, &i) in idx.iter().enumerate() {
+ pos += i * buf.strides[k];
+ }
+ out.push(buf.buffer[pos as usize]);
+ // Increment the multi-index in C-order (last axis fastest).
+ for k in (0..buf.shape.len()).rev() {
+ idx[k] += 1;
+ if idx[k] < buf.shape[k] {
+ break;
+ }
+ idx[k] = 0;
+ }
+ }
+ out
+ }
+
+ #[test]
+ fn view_identity_via_start_band_is_null_and_borrows() {
+ // An identity view passed through start_band as an explicit `Some` view
+ // is stored as the canonical null sentinel (not an explicit row), so it
+ // is indistinguishable from the `None` path: null view row, same
+ // visible shape/strides, zero-copy borrow.
+ use arrow_array::Array;
+ let view = [ve(0, 0, 1, 2), ve(1, 0, 1, 3)];
+ let pixels: Vec<u8> = (0..6).collect();
+ let array = build_viewed_u8(&[2, 3], &["y", "x"], &view, pixels.clone());
+
+ let bands_struct = array
+ .column(sedona_schema::raster::raster_indices::BANDS)
+ .as_any()
+ .downcast_ref::<ListArray>()
+ .unwrap()
+ .values()
+ .as_any()
+ .downcast_ref::<StructArray>()
+ .unwrap();
+ let view_list = bands_struct
+ .column(sedona_schema::raster::band_indices::VIEW)
+ .as_any()
+ .downcast_ref::<ListArray>()
+ .unwrap();
+ assert!(
+ view_list.is_null(0),
+ "identity view must serialise as a null view row even via an explicit start_band view"
+ );
+
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let r = rasters.get(0).unwrap();
+ let band = r.band(0).unwrap();
+ assert_eq!(band.shape(), &[2, 3]);
+ let buf = band.nd_buffer().unwrap();
+ assert_eq!(buf.strides, &[3, 1]);
+ assert_eq!(buf.offset, 0);
+ assert!(buf.is_contiguous());
+ assert_eq!(buf.as_contiguous().unwrap(), pixels.as_slice());
+ assert_eq!(gather_u8(&buf), pixels);
+ }
+
+ #[test]
+ fn view_slice_outer_axis_is_contiguous() {
+ // Slice the outer axis of a 3x3 source to its first 2 rows. The view is
+ // non-identity, but its byte strides are still C-order packed from
+ // offset 0, so the region is contiguous and borrows the source prefix
+ // zero-copy.
+ let data: Vec<u8> = (0..9).collect();
+ let view = [ve(0, 0, 1, 2), ve(1, 0, 1, 3)];
+ let array = build_viewed_u8(&[3, 3], &["y", "x"], &view, data.clone());
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let r = rasters.get(0).unwrap();
+ let band = r.band(0).unwrap();
+
+ assert_eq!(band.shape(), &[2, 3]);
+ assert_eq!(band.raw_source_shape(), &[3, 3]);
+ let buf = band.nd_buffer().unwrap();
+ assert_eq!(buf.strides, &[3, 1]);
+ assert_eq!(buf.offset, 0);
+ assert!(buf.is_contiguous());
+ assert_eq!(buf.as_contiguous().unwrap(), &data[0..6]);
+ assert_eq!(gather_u8(&buf), vec![0, 1, 2, 3, 4, 5]);
+ }
+
+ #[test]
+ fn view_slice_strided_reads_expected_values() {
+ // Every-other slice of an 8-element source: start=1, step=2, steps=3
+ // addresses source indices 1, 3, 5.
+ let data: Vec<u8> = (0..8).collect();
+ let array = build_viewed_u8(&[8], &["x"], &[ve(0, 1, 2, 3)], data);
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let r = rasters.get(0).unwrap();
+ let band = r.band(0).unwrap();
+
+ assert_eq!(band.shape(), &[3]);
+ let buf = band.nd_buffer().unwrap();
+ assert_eq!(buf.shape, &[3]);
+ assert_eq!(buf.strides, &[2]);
+ assert_eq!(buf.offset, 1);
+ // Strided → not C-order packed, so as_contiguous rejects it.
+ assert!(!buf.is_contiguous());
+ assert!(buf.as_contiguous().is_err());
+ assert_eq!(gather_u8(&buf), vec![1, 3, 5]);
+ }
+
+ #[test]
+ fn view_broadcast_zero_stride_repeats_source_row() {
+ // 2D broadcast: source shape [1, 3], the view broadcasts axis 0 four
+ // times (step 0) so every visible row equals the source's single row.
+ let view = [ve(0, 0, 0, 4), ve(1, 0, 1, 3)];
+ let array = build_viewed_u8(&[1, 3], &["row", "col"], &view, vec![10u8, 20, 30]);
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let r = rasters.get(0).unwrap();
+ let band = r.band(0).unwrap();
+
+ assert_eq!(band.shape(), &[4, 3]);
+ let buf = band.nd_buffer().unwrap();
+ assert_eq!(buf.shape, &[4, 3]);
+ assert_eq!(buf.strides, &[0, 1]);
+ assert_eq!(buf.offset, 0);
+ // Zero stride is not packed → non-contiguous, rejected.
+ assert!(!buf.is_contiguous());
+ assert!(buf.as_contiguous().is_err());
+ assert_eq!(
+ gather_u8(&buf),
+ vec![10, 20, 30, 10, 20, 30, 10, 20, 30, 10, 20, 30]
+ );
+ }
+
+ #[test]
+ fn view_axis_permutation_and_slice_reads_expected_values() {
+ // 2D source [Y=4, X=3], data = 0..12 row-major. The view permutes to
+ // visible order [X, Y] and slices Y from start=1, step=2, steps=2.
+ // byte_strides = [step_X * src_stride_X, step_Y * src_stride_Y]
+ // = [1 * 1, 2 * 3] = [1, 6]
+ // byte_offset = start_X * src_stride_X + start_Y * src_stride_Y
+ // = 0 * 1 + 1 * 3 = 3
+ // Visible[i, j] (i over X 0..3, j over Y {1,3}) sits at source byte
+ // 3 + i + 6j → C-order gather = [3, 9, 4, 10, 5, 11].
+ let data: Vec<u8> = (0..12).collect();
+ let view = [ve(1, 0, 1, 3), ve(0, 1, 2, 2)];
+ let array = build_viewed_u8(&[4, 3], &["x", "y"], &view, data);
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let r = rasters.get(0).unwrap();
+ let band = r.band(0).unwrap();
+
+ assert_eq!(band.shape(), &[3, 2]);
+ let buf = band.nd_buffer().unwrap();
+ assert_eq!(buf.shape, &[3, 2]);
+ assert_eq!(buf.strides, &[1, 6]);
+ assert_eq!(buf.offset, 3);
+ assert!(!buf.is_contiguous());
+ assert!(buf.as_contiguous().is_err());
+ assert_eq!(gather_u8(&buf), vec![3, 9, 4, 10, 5, 11]);
+ }
+
+ #[test]
+ fn view_reverse_negative_step_reads_expected_values() {
+ // 1D source [0..8]; start=6, step=-2, steps=3 walks backwards picking
+ // every other element: source indices 6, 4, 2.
+ let data: Vec<u8> = (0..8).collect();
+ let array = build_viewed_u8(&[8], &["x"], &[ve(0, 6, -2, 3)], data);
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let r = rasters.get(0).unwrap();
+ let band = r.band(0).unwrap();
+
+ assert_eq!(band.shape(), &[3]);
+ let buf = band.nd_buffer().unwrap();
+ assert_eq!(buf.shape, &[3]);
+ assert_eq!(buf.strides, &[-2]);
+ assert_eq!(buf.offset, 6);
+ // Negative stride is not packed → non-contiguous, rejected.
+ assert!(!buf.is_contiguous());
+ assert!(buf.as_contiguous().is_err());
+ assert_eq!(gather_u8(&buf), vec![6, 4, 2]);
+ }
+
+ #[test]
+ fn view_multidim_with_zero_axis_borrows_empty() {
+ // A zero-extent middle axis addresses no bytes: the visible region is
+ // empty, trivially contiguous, and as_contiguous borrows an empty slice.
+ let view = [ve(0, 0, 1, 3), ve(1, 0, 1, 0), ve(2, 0, 1, 5)];
+ let array = build_viewed_u8(&[3, 4, 5], &["a", "b", "c"], &view, vec![0u8; 60]);
+ let rasters = RasterStructArray::try_new(&array).unwrap();
+ let r = rasters.get(0).unwrap();
+ let band = r.band(0).unwrap();
+
+ assert_eq!(band.shape(), &[3, 0, 5]);
+ let buf = band.nd_buffer().unwrap();
+ assert_eq!(buf.shape, &[3, 0, 5]);
+ assert!(buf.is_contiguous());
+ assert!(buf.as_contiguous().unwrap().is_empty());
+ }
+
+ #[test]
+ fn start_band_explicit_view_rejects_zero_dim() {
+ // An explicit `Some` view must apply the same 0-D guard as the identity
+ // path — accepting empty dim_names would otherwise bypass it via the
+ // explicit view path.
+ let mut builder = RasterBuilder::new(1);
+ builder
+ .start_raster_nd(&[0.0, 1.0, 0.0, 0.0, 0.0, -1.0], &[], &[], None)
+ .unwrap();
+ let err = builder
+ .start_band(StartBandArgs {
+ view: Some(&[]),
+ ..StartBandArgs::new(&[], &[], BandDataType::UInt8)
+ })
+ .unwrap_err();
+ assert!(
+ err.to_string().contains("0-dimensional"),
+ "unexpected error: {err}"
+ );
+ }
+
+ #[test]
+ fn start_band_explicit_view_rejects_step_overrun() {
+ // start=1, step=2, steps=4 addresses element 1 + (4-1)*2 = 7, out of
+ // range for a source axis of size 7 — validation must reject it before
+ // any column is written.
+ let mut builder = RasterBuilder::new(1);
+ builder
+ .start_raster_nd(&[0.0, 1.0, 0.0, 0.0, 0.0, -1.0], &[], &[], None)
+ .unwrap();
+ let err = builder
+ .start_band(StartBandArgs {
+ view: Some(&[ve(0, 1, 2, 4)]),
+ ..StartBandArgs::new(&["x"], &[7], BandDataType::UInt8)
+ })
+ .unwrap_err();
+ assert!(
+ err.to_string().contains("out of range"),
+ "unexpected error: {err}"
+ );
+ }
+
+ // ---- with_view: public "create a new view into an existing band" ----
+
+ /// Build a 1-D UInt8 raster with `source_shape=[8]` and bytes `[0..8]`.
+ /// Identity-view; used as input to the with_view tests.
+ fn build_1d_identity_raster() -> StructArray {
+ let mut b = RasterBuilder::new(1);
+ b.start_raster_nd(&[0.0, 1.0, 0.0, 0.0, 0.0, -1.0], &["x"], &[8], None)
+ .unwrap();
+ b.start_band(StartBandArgs::new(&["x"], &[8], BandDataType::UInt8))
+ .unwrap();
+ b.band_data_writer()
+ .append_value((0u8..8).collect::<Vec<u8>>());
+ b.finish_band().unwrap();
+ b.finish_raster().unwrap();
+ b.finish().unwrap()
+ }
+
+ #[test]
+ fn with_view_over_identity_input_produces_expected_visible_bytes() {
+ // Input is identity over [0..8]. with_view layers a slice
+ // (start=1, step=2, steps=3) producing visible bytes [1, 3, 5].
+ let input_array = build_1d_identity_raster();
+ let input_rasters = RasterStructArray::try_new(&input_array).unwrap();
+ let input_raster = input_rasters.get(0).unwrap();
+ let input_band = input_raster.band(0).unwrap();
+
+ let mut b = RasterBuilder::new(1);
+ b.start_raster_nd(&[0.0, 1.0, 0.0, 0.0, 0.0, -1.0], &["x"], &[3], None)
+ .unwrap();
+ b.with_view(WithViewArgs {
+ name: None,
+ dim_names: &["x"],
+ input: input_band.as_ref(),
+ view: &[ve(0, 1, 2, 3)],
+ nodata: None,
+ outdb_uri: None,
+ outdb_format: None,
+ })
+ .unwrap();
+ b.finish_band().unwrap();
+ b.finish_raster().unwrap();
+ let out_array = b.finish().unwrap();
+
+ let out_rasters = RasterStructArray::try_new(&out_array).unwrap();
+ let out_raster = out_rasters.get(0).unwrap();
+ let out_band = out_raster.band(0).unwrap();
+ assert_eq!(out_band.shape(), &[3]);
+ // The output's source_shape is inherited from the input.
+ assert_eq!(out_band.raw_source_shape(), &[8]);
+ let buf = out_band.nd_buffer().unwrap();
+ assert_eq!(buf.strides, &[2]);
+ assert_eq!(buf.offset, 1);
+ assert!(!buf.is_contiguous());
+ assert_eq!(gather_u8(&buf), vec![1, 3, 5]);
+ }
+
+ #[test]
+ fn with_view_chained_composes_into_single_view() {
+ // Round 1: with_view layers (start=1, step=2, steps=4) → visible bytes
+ // [1, 3, 5, 7] over source [0..8].
+ // Round 2: with_view on that, layering (start=1, step=1, steps=2) →
+ // visible bytes [3, 5] (input visible indices 1 and 2).
+ // compose collapses the chain into one source-space view; the read
+ // back must reference the ORIGINAL 8-byte source.
+ let input_array = build_1d_identity_raster();
+ let input_rasters = RasterStructArray::try_new(&input_array).unwrap();
+ let input_raster = input_rasters.get(0).unwrap();
+ let input_band = input_raster.band(0).unwrap();
+
+ // Round 1.
+ let mut b1 = RasterBuilder::new(1);
+ b1.start_raster_nd(&[0.0, 1.0, 0.0, 0.0, 0.0, -1.0], &["x"], &[4], None)
+ .unwrap();
+ b1.with_view(WithViewArgs {
+ name: None,
+ dim_names: &["x"],
+ input: input_band.as_ref(),
+ view: &[ve(0, 1, 2, 4)],
+ nodata: None,
+ outdb_uri: None,
+ outdb_format: None,
+ })
+ .unwrap();
+ b1.finish_band().unwrap();
+ b1.finish_raster().unwrap();
+ let mid_array = b1.finish().unwrap();
+
+ let mid_rasters = RasterStructArray::try_new(&mid_array).unwrap();
+ let mid_raster = mid_rasters.get(0).unwrap();
+ let mid_band = mid_raster.band(0).unwrap();
+ assert_eq!(mid_band.shape(), &[4]);
+ assert_eq!(gather_u8(&mid_band.nd_buffer().unwrap()), vec![1, 3, 5, 7]);
+
+ // Round 2: with_view applied on the view-bearing mid_band.
+ let mut b2 = RasterBuilder::new(1);
+ b2.start_raster_nd(&[0.0, 1.0, 0.0, 0.0, 0.0, -1.0], &["x"], &[2], None)
+ .unwrap();
+ b2.with_view(WithViewArgs {
+ name: None,
+ dim_names: &["x"],
+ input: mid_band.as_ref(),
+ view: &[ve(0, 1, 1, 2)],
+ nodata: None,
+ outdb_uri: None,
+ outdb_format: None,
+ })
+ .unwrap();
+ b2.finish_band().unwrap();
+ b2.finish_raster().unwrap();
+ let final_array = b2.finish().unwrap();
+
+ let final_rasters = RasterStructArray::try_new(&final_array).unwrap();
+ let final_raster = final_rasters.get(0).unwrap();
+ let final_band = final_raster.band(0).unwrap();
+ assert_eq!(final_band.shape(), &[2]);
+ // The composed view still references the original 8-byte source.
+ assert_eq!(final_band.raw_source_shape(), &[8]);
+ let final_buf = final_band.nd_buffer().unwrap();
+ assert_eq!(final_buf.strides, &[2]);
+ assert_eq!(final_buf.offset, 3);
+ assert!(!final_buf.is_contiguous());
+ assert_eq!(gather_u8(&final_buf), vec![3, 5]);
+ }
+
+ #[test]
+ fn with_view_on_outdb_input_produces_outdb_output_with_composed_view() {
+ // Viewing an OutDb band doesn't need the source bytes — the output band
+ // is itself OutDb, pointing at the same external resource via an
+ // inherited outdb_uri, with the composed view describing the slice.
+ let mut b = RasterBuilder::new(1);
+ b.start_raster_nd(&[0.0, 1.0, 0.0, 0.0, 0.0, -1.0], &["x"], &[8], None)
+ .unwrap();
+ b.start_band(StartBandArgs {
+ outdb_uri: Some("s3://bucket/file.tif#band=1"),
+ outdb_format: Some("geotiff"),
+ ..StartBandArgs::new(&["x"], &[8], BandDataType::UInt8)
+ })
+ .unwrap();
+ b.band_data_writer().append_value([0u8; 0]); // empty → OutDb
+ b.finish_band().unwrap();
+ b.finish_raster().unwrap();
+ let input_array = b.finish().unwrap();
+
+ let input_rasters = RasterStructArray::try_new(&input_array).unwrap();
+ let input_raster = input_rasters.get(0).unwrap();
+ let input_band = input_raster.band(0).unwrap();
+ assert!(!input_band.is_indb(), "fixture must be OutDb");
+
+ let mut b2 = RasterBuilder::new(1);
+ b2.start_raster_nd(&[0.0, 1.0, 0.0, 0.0, 0.0, -1.0], &["x"], &[3], None)
+ .unwrap();
+ b2.with_view(WithViewArgs {
+ name: None,
+ dim_names: &["x"],
+ input: input_band.as_ref(),
+ view: &[ve(0, 1, 2, 3)],
+ nodata: None,
+ outdb_uri: None,
+ outdb_format: None,
+ })
+ .unwrap();
+ b2.finish_band().unwrap();
+ b2.finish_raster().unwrap();
+ let out_array = b2.finish().unwrap();
+
+ let out_rasters = RasterStructArray::try_new(&out_array).unwrap();
+ let out_raster = out_rasters.get(0).unwrap();
+ let out_band = out_raster.band(0).unwrap();
+
+ assert!(
+ !out_band.is_indb(),
+ "output of OutDb-input with_view must be OutDb"
+ );
+ assert_eq!(out_band.outdb_uri(), Some("s3://bucket/file.tif#band=1"));
+ assert_eq!(out_band.outdb_format(), Some("geotiff"));
+ // Input had identity view, so composed == supplied view verbatim.
+ assert_eq!(out_band.view(), &[ve(0, 1, 2, 3)]);
+ assert_eq!(out_band.raw_source_shape(), &[8]);
+ assert_eq!(out_band.shape(), &[3]);
+ }
+
+ #[test]
+ fn with_view_inherits_source_nodata() {
+ // Viewing a band must not drop its nodata sentinel. The earlier
+ // implementation forwarded the caller's `nodata` (None here) verbatim
+ // and never inherited the source's, so former-nodata pixels silently
+ // became valid. Delegating through `copy_into` inherits it.
+ let transform = [0.0, 1.0, 0.0, 0.0, 0.0, -1.0];
+ let mut ib = RasterBuilder::new(1);
+ ib.start_raster_nd(&transform, &["x"], &[4], None).unwrap();
+ ib.start_band(StartBandArgs {
+ name: Some("orig"),
+ nodata: Some(&[255u8]),
+ ..StartBandArgs::new(&["x"], &[4], BandDataType::UInt8)
+ })
+ .unwrap();
+ ib.band_data_writer().append_value(vec![1u8, 2, 3, 4]);
+ ib.finish_band().unwrap();
+ ib.finish_raster().unwrap();
+ let in_array = ib.finish().unwrap();
+ let in_rasters = RasterStructArray::try_new(&in_array).unwrap();
+ let in_raster = in_rasters.get(0).unwrap();
+ let in_band = in_raster.band(0).unwrap();
+ assert_eq!(
+ in_band.nodata(),
+ Some(&[255u8][..]),
+ "fixture must carry nodata"
+ );
+
+ // A non-identity view (every other element) with no explicit nodata
+ // override — the source's [255] must carry over to the derived band.
+ let mut ob = RasterBuilder::new(1);
+ ob.start_raster_nd(&transform, &["x"], &[2], None).unwrap();
+ ob.with_view(WithViewArgs {
+ name: None,
+ dim_names: &["x"],
+ input: in_band.as_ref(),
+ view: &[ve(0, 0, 2, 2)],
+ nodata: None,
+ outdb_uri: None,
+ outdb_format: None,
+ })
+ .unwrap();
+ ob.finish_band().unwrap();
+ ob.finish_raster().unwrap();
+ let out_array = ob.finish().unwrap();
+ let out_rasters = RasterStructArray::try_new(&out_array).unwrap();
+ let out_raster = out_rasters.get(0).unwrap();
+ let out_band = out_raster.band(0).unwrap();
+
+ assert_eq!(
+ out_band.nodata(),
+ Some(&[255u8][..]),
+ "with_view must inherit the source band's nodata"
+ );
+ // The view is still applied: visible bytes are the every-other slice.
+ assert_eq!(out_band.shape(), &[2]);
+ assert_eq!(gather_u8(&out_band.nd_buffer().unwrap()), vec![1, 3]);
+ }
+
#[test]
fn test_view_field_is_null_for_identity_band() {
// Schema invariant: identity views are stored as null list rows so
@@ -1909,10 +2564,13 @@ mod tests {
// null view row, and the null must survive an Arrow IPC round-trip.
// If a future change accidentally writes a non-null empty list
// instead, downstream readers (DuckDB, PyArrow, sedona-py) will
- // disagree about whether the view is identity.
+ // disagree about whether the view is identity. A second raster carries
+ // an explicit non-identity view to confirm the non-null row (and the
+ // visible shape it decodes to) also survives the round-trip.
- let mut builder = RasterBuilder::new(1);
+ let mut builder = RasterBuilder::new(2);
let transform = [0.0, 1.0, 0.0, 0.0, 0.0, -1.0];
+ // Raster 0: identity-view band → null view row.
builder
.start_raster_nd(&transform, &["x", "y"], &[3, 2], None)
.unwrap();
@@ -1926,6 +2584,21 @@ mod tests {
builder.band_data_writer().append_value(vec![0u8; 6]);
builder.finish_band().unwrap();
builder.finish_raster().unwrap();
+ // Raster 1: explicit non-identity view → non-null view row.
+ builder
+ .start_raster_nd(&transform, &["x"], &[3], None)
+ .unwrap();
+ builder
+ .start_band(StartBandArgs {
+ view: Some(&[ve(0, 1, 2, 3)]),
+ ..StartBandArgs::new(&["x"], &[8], BandDataType::UInt8)
+ })
+ .unwrap();
+ builder
+ .band_data_writer()
+ .append_value(vec![0u8, 1, 2, 3, 4, 5, 6, 7]);
+ builder.finish_band().unwrap();
+ builder.finish_raster().unwrap();
let array = builder.finish().unwrap();
let schema = Arc::new(Schema::new(vec![Arc::new(arrow_schema::Field::new(
@@ -1967,15 +2640,21 @@ mod tests {
.as_any()
.downcast_ref::<ListArray>()
.unwrap();
- assert_eq!(view_list.len(), 1);
+ assert_eq!(view_list.len(), 2);
assert!(
view_list.is_null(0),
"identity-view band must remain a null view row after IPC round-trip"
);
+ assert!(
+ !view_list.is_null(1),
+ "explicit-view band must remain non-null after IPC round-trip"
+ );
let rasters = RasterStructArray::try_new(restored_struct).unwrap();
let r0 = rasters.get(0).unwrap();
assert_eq!(r0.band(0).unwrap().shape(), &[2, 3]);
+ let r1 = rasters.get(1).unwrap();
+ assert_eq!(r1.band(0).unwrap().shape(), &[3]);
}
/// Navigate an output raster `StructArray` to its bands' `data`
diff --git a/rust/sedona-raster/src/traits.rs b/rust/sedona-raster/src/traits.rs
index 055392138..d988d2e48 100644
--- a/rust/sedona-raster/src/traits.rs
+++ b/rust/sedona-raster/src/traits.rs
@@ -297,8 +297,9 @@ pub struct BandOverrides<'a> {
/// coordinates**. [`BandRef::copy_into`] composes it onto the source's own
/// view for you — you don't manage that composition and don't need to know
/// whether the source already carries a view. `None` inherits the source's
- /// view unchanged. (A non-identity result isn't persistable yet and
- /// `copy_into` rejects it; see <https://github.com/apache/sedona-db/issues/897>.)
+ /// view unchanged. A non-identity result is persisted on the derived band
+ /// (as a slice, broadcast, permutation, or reverse) and decoded back by the
+ /// reader; the underlying bytes are carried over unchanged.
pub view: Option<&'a [ViewEntry]>,
}
@@ -455,12 +456,11 @@ pub trait BandRef {
/// inherits the source's view unchanged.
///
/// The composition + persistence is delegated to
- /// [`RasterBuilder::start_band`]. Today that stores views only as
- /// the canonical identity null sentinel, so a non-identity effective view
- /// is rejected rather than copying mislocated bytes; in practice the source
- /// is identity-viewed and any override must compose back to the identity.
- /// View persistence is tracked in
- /// <https://github.com/apache/sedona-db/issues/897>.
+ /// [`RasterBuilder::start_band`]: an identity effective view is
+ /// stored as the canonical null sentinel, and a non-identity one (a slice,
+ /// broadcast, permutation, or reverse) is persisted explicitly and decoded
+ /// back by the reader. The source bytes are carried over unchanged — the
+ /// view relocates the visible window over them, it does not repack them.
fn copy_into(
&self,
builder: &mut RasterBuilder,
@@ -872,27 +872,82 @@ mod tests {
}
#[test]
- fn copy_into_rejects_non_identity_source_view() {
- // A sliced source view (step 2 on the outer axis) composes to a
- // non-identity effective view; copy_into can't persist it yet, so it
- // must error rather than copy mislocated bytes.
- let src = band(&["y", "x"], &[4, 5], &[ve(0, 0, 2, 2), ve(1, 0, 1, 5)]);
+ fn copy_into_persists_non_identity_source_view() {
+ // A source band that already carries a non-identity view (an every-
+ // other slice over an 8-element axis) is copied with default overrides:
+ // the effective view is the source's own slice, and copy_into persists
+ // it and carries the source bytes over. Read back, the derived band
+ // exposes the same slice — visible values [1, 3, 5].
+ use crate::array::RasterStructArray;
+ let transform = [0.0, 1.0, 0.0, 0.0, 0.0, -1.0];
+
+ // Source: source_shape [8], data [0..8], view (start=1, step=2, steps=3).
+ let mut ib = RasterBuilder::new(1);
+ ib.start_raster_nd(&transform, &["x"], &[3], None).unwrap();
+ ib.start_band(StartBandArgs {
+ view: Some(&[ve(0, 1, 2, 3)]),
+ ..StartBandArgs::new(&["x"], &[8], BandDataType::UInt8)
+ })
+ .unwrap();
+ ib.band_data_writer()
+ .append_value((0u8..8).collect::<Vec<u8>>());
+ ib.finish_band().unwrap();
+ ib.finish_raster().unwrap();
+ let in_array = ib.finish().unwrap();
+ let in_rasters = RasterStructArray::try_new(&in_array).unwrap();
+ let in_raster = in_rasters.get(0).unwrap();
+ let in_band = in_raster.band(0).unwrap();
+
let mut ob = RasterBuilder::new(1);
- let err = src
+ ob.start_raster_nd(&transform, &["x"], &[3], None).unwrap();
+ in_band
.copy_into(&mut ob, BandOverrides::default())
- .unwrap_err()
- .to_string();
- assert!(err.contains("non-identity"), "unexpected error: {err}");
+ .unwrap();
+ ob.finish_band().unwrap();
+ ob.finish_raster().unwrap();
+ let out_array = ob.finish().unwrap();
+ let out_rasters = RasterStructArray::try_new(&out_array).unwrap();
+ let out_raster = out_rasters.get(0).unwrap();
+ let out_band = out_raster.band(0).unwrap();
+
+ assert_eq!(out_band.view(), &[ve(0, 1, 2, 3)]);
+ assert_eq!(out_band.shape(), &[3]);
+ assert_eq!(out_band.raw_source_shape(), &[8]);
+ let buf = out_band.nd_buffer().unwrap();
+ assert_eq!(buf.strides, &[2]);
+ assert_eq!(buf.offset, 1);
+ // Strided → not contiguous; the visible bytes must be [1, 3, 5].
+ assert!(!buf.is_contiguous());
+ let visited: Vec<u8> = (0..buf.shape[0])
+ .map(|i| buf.buffer[(buf.offset as i64 + i * buf.strides[0]) as usize])
+ .collect();
+ assert_eq!(visited, vec![1, 3, 5]);
}
#[test]
- fn copy_into_rejects_non_identity_override_view() {
- // Identity source, but the caller's override slices it (step 2); the
- // composed effective view is non-identity and can't be persisted yet.
- let src = band(&["y", "x"], &[4, 5], &[ve(0, 0, 1, 4), ve(1, 0, 1, 5)]);
- let override_view = [ve(0, 0, 2, 2), ve(1, 0, 1, 5)];
+ fn copy_into_persists_non_identity_override_view() {
+ // Identity source; the caller's override slices every other element
+ // (step 2). copy_into composes the override onto the identity source
+ // view and persists the resulting slice — visible values [1, 3].
+ use crate::array::RasterStructArray;
+ let transform = [0.0, 1.0, 0.0, 0.0, 0.0, -1.0];
+
+ let mut ib = RasterBuilder::new(1);
+ ib.start_raster_nd(&transform, &["x"], &[4], None).unwrap();
+ ib.start_band(StartBandArgs::new(&["x"], &[4], BandDataType::UInt8))
+ .unwrap();
+ ib.band_data_writer().append_value(vec![1u8, 2, 3, 4]);
+ ib.finish_band().unwrap();
+ ib.finish_raster().unwrap();
+ let in_array = ib.finish().unwrap();
+ let in_rasters = RasterStructArray::try_new(&in_array).unwrap();
+ let in_raster = in_rasters.get(0).unwrap();
+ let in_band = in_raster.band(0).unwrap();
+
+ let override_view = [ve(0, 0, 2, 2)];
let mut ob = RasterBuilder::new(1);
- let err = src
+ ob.start_raster_nd(&transform, &["x"], &[2], None).unwrap();
+ in_band
.copy_into(
&mut ob,
BandOverrides {
@@ -900,9 +955,25 @@ mod tests {
..Default::default()
},
)
- .unwrap_err()
- .to_string();
- assert!(err.contains("non-identity"), "unexpected error: {err}");
+ .unwrap();
+ ob.finish_band().unwrap();
+ ob.finish_raster().unwrap();
+ let out_array = ob.finish().unwrap();
+ let out_rasters = RasterStructArray::try_new(&out_array).unwrap();
+ let out_raster = out_rasters.get(0).unwrap();
+ let out_band = out_raster.band(0).unwrap();
+
+ assert_eq!(out_band.view(), &[ve(0, 0, 2, 2)]);
+ assert_eq!(out_band.shape(), &[2]);
+ assert_eq!(out_band.raw_source_shape(), &[4]);
+ let buf = out_band.nd_buffer().unwrap();
+ assert_eq!(buf.strides, &[2]);
+ assert_eq!(buf.offset, 0);
+ assert!(!buf.is_contiguous());
+ let visited: Vec<u8> = (0..buf.shape[0])
+ .map(|i| buf.buffer[(buf.offset as i64 + i * buf.strides[0]) as usize])
+ .collect();
+ assert_eq!(visited, vec![1, 3]);
}
#[test]
diff --git a/rust/sedona-raster/src/view_entries.rs b/rust/sedona-raster/src/view_entries.rs
index 3e8f767d5..a16e9e753 100644
--- a/rust/sedona-raster/src/view_entries.rs
+++ b/rust/sedona-raster/src/view_entries.rs
@@ -230,6 +230,11 @@ impl ViewEntries {
/// - `out.step = next.step * self.step`
/// - `out.steps = next.steps`
///
+ /// Each `next` entry is additionally bounded against the parent's *visible*
+ /// extent (`self[next.source_axis].steps`), not just the underlying source:
+ /// a delta may not address an element outside the window `self` already
+ /// exposes, which would re-expose bytes the parent view had sliced away.
+ ///
/// Uses checked arithmetic at every step. The caller must
/// [`validate`] the result against the source shape before use.
///
@@ -246,6 +251,43 @@ impl ViewEntries {
)));
}
let input = &self.0[next_entry.source_axis as usize];
+
+ // Bound the delta against the PARENT's visible extent. `next_entry`
+ // indexes the parent's visible axis `next_entry.source_axis`, which
+ // exposes exactly `input.steps` elements; a delta reaching past that
+ // window would re-expose bytes the parent had sliced away. Mirror
+ // `validate`'s start / last-element bounds, but against `input.steps`
+ // (the parent's visible length) rather than the raw source size.
+ if next_entry.steps > 0 {
+ let visible_len = input.steps;
+ if next_entry.start < 0 || next_entry.start >= visible_len {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "compose: next[{k}].start ({}) is out of range [0, {visible_len}) \
+ for the parent's visible axis {}",
+ next_entry.start, next_entry.source_axis
+ )));
+ }
+ if next_entry.step != 0 {
+ let last = (next_entry.steps - 1)
+ .checked_mul(next_entry.step)
+ .and_then(|d| next_entry.start.checked_add(d))
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "compose: next[{k}] last-element index overflows i64 \
+ (start={}, step={}, steps={})",
+ next_entry.start, next_entry.step, next_entry.steps
+ ))
+ })?;
+ if last < 0 || last >= visible_len {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "compose: next[{k}] addresses element {last}, out of range \
+ [0, {visible_len}) for the parent's visible axis {}",
+ next_entry.source_axis
+ )));
+ }
+ }
+ }
+
let step = next_entry.step.checked_mul(input.step).ok_or_else(|| {
ArrowError::InvalidArgumentError(format!(
"compose: step product overflows i64 at axis {k} \
@@ -601,4 +643,26 @@ mod tests {
"got {err}"
);
}
+
+ #[test]
+ fn compose_rejects_delta_escaping_parent_visible_window() {
+ // Parent exposes source[0..4] (steps=4) of an 8-long source. A delta
+ // that reads 8 elements would reach source indices 4..8 — bytes the
+ // parent sliced away. Composition must reject it even though those
+ // indices are valid in the raw source.
+ let parent = entries(&[ve(0, 0, 1, 4)]);
+ let escaping = entries(&[ve(0, 0, 1, 8)]);
+ let err = parent.compose(&escaping).unwrap_err();
+ assert!(err.to_string().contains("visible axis"), "got {err}");
+ }
+
+ #[test]
+ fn compose_accepts_delta_within_parent_visible_window() {
+ // The same parent, with a delta that exactly fills the 4-element
+ // window, composes cleanly and stays inside the parent's extent.
+ let parent = entries(&[ve(0, 0, 1, 4)]);
+ let fitting = entries(&[ve(0, 0, 1, 4)]);
+ let composed = parent.compose(&fitting).unwrap();
+ assert_eq!(composed.as_slice(), &[ve(0, 0, 1, 4)]);
+ }
}