james-willis opened a new pull request, #1161:
URL: https://github.com/apache/sedona-db/pull/1161
## What
Extracts `RasterBuilder`'s per-band column state and writing methods into a
new, standalone `BandArrayBuilder` (`band_builder.rs`) — one row per band, with
none of the raster-envelope concerns (`crs`, `transform`,
`spatial_dims`/`spatial_shape`, or grouping bands into a per-raster list).
`RasterBuilder` now holds one internally and delegates to it, layering its own
per-raster bookkeeping on top.
`BandRef::copy_into`/`append_data_into` now target a new `BandWriter` trait
object (`&mut dyn BandWriter`) instead of a concrete `&mut RasterBuilder`.
`RasterBuilder` implements `BandWriter`, so every existing call site behaves
exactly as before — but `copy_into` can now also target a bare
`BandArrayBuilder` directly, with no raster envelope at all.
Pure internal refactor: no schema change, no new column, no behavior change
for any existing caller. `RasterBuilder`'s public inherent method names and
signatures are all unchanged.
## Why
A derived band value with no enclosing raster — e.g. a tagged extension type
reusing the Band physical layout, one row per value rather than one row per
multi-band raster — currently has no way to reuse `copy_into`'s
view-composition/override-resolution logic, or the builder-side column
assembly, without wrapping and then unwrapping a throwaway single-band
`Raster`. The hard parts of that (`ViewEntries::compose`, `BandRef`'s default
methods) were already reusable as a `sedona-raster` dependency; this closes the
gap on the write side, where `start_band`/`copy_into` were hardcoded to the
full raster envelope.
## How
- `band_builder.rs` (new): `BandArrayBuilder` — the moved-verbatim
band-column builders (`name`, `dim_names`, `shape`, `datatype`, `nodata`,
`view`, `outdb_uri`/`format`, `data`) plus
`start_band`/`finish_band`/`band_data_writer`/`append_band_data_buffer`/`append_band_data_from`/`finish`.
`finish()` returns the flat band `StructArray` directly — one row per band
appended, no outer `Raster` wrapper. Also defines `BandWriter`, the trait
`copy_into`/`append_data_into` target.
- `builder.rs`: `RasterBuilder` holds `bands: BandArrayBuilder`; every
method that used to touch band columns directly now delegates to it.
`start_band`'s bookkeeping (`current_band_count`, `current_raster_bands` for
`finish_raster`'s spatial-grid validation) is factored into a private
`start_band_and_record`, shared between the inherent `start_band` (unchanged
signature, used by ~20 call sites across the raster function crates) and the
new `impl BandWriter for RasterBuilder` (used when `copy_into` targets a
`RasterBuilder` through the trait object) — both delegate to it rather than to
each other, so there's no same-name inherent-vs-trait self-call to disambiguate.
- `traits.rs`/`array.rs`: `copy_into`'s default impl and its one override
(`BandRefImpl::append_data_into`) now take `&mut dyn BandWriter`.
## What's NOT in it
- No reader-side change. `BandRef` is unchanged; a future non-Raster
consumer (e.g. a Tensor-shaped extension type) implements it directly against
its own tagged struct array, same as any `BandRef` impl does today.
- No `chunk_index`/schema work. This is independent of and unblocked by that
discussion.
## Verification
- `cargo test -p sedona-raster`: 177 passed (175 existing, unmodified, plus
2 new — `bare_band_array_builder_produces_expected_struct_array` proves
`BandArrayBuilder` builds a real band row with no raster envelope at all;
`copy_into_targets_a_bare_band_array_builder` proves `copy_into` works end to
end against a bare `BandArrayBuilder` target, the actual point of this
refactor), 0 failed. The existing module-level doctest on `RasterBuilder`
(using `band_data_writer()`) passes unmodified.
- `cargo test -p {sedona-raster-functions, sedona-raster-zarr,
sedona-spatial-join-raster, sedona-spatial-join, sedona-testing}`: all passing,
0 regressions — every crate depending on
`RasterBuilder`/`StartBandArgs`/`BandOverrides` that can be built on this
machine.
- `sedona-raster-gdal`/`sedona-gdal` could not be checked on this machine —
blocked by a pre-existing, unrelated local GDAL-version/`gdal-sys`-bindgen
mismatch (homebrew GDAL 3.13.1 vs. `gdal-sys`'s pre-built binding versions),
reproducible identically on a clean `main` checkout. Their call sites
(`rs_clip.rs`, `rs_tile.rs`, `rs_frompath.rs`, etc.) use only unchanged
inherent `RasterBuilder` methods (`start_band`, `finish_band`,
`band_data_writer`, `finish`, …) — none override `copy_into`/`append_data_into`
(confirmed: the only such override anywhere in the workspace is `BandRefImpl`
in `sedona-raster/src/array.rs`, updated here).
- `cargo clippy --all-targets -- -D warnings` and `cargo fmt --all --
--check`: clean across all of the above.
--
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]
