refactor(raster): use i64 throughout the view-machinery internals

Per Dewey's review: every `source_axis`/`source_shape`/`visible_shape`
in the view helpers should be `i64` to match `ViewEntry`'s signed
fields and the stride arithmetic, instead of round-tripping through
`u64` and sprinkling `as` casts through helper bodies.

Surface kept the same:

- `BandRef::shape() -> &[u64]` still returns u64 (matches `width()`,
  `height()`, and Arrow's `UInt64Array` storage). Bridged via an
  `unsafe` slice transmute with a documented invariant —
  `validate_view` guarantees every entry is `>= 0` so the bit-layout
  reinterpretation is sound.
- `BandRef::raw_source_shape() -> &[u64]` unchanged (raw Arrow view).

Internals now `i64`:

- `validate_view(view, source_shape: &[i64])`, with an added
  `source_shape[sa] >= 0` check at the boundary.
- `is_identity_view(view, source_shape: &[i64])`.
- `visible_shape_from_view(view) -> Vec<i64>`.
- `BandRefImpl::visible_shape: Vec<i64>`.
- `materialize_strided` and `check_view_buffer_bounds` take
  `visible_shape: &[i64]`.
- `NdBuffer.shape: Vec<i64>`, `NdBuffer.offset: i64` (internal struct
  with no out-of-crate consumers — dropping the `i64 → u64 → i64`
  round-trip Dewey called out).

Conversions concentrated at two well-defined boundaries:

- `RasterRefImpl::band()` converts `raw_source_shape() -> &[u64]`
  into `Vec<i64>` once with a `try_from` overflow check.
- `RasterBuilder::start_band_with_view` does the same conversion at
  its `source_shape: &[u64]` entry point before calling
  `validate_view`.

Tests pass: sedona-raster 131, sedona-raster-functions 143,
sedona-raster-gdal 53. Trait surface and Arrow storage unchanged.

Author: james-willis

rust/sedona-raster/src/array.rs

@@ -50,8 +50,10 @@ struct BandRefImpl<'a> {
     data_type: BandDataType,
     /// Per-visible-axis view, length = ndim
     view_entries: Vec<ViewEntry>,
-    /// Visible shape (== `[v.steps for v in view_entries]`), length = ndim
-    visible_shape: Vec<u64>,
+    /// Visible shape (== `[v.steps for v in view_entries]`), length = ndim.
+    /// `i64` to match the surrounding view-machinery arithmetic
+    /// (strides, offsets); `validate_view` guarantees `>= 0`.
+    visible_shape: Vec<i64>,
     /// Byte strides per visible axis. May be 0 (broadcast) or negative.
     byte_strides: Vec<i64>,
     /// Byte offset into `data` of the visible region's `[0,...,0]` element.
@@ -84,7 +86,18 @@ impl<'a> BandRef for BandRefImpl<'a> {
     }
 
     fn shape(&self) -> &[u64] {
-        &self.visible_shape
+        // SAFETY: `visible_shape` elements are i64 but `validate_view`
+        // guarantees they are all `>= 0` at construction. i64 and u64
+        // have identical bit layout for non-negative values, so
+        // reinterpreting the slice is sound under that invariant. The
+        // alternative (storing a parallel `Vec<u64>` or allocating on
+        // every call) is wasteful for a hot accessor.
+        unsafe {
+            std::slice::from_raw_parts(
+                self.visible_shape.as_ptr() as *const u64,
+                self.visible_shape.len(),
+            )
+        }
     }
 
     fn raw_source_shape(&self) -> &[u64] {
@@ -168,13 +181,11 @@ impl<'a> BandRef for BandRefImpl<'a> {
         }
         // shape and strides are owned by NdBuffer (see its doc comment).
         // Cloning here is cheap — both vecs are O(ndim), a handful of values.
-        // Cast offset i64 -> u64: safe because RasterRefImpl::band asserts
-        // byte_offset >= 0 before storing.
         Ok(NdBuffer {
             buffer: self.data_array.value(self.band_row),
             shape: self.visible_shape.clone(),
             strides: self.byte_strides.clone(),
-            offset: self.byte_offset as u64,
+            offset: self.byte_offset,
             data_type: self.data_type,
         })
     }
@@ -186,10 +197,8 @@ impl<'a> BandRef for BandRefImpl<'a> {
             // ARE the visible bytes. Borrow them.
             return Ok(Cow::Borrowed(buf.buffer));
         }
-        // Use self.* layout fields rather than `buf.*` to avoid the
-        // i64 -> u64 -> i64 round-trip through NdBuffer for `byte_offset`.
-        // The visible shape and byte strides are precomputed once at
-        // construction; `buf.shape` / `buf.strides` are clones of those.
+        // Use self.* layout fields directly; `buf.shape` / `buf.strides`
+        // are clones of these same fields.
         let out = materialize_strided(
             buf.buffer,
             &self.visible_shape,
@@ -220,7 +229,7 @@ impl<'a> BandRef for BandRefImpl<'a> {
 /// and skip the check.
 fn check_view_buffer_bounds(
     buffer_len: usize,
-    visible_shape: &[u64],
+    visible_shape: &[i64],
     byte_strides: &[i64],
     byte_offset: i64,
     dtype_size: usize,
@@ -231,9 +240,9 @@ fn check_view_buffer_bounds(
     let mut min_offset = byte_offset;
     let mut max_offset = byte_offset;
     for (k, &stride) in byte_strides.iter().enumerate() {
-        let last_idx = i64::try_from(visible_shape[k] - 1).map_err(|_| {
-            ArrowError::InvalidArgumentError(format!("visible_shape[{k}] - 1 exceeds i64::MAX"))
-        })?;
+        // `validate_view` guarantees `steps >= 0`, so `visible_shape[k] >= 0`
+        // and `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"
@@ -285,7 +294,7 @@ fn check_view_buffer_bounds(
 /// surface the failure.
 fn materialize_strided(
     buffer: &[u8],
-    visible_shape: &[u64],
+    visible_shape: &[i64],
     byte_strides: &[i64],
     byte_offset: i64,
     dtype_size: usize,
@@ -302,7 +311,7 @@ fn materialize_strided(
     // we'd otherwise repeat here, so this walk uses plain arithmetic and
     // unchecked slice indexing within the buffer.
     let ndim = visible_shape.len();
-    let total: u64 = visible_shape.iter().product();
+    let total: i64 = visible_shape.iter().product();
     if total == 0 {
         return Vec::new();
     }
@@ -322,13 +331,13 @@ fn materialize_strided(
 
     // Precompute a small index vector for outer axes (everything except
     // the innermost). For 1D this is empty and we run a single pass.
-    let mut outer_idx = vec![0u64; ndim.saturating_sub(1)];
+    let mut outer_idx = vec![0i64; ndim.saturating_sub(1)];
     loop {
         // Compute the byte offset of the row's first element from the
         // current outer index combination.
         let mut row_off = base;
         for (k, &i) in outer_idx.iter().enumerate() {
-            row_off += (i as i64) * byte_strides[k];
+            row_off += i * byte_strides[k];
         }
 
         if row_bytes_contiguous {
@@ -435,18 +444,37 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
         // Read source shape slice.
         let ss_start = self.band_source_shape_list.value_offsets()[band_row] as usize;
         let ss_end = self.band_source_shape_list.value_offsets()[band_row + 1] as usize;
-        let source_shape: &[u64] = &self.band_source_shape_values.values()[ss_start..ss_end];
+        let source_shape_u64: &[u64] = &self.band_source_shape_values.values()[ss_start..ss_end];
 
         // Reject 0-D bands at the read boundary. Schema doesn't forbid them
         // outright but every consumer assumes ndim >= 1.
-        if source_shape.is_empty() {
+        if source_shape_u64.is_empty() {
             return Err(ArrowError::ExternalError(Box::new(
                 sedona_common::sedona_internal_datafusion_err!(
                     "band {band_row} has empty source_shape; ndim must be >= 1"
                 ),
             )));
         }
 
+        // Convert source_shape u64 → i64 once with overflow check. Every
+        // downstream consumer in the view machinery wants i64 (matches
+        // ViewEntry's signed fields and the stride arithmetic). The cast
+        // is fallible only on cosmically large dims (> 2^63); a clean
+        // internal error is better than a wrap that silently passes
+        // later bound checks.
+        let source_shape: Vec<i64> = source_shape_u64
+            .iter()
+            .map(|&s| {
+                i64::try_from(s).map_err(|_| {
+                    ArrowError::ExternalError(Box::new(
+                        sedona_common::sedona_internal_datafusion_err!(
+                            "band {band_row}: source_shape axis {s} exceeds i64::MAX"
+                        ),
+                    ))
+                })
+            })
+            .collect::<Result<_, _>>()?;
+
         // Resolve data type up front; an unknown discriminant is a
         // schema-corruption bug, not user data, so failing the band loudly
         // here is appropriate.
@@ -474,7 +502,7 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
                     source_axis: i as i64,
                     start: 0,
                     step: 1,
-                    steps: s as i64,
+                    steps: s,
                 })
                 .collect()
         } else {
@@ -492,7 +520,7 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
 
         // Full validation: length match, source_axis permutation, bounds,
         // and steps >= 0. Anything malformed is schema-level corruption.
-        if let Err(e) = validate_view(&view_entries, source_shape) {
+        if let Err(e) = validate_view(&view_entries, &source_shape) {
             return Err(ArrowError::ExternalError(Box::new(
                 sedona_common::sedona_internal_datafusion_err!(
                     "band {band_row} has malformed view: {e}"
@@ -517,17 +545,14 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
         // C-order byte strides over the source_shape:
         //   source_strides_bytes[k] = dtype_size * Π_{j>k} source_shape[j]
         //
-        // Computed with checked arithmetic so a corrupt source_shape (a u64
-        // that doesn't fit in i64, or a product that overflows) is rejected
-        // here rather than producing a wrapped stride that silently passes
-        // later bound checks.
+        // Computed with checked arithmetic so a corrupt source_shape whose
+        // product overflows i64 is rejected rather than producing a wrapped
+        // stride that silently passes later bound checks.
         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() {
-            let next_axis = i64::try_from(source_shape[k + 1])
-                .map_err(|_| overflow_err("source_shape axis exceeds i64::MAX"))?;
             source_strides_bytes[k] = source_strides_bytes[k + 1]
-                .checked_mul(next_axis)
+                .checked_mul(source_shape[k + 1])
                 .ok_or_else(|| overflow_err("source-stride product overflows i64"))?;
         }
 
@@ -552,7 +577,7 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
                 .checked_add(start_off)
                 .ok_or_else(|| overflow_err("view offset accumulation overflows i64"))?;
         }
-        let is_identity_view = is_identity_view(&view_entries, source_shape);
+        let is_identity_view = is_identity_view(&view_entries, &source_shape);
         // byte_offset is non-negative by construction (start >= 0,
         // src_stride > 0). Check defensively so a future refactor that
         // breaks the invariant surfaces a clean internal error rather than

rust/sedona-raster/src/builder.rs

@@ -419,7 +419,21 @@ impl RasterBuilder {
             )));
         }
 
-        validate_view(view, source_shape)?;
+        // Convert source_shape u64 → i64 for validation. `start_band_with_view`
+        // is a builder entry point so producers can express a source_shape
+        // that doesn't fit in i64 — reject it here rather than letting it
+        // wrap inside `validate_view`.
+        let source_shape_i64: Vec<i64> = source_shape
+            .iter()
+            .map(|&s| {
+                i64::try_from(s).map_err(|_| {
+                    ArrowError::InvalidArgumentError(format!(
+                        "start_band_with_view: source_shape axis {s} exceeds i64::MAX"
+                    ))
+                })
+            })
+            .collect::<Result<_, _>>()?;
+        validate_view(view, &source_shape_i64)?;
 
         // Write fields.
         match name {
@@ -470,9 +484,16 @@ impl RasterBuilder {
         self.band_data_count_at_start = self.band_data.len();
 
         // finish_raster compares visible shape against spatial_shape.
+        // `visible_shape_from_view` returns Vec<i64>; the entries are
+        // guaranteed non-negative by `validate_view` above, so the cast
+        // to u64 is lossless.
+        let visible_shape_u64: Vec<u64> = visible_shape_from_view(view)
+            .into_iter()
+            .map(|v| v as u64)
+            .collect();
         self.current_raster_bands.push((
             dim_names.iter().map(|s| s.to_string()).collect(),
-            visible_shape_from_view(view),
+            visible_shape_u64,
         ));
 
         Ok(())

rust/sedona-raster/src/traits.rs

@@ -30,6 +30,11 @@ use sedona_schema::raster::BandDataType;
 /// C-order. Consumers that need a flat row-major buffer should use
 /// `BandRef::contiguous_data()` instead.
 ///
+/// `shape` and `offset` are `i64` (not `u64`) to match the surrounding
+/// stride arithmetic (`strides: Vec<i64>` to allow negative steps);
+/// shape elements are always non-negative — `validate_view` enforces
+/// this — and `offset` is non-negative by construction.
+///
 /// Only `buffer` is tied to the producer's lifetime `'a` (it can be tens of
 /// MBs of pixel data and must not be copied). `shape` and `strides` are
 /// owned `Vec`s — they're tiny (ndim ≤ a handful) so an allocation here is
@@ -38,9 +43,9 @@ use sedona_schema::raster::BandDataType;
 #[derive(Debug)]
 pub struct NdBuffer<'a> {
     pub buffer: &'a [u8],
-    pub shape: Vec<u64>,
+    pub shape: Vec<i64>,
     pub strides: Vec<i64>,
-    pub offset: u64,
+    pub offset: i64,
     pub data_type: BandDataType,
 }
 
@@ -678,13 +683,10 @@ pub trait BandRef {
 /// `view[k].steps` is the visible extent along view axis `k` after slicing /
 /// broadcasting. Callers should treat the returned shape as authoritative for
 /// the visible region; `source_shape` is only meaningful in conjunction with
-/// the per-entry `source_axis`.
-///
-/// `validate_view` guarantees `steps >= 0`, so the `as u64` cast is lossless
-/// when the input has already been validated. Callers that haven't validated
-/// yet should still call `validate_view` first.
-pub(crate) fn visible_shape_from_view(view: &[ViewEntry]) -> Vec<u64> {
-    view.iter().map(|v| v.steps as u64).collect()
+/// the per-entry `source_axis`. `validate_view` guarantees `steps >= 0`,
+/// so the entries are all non-negative when input has been validated.
+pub(crate) fn visible_shape_from_view(view: &[ViewEntry]) -> Vec<i64> {
+    view.iter().map(|v| v.steps).collect()
 }
 
 /// True iff `view` is the canonical identity over a C-order source buffer:
@@ -698,16 +700,12 @@ pub(crate) fn visible_shape_from_view(view: &[ViewEntry]) -> Vec<u64> {
 ///
 /// Callers should validate the view first; this function checks lengths
 /// defensively but trusts the inputs otherwise.
-pub(crate) fn is_identity_view(view: &[ViewEntry], source_shape: &[u64]) -> bool {
+pub(crate) fn is_identity_view(view: &[ViewEntry], source_shape: &[i64]) -> bool {
     if view.len() != source_shape.len() {
         return false;
     }
     view.iter().enumerate().all(|(k, v)| {
-        v.source_axis == k as i64
-            && v.start == 0
-            && v.step == 1
-            && v.steps >= 0
-            && (v.steps as u64) == source_shape[k]
+        v.source_axis == k as i64 && v.start == 0 && v.step == 1 && v.steps == source_shape[k]
     })
 }
 
@@ -789,7 +787,7 @@ pub(crate) fn compose_view(
 ///
 /// Runs implicitly inside `RasterBuilder::with_view` (writer) and
 /// `RasterRef::band` (reader); external callers don't need to invoke it.
-pub(crate) fn validate_view(view: &[ViewEntry], source_shape: &[u64]) -> Result<(), ArrowError> {
+pub(crate) fn validate_view(view: &[ViewEntry], source_shape: &[i64]) -> Result<(), ArrowError> {
     let ndim = source_shape.len();
     if view.len() != ndim {
         return Err(ArrowError::InvalidArgumentError(format!(
@@ -820,8 +818,14 @@ pub(crate) fn validate_view(view: &[ViewEntry], source_shape: &[u64]) -> Result<
                 v.steps
             )));
         }
+        if source_shape[sa] < 0 {
+            return Err(ArrowError::InvalidArgumentError(format!(
+                "source_shape[{sa}] = {} must be >= 0",
+                source_shape[sa]
+            )));
+        }
         if v.steps > 0 {
-            let s = source_shape[sa] as i64;
+            let s = source_shape[sa];
             if v.start < 0 || v.start >= s {
                 return Err(ArrowError::InvalidArgumentError(format!(
                     "view[{k}].start = {} is out of range [0, {s}) for source axis {sa}",
@@ -1357,7 +1361,13 @@ mod tests {
         StubBand {
             dim_names: dims.iter().map(|s| (*s).to_string()).collect(),
             source_shape: source_shape.to_vec(),
-            shape: visible_shape_from_view(view),
+            // `visible_shape_from_view` returns Vec<i64>; the StubBand
+            // stores `shape: Vec<u64>` to match BandRef::shape()'s return
+            // type. validate_view guarantees `steps >= 0`.
+            shape: visible_shape_from_view(view)
+                .into_iter()
+                .map(|v| v as u64)
+                .collect(),
             view: view.to_vec(),
         }
     }