james-willis commented on PR #749: URL: https://github.com/apache/sedona-db/pull/749#issuecomment-4435018030
**Design note on `raster_ref_to_gdal_mem` lifetime plumbing** The GDAL backend in this PR uses the shim's `BandRef::data() -> &[u8]` rather than `BandRef::contiguous_data() -> Result<Cow<'_, [u8]>>`. That's deliberate, and it lets us drop the `(Dataset, Vec<Vec<u8>>)` tuple return / `_owned_band_bytes: Vec<Vec<u8>>` field on `RasterDataset`. **Why it works in this PR**: every band is identity-viewed, so the bytes GDAL points at are a borrow into the StructArray's backing buffer. `RasterDataset` already keeps the source raster alive via `_source_raster: PhantomData<&'a dyn RasterRef>`, so the pointer is valid for the dataset's lifetime — no extra owned-bytes plumbing needed. Non-identity views error at read time here, so `Cow::Owned` cannot appear. **What changes when the view machinery lands**: once non-identity views are supported, `contiguous_data()` may return `Cow::Owned` for sliced/permuted bands (the reader materializes a new buffer). That `Vec<u8>` will need to outlive the GDAL dataset that points into it. Bringing back the `_owned_band_bytes: Vec<Vec<u8>>` field on `RasterDataset` (and the matching tuple return from `raster_ref_to_gdal_mem`) is the right way to do that — earlier revisions of this PR carried that plumbing for exactly this reason. Short version: if you're picking this code up later to add view support, expect to re-thread owned band bytes from the reader, through `raster_ref_to_gdal_mem`, and onto `RasterDataset`. -- 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]
