feat(rust/sedona-raster-gdal): add RS_FromGDALRaster - #1030
Merged
Conversation
Decode GDAL-readable raster bytes (e.g. GeoTIFF) into an in-db raster — the inverse of RS_AsGeoTiff and the binary counterpart of RS_FromPath. Ported from the GDAL raster draft apache#704 reusing that implementation, but adapted to main: decode via the public open + append_as_indb_raster path (dropping apache#704's test-only load_as_indb_raster and the arrow umbrella concat), accumulating into a single RasterBuilder. Tests reworked onto the RasterSpec/ScalarUdfTester harness with GDAL-generated bytes (no .tiff fixtures); adds a benchmark and docs.
…on this branch) RS_AsGeoTiff's doc page ships in a separate PR, so linking rs_asgeotiff.qmd breaks the mkdocs --strict build here. Reference it as plain text; keep the valid RS_FromPath link.
# Conflicts: # rust/sedona-raster-gdal/Cargo.toml # rust/sedona-raster-gdal/src/lib.rs # rust/sedona-raster-gdal/src/register.rs
…ata_buffer The in-db decode path (append_as_indb_raster, used by RS_FromGDALRaster, RS_Metadata and RS_Polygonize) already handed each band's freshly-read Vec to Arrow without a copy, but did so by open-coding append_block + try_append_view. Route it through the dedicated RasterBuilder helper instead: it still attaches the allocation as a shared data block (a refcount bump, never a copy), and additionally stores sub-inline-threshold bands inline so the view stays canonical (a block-referencing view of <= 12 bytes fails array validation on roundtrip — a latent bug for small bands). Perf-neutral, as expected for an already-zero-copy path. rs_from_gdal_raster criterion bench (GeoTIFF bytes -> in-db raster), decode of test4.tiff: decode/1: 62.8 us -> 63.1 us (within noise threshold) decode/32: 1.741 ms -> 1.746 ms (no change detected)
…raster_spec helpers The decode tests navigated the result by hand (RasterStructArray -> metadata -> band chains) and only checked width/height/band count. Replace that with assert_rasters_equal / assert_raster_scalar_equals against declarative RasterSpec expectations derived from each fixture's own GeoTIFF construction, which additionally pin the geotransform, per-band data type, nodata, in-db storage and exact pixel values. The CRS is read back from the fixture bytes (not copied from a decode result) so the spec pins CRS preservation without hard-coding a PROJJSON blob that drifts across PROJ versions. Also add coverage that was missing: - decoded_two_band_raster_is_zero_copy pins the no-copy property (one band-data buffer per band; a copying path would consolidate them). - error pathways: empty, unparseable and truncated bytes all error cleanly rather than panic.
RS_AsGeoTiff returns BinaryView, and the is_binary matcher accepts both Binary and BinaryView, but the kernel narrowed its input through as_binary_array (i32-offset Binary only). That made RS_FromGDALRaster(RS_AsGeoTiff(...)) fail at execution with a BinaryView -> Binary cast error, breaking the round-trip documented in rs_fromgdalraster.qmd. Read Binary and BinaryView arrays directly instead of narrowing view offsets into Binary's i32 range. Add regression coverage for BinaryView input and for the RS_AsGeoTiff -> RS_FromGDALRaster round trip.
james-willis
marked this pull request as ready for review
July 23, 2026 16:10
…o RS_EnsureLoaded skips re-wrapping
paleolimbot
reviewed
Jul 24, 2026
paleolimbot
left a comment
paleolimbot
left a comment
Member
There was a problem hiding this comment.
Thank you!
Should this be named something without GDAL in the name so it can be replicated in SedonaSpark if it's useful? FromBytes? FromContent?
Does it need a "format" parameter to give GDAL a hint or is GDAL's autodetection sufficient?
paleolimbot
approved these changes
Jul 24, 2026
Contributor
Author
I think the utility of the function is that it can load anything GDAL can. So to signify that we need GDAL in the name. I think we're pretty screwed if we want to reproduce this in spark because of that! I think we need to consider this function outside the scope of sedona spark for the foreseeable future.
My understanding of GDAL is that its autodetection is sufficient; I dont see formats being passed in their tutorials. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RS_FromGDALRaster(binary) -> raster— decode GDAL-readable raster bytes (e.g. a GeoTIFF) into an in-db raster. The inverse ofRS_AsGeoTiffand the binary counterpart ofRS_FromPath(which references a file path as out-db).Split out of the GDAL raster draft #704, reusing that implementation, but adapted to main: decode via the public
open+append_as_indb_rasterpath (dropping #704's test-onlyload_as_indb_rasterhelper and thearrowumbrellaconcat), accumulating rows into a singleRasterBuilder. Tests were reworked onto the RasterSpec /ScalarUdfTesterharness with GDAL-generated bytes — no.tifffixtures. Adds a benchmark and SQL docs.The doc example (round-tripping through
RS_AsGeoTiff) is shown illustratively rather than executed, so the docs build stays independent of that PR.