You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "james-willis (via GitHub)" <gi...@apache.org> on 2026/05/15 20:28:58 UTC
[PR] feat(raster): transparent OutDb byte loading via a GDAL loader hook [sedona-db]
james-willis opened a new pull request, #849:
URL: https://github.com/apache/sedona-db/pull/849
## Summary
Make `BandRef::nd_buffer()`, `contiguous_data()`, and `data()` Just Work for OutDb bands (bands whose Arrow `data` column is empty and whose `outdb_uri` points elsewhere). Today those methods return `NotYetImplemented`, blocking every UDF and downstream consumer from reading OutDb pixel bytes — the entire reason OutDb references exist as a schema feature is moot without a working byte path.
Approach: a single statically-typed function-pointer hook in `sedona-raster`, populated by `sedona-raster-gdal` at session bootstrap. No trait, no HashMap-keyed registry, no async — \"compiled in, not pluggable\". The only abstraction introduced is the one the crate-boundary forces (`sedona-raster-gdal` depends on `sedona-raster`, not the reverse, so a direct call from `BandRefImpl` to GDAL is impossible).
## Changes
- **New** `rust/sedona-raster/src/outdb_loader.rs` — `OutDbLoadRequest`, `OutDbBandLoader` fn-pointer type, `OnceLock`, `set_outdb_band_loader`, internal `load_outdb` dispatcher.
- **Modified** `rust/sedona-raster/src/array.rs` — `BandRefImpl` gains `outdb_loaded: OnceCell<Vec<u8>>` (lifetime anchor, not a cross-band cache). `nd_buffer()`, `contiguous_data()`, `data()` route through a new `source_bytes()` helper that zero-copies from the Arrow `data` column for schema-InDb bands and falls back to the loader hook (anchored in `outdb_loaded`) for schema-OutDb bands. The legacy `data()` accessor collapses errors to `&[]` to preserve the pre-N-D contract.
- **New** `rust/sedona-raster-gdal/src/outdb_loader.rs` — `gdal_load` impl using the existing `GDALDatasetCache::get_or_create_outdb_source` (thread-local LRU, VSI translation, `#band=N` fragment handling). `pub fn register_outdb_loader()` registers it into the hook.
- **Modified** `rust/sedona/src/context.rs` — calls `sedona_raster_gdal::register_outdb_loader()` from `SedonaContext::new_from_context()` next to the existing function-set registration.
## Tests
`sedona-raster` (mock loader): five `outdb_loader` unit tests cover loader registration, returned-bytes round-trip, per-band caching, missing-uri error, undersized-loader-output error, and loader-failure propagation.
`sedona-raster-gdal` (real GDAL): three integration tests write tiny GeoTIFFs to a temp dir and verify the end-to-end byte path:
- `loads_outdb_band_bytes_from_geotiff` — 4×3 single-band tiff, `band.nd_buffer().buffer` matches the file bytes.
- `second_call_on_same_band_reuses_cache` — verifies `BandRefImpl.outdb_loaded` reuse on a second `nd_buffer()` call.
- `band_fragment_selects_correct_band` — two-band tiff with `#band=N` fragment selects the correct band.
## Relationship to PR #813 (view machinery)
This PR is **independent of PR-D** (#813 — view machinery / `materialized` cell / strided walk) and based directly on `main`. The view-composition path remains rejected at construction in `RasterRefImpl::band()`, so the OutDb byte path here only needs to handle the identity-view case.
If PR-D lands first, this PR will need a small follow-up to integrate OutDb bytes with the strided walk in `data()` (the byte-access surface is the soft conflict — both PRs rewrite `data()` / `nd_buffer()` / `contiguous_data()`). If this PR lands first, PR-D folds the second `OnceCell` into its existing materialization pattern. Either order is manageable; there is no hard duplication.
## Test plan
- [x] \`cargo test -p sedona-raster --lib\` (74 passing)
- [x] \`cargo test -p sedona-raster-gdal --lib outdb_loader\` (3 passing)
- [x] \`cargo clippy --all-targets -p sedona-raster -p sedona-raster-gdal -p sedona -- -D warnings\`
- [x] \`cargo fmt --all -- --check\`
- [ ] Smoke test: end-to-end SQL against an OutDb raster reading bytes through an `RS_*` kernel.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster): transparent OutDb byte loading via a GDAL loader hook [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on PR #849:
URL: https://github.com/apache/sedona-db/pull/849#issuecomment-4567338840
Superseded by #886.
The design pivoted from a transparent-sync hook on `BandRef` accessors with a single fn-pointer registry to:
- An async `AsyncByteLoader` trait + format-keyed `OutDbLoaderRegistry` in `sedona-raster`
- An explicit `RS_EnsureLoaded(raster) -> raster` async UDF that materialises OutDb bytes on demand
- A `needs_bytes` annotation on `SedonaScalarUDF` + an analyzer rule that wraps raster args of `needs_bytes` UDFs with `RS_EnsureLoaded`, with DataFusion's `CommonSubexprEliminate` doing dedup
- `BandRef::nd_buffer()` / `contiguous_data()` / `data()` unchanged from `main` — accessors return whatever is in the `data` column verbatim, no hidden I/O
The GDAL byte-read core from this PR survives as `sedona-raster-gdal::GdalLoader` (Commit 5 of #886). The Zarr backend lands alongside (Commit 6) so both delivery modes — compiled-in and out-of-tree plugin — are exercised in the same PR.
Closing this PR rather than rebasing because the trait surface and registry shape differ enough that a fresh history reviews better than a morph.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster): transparent OutDb byte loading via a GDAL loader hook [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on PR #849:
URL: https://github.com/apache/sedona-db/pull/849#issuecomment-4502786327
Converting back to draft while the design pivots. Three motivations driving the rewrite:
**No magic loading.** Today byte fetching happens transparently inside `Band::nd_buffer` / `Band::contiguous_data`. That hides I/O from `EXPLAIN`, hides it from the optimiser, and produces a callstack-rooted exception when a network fetch fails. The replacement is an explicit `RS_EnsureLoaded(raster) -> raster` UDF that the optimiser auto-injects ahead of `RS_*` functions that need pixel bytes. Band accessors return whatever's in `data` verbatim — no hidden side effects.
**Async UDF perf gains.** DataFusion 52.5's `AsyncScalarUDFImpl` + `AsyncFuncExec` give us cooperative scheduling, `buffer_unordered` intra-batch fan-out, bounded memory via `ideal_batch_size`, and cancellation for free. A sync transparent loader can't access any of that — every fetch blocks one tokio worker and there's no way to parallelise N row fetches into one batch.
**Plugin story for additional formats.** The single global function-pointer registry becomes a format-keyed trait-object registry living in `sedona-raster`. Format crates (this PR's GDAL, the in-flight Zarr work, future COG / Icechunk / …) register themselves under `outdb_format` keys; the `sedona` crate consumes the registry from `RS_EnsureLoaded` without depending on any specific format crate. Adding a backend becomes a new entry in the registry, not a change to consumer code.
Survives from this PR as-is: the GDAL backend impl and the session-bootstrap registration scaffolding. Goes away: the transparent dispatch hook on the `Band` accessor; the single-loader API. Will reopen as the rewrite once the new infra lands.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster): transparent OutDb byte loading via a GDAL loader hook [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis closed pull request #849: feat(raster): transparent OutDb byte loading via a GDAL loader hook
URL: https://github.com/apache/sedona-db/pull/849
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org