james-willis opened a new pull request, #1198:
URL: https://github.com/apache/sedona-db/pull/1198
Closes DB-99. **Stacked on #1158** — review that first; this diff shows only
its own commit.
## The problem
A raster is an Arrow `StructArray` with five columns:
```
[0] crs Utf8View
[1] transform List<Float64> (6 per row)
[2] spatial_dims List<Utf8View>
[3] spatial_shape List<Int64>
[4] bands ← every pixel byte in the batch
```
`RS_SetCRS`, `RS_SetSRID` and `RS_SetGeoReference` change column 0 or 1.
Column 4 is untouched by definition. But the three setters were split across
two very different ways of producing that output:
| | how | cost |
|---|---|---|
| `swap_crs_column` | replace one column, carry the rest over as the same
`Arc` | correct, but **private to `rs_setsrid.rs` and hardcoded to CRS** |
| `copy_raster_from` | walk every band, re-emit every band field into a
fresh builder | rebuilds all band metadata to change one number |
`RS_SetGeoReference` used the builder. Pixel buffers were shared by
refcount, but the bands came out identical only *if the rebuild was faithful*.
#1192 is what happens when it isn't — that same rebuild silently dropped every
band name.
## The change
Generalize the first into a shared free function and move all three setters
onto it.
```rust
pub struct RasterColumnOverrides {
pub crs: Override<ArrayRef>,
pub transform: Override<ArrayRef>,
}
pub fn with_column_overrides(
raster: &StructArray,
overrides: RasterColumnOverrides,
input_nulls: Option<&NullBuffer>,
) -> Result<StructArray, RasterError>;
```
- **`Override<T>`, not `Option`** — `RS_SetSRID(raster, 0)` must *clear* the
CRS while keeping the raster, which is distinct from leaving the column alone.
`Option` can't say both; that's exactly why `swap_crs_column` was bespoke.
- **Absorbs the null merge** duplicated inline at both `rs_setsrid.rs` call
sites, including the length-1 broadcast for scalar arguments. A null *argument*
nulls the raster row — a separate axis from `Clear`ing a column.
- **A free function taking `&StructArray`**, not a method on
`RasterStructArray`. This is pure column surgery; routing it through `try_new`
would parse and validate all five columns and add a failure mode
`swap_crs_column` never had.
- **`RS_SetGeoReference` drops `copy_raster_from`**, building a
`List<Float64>` transform column directly. Its bands are now provably untouched.
## Two override vocabularies, deliberately
The ticket asked for one shared `RasterOverrides`. That can't work:
`RasterOverrides` is a **scalar** — one value per call — while
`broadcast_string_view` passes an N-row CRS array straight through when it's
already the right length. `RS_SetSRID(raster_col, srid_col)` assigns a
different CRS per row, and folding it onto a scalar struct would regress that
to one CRS for the whole call.
So `RasterOverrides` stays the **builder's** language (a scalar applied
while constructing a row), and the array path gets column-shaped overrides.
They share the `Override<T>` trinary from #1158, which is the vocabulary that
actually matters.
Also worth noting: the ticket assigns "extend `RasterOverrides` with a `crs`
field and switch it to `Override<T>`" to DB-98, but #1158 only converted
`BandOverrides`. That prerequisite is moot under this design — the array path
doesn't use `RasterOverrides` at all.
## Tests
`with_column_overrides_shares_untouched_columns` asserts `Arc::ptr_eq` on
every carried-over column — the only assertion that distinguishes a shared
column from an equal-looking rebuild.
`with_column_overrides_clear_and_null_merge` pins `Clear` (nulls the column,
keeps the raster) against `input_nulls` (nulls the raster).
I tried the same `ptr_eq` assertion end-to-end through `ScalarUdfTester` and
**removed it**: the harness re-materializes the raster argument through
DataFusion's extension-type plumbing, so it measures the harness rather than
the contract.
```
sedona-raster 187 passed; 0 failed
sedona-raster-functions 257 passed; 0 failed
sedona-raster-zarr 66 passed; 0 failed
sedonadb 10 passed; 0 failed
```
fmt and clippy clean; `sedona-raster-gdal` builds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
