You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sedona.apache.org by ja...@apache.org on 2026/09/30 17:49:43 UTC
(sedona-db) branch main updated: fix(rust/sedona-raster-gdal): read a file nodata the band type cannot hold as none (#1357)
This is an automated email from the ASF dual-hosted git repository.
james-willis pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/sedona-db.git
The following commit(s) were added to refs/heads/main by this push:
new 751705633 fix(rust/sedona-raster-gdal): read a file nodata the band type cannot hold as none (#1357)
751705633 is described below
commit 751705633483e7e6fd619bd2e6c89ae24d455702
Author: James Willis <ja...@users.noreply.github.com>
AuthorDate: Wed Sep 30 10:49:37 2026 -0700
fix(rust/sedona-raster-gdal): read a file nodata the band type cannot hold as none (#1357)
---
.../spark-parity/test_rs_bandnodatavalue.py | 47 +++++++++++----
integration/spark-parity/test_rs_value.py | 27 +++++++++
python/sedonadb/python/sedonadb/raster_testing.py | 15 +++--
python/sedonadb/tests/functions/test_rs_value.py | 56 +++++++++++++++---
python/sedonadb/tests/test_raster_testing.py | 21 +++++++
rust/sedona-raster-gdal/src/gdal_common.rs | 66 ++++++++++++++++------
rust/sedona-raster-gdal/src/lib.rs | 2 +-
7 files changed, 192 insertions(+), 42 deletions(-)
diff --git a/integration/spark-parity/test_rs_bandnodatavalue.py b/integration/spark-parity/test_rs_bandnodatavalue.py
index 93a0f4223..d187fc77c 100644
--- a/integration/spark-parity/test_rs_bandnodatavalue.py
+++ b/integration/spark-parity/test_rs_bandnodatavalue.py
@@ -107,24 +107,49 @@ def test_rs_band_nodata_nan(dtype, tmp_path):
compare(sql, sedona, spark)
+def test_rs_band_nodata_nan_on_int_band(tmp_path):
+ """TIFFTAG_GDAL_NODATA is ASCII metadata, so a file can claim a nodata its
+ band dtype cannot hold. NaN on an integer band matches no pixel, and both
+ engines read the band as having no nodata."""
+ sedona, spark = SedonaDB(), SedonaSpark()
+ for eng in (sedona, spark):
+ eng.create_random_raster_view(
+ "nan_int_nd_raster",
+ tmp_path / "nan_int_nd.tif",
+ dtype="uint8",
+ bands=1,
+ nodata=float("nan"),
+ )
+ sql = "SELECT RS_BandNoDataValue(rast, 1) FROM nan_int_nd_raster"
+ compare(sql, sedona, spark, expected=[(None,)])
+
+
+@pytest.mark.parametrize(
+ ("dtype", "nodata"),
+ [
+ pytest.param("uint8", -9999.0, id="below_range"),
+ pytest.param("uint8", 256.0, id="above_range"),
+ pytest.param("int32", 0.5, id="fraction"),
+ ],
+)
@pytest.mark.xfail(
- reason="SedonaDB packs the file nodata into the band dtype (0.5 becomes 0); "
- "Sedona Spark reports the GDAL metadata value verbatim (0.5)"
+ reason="SedonaDB stores nodata in the band dtype, so it reads a file nodata "
+ "the dtype cannot hold as none (NULL); Sedona Spark reports the GDAL "
+ "metadata value verbatim. Both match no pixel (see test_rs_value.py)"
)
-def test_rs_band_nodata_fractional_on_int_band(tmp_path):
- """TIFFTAG_GDAL_NODATA is ASCII metadata, so a file can claim a nodata its
- band dtype cannot hold. rasterio refuses to write one beyond the dtype's
- range, so the writable case is a fractional nodata on an integer band."""
+def test_rs_band_nodata_unrepresentable_on_int_band(dtype, nodata, tmp_path):
+ """A finite file nodata the integer band dtype cannot hold exactly —
+ out of range, or fractional — reads back the same from both engines."""
sedona, spark = SedonaDB(), SedonaSpark()
for eng in (sedona, spark):
eng.create_random_raster_view(
- "frac_nd_raster",
- tmp_path / "frac_nd.tif",
- dtype="int32",
+ "unrep_nd_raster",
+ tmp_path / "unrep_nd.tif",
+ dtype=dtype,
bands=1,
- nodata=0.5,
+ nodata=nodata,
)
- sql = "SELECT RS_BandNoDataValue(rast, 1) FROM frac_nd_raster"
+ sql = "SELECT RS_BandNoDataValue(rast, 1) FROM unrep_nd_raster"
compare(sql, sedona, spark)
diff --git a/integration/spark-parity/test_rs_value.py b/integration/spark-parity/test_rs_value.py
index 4de838af6..e87822f93 100644
--- a/integration/spark-parity/test_rs_value.py
+++ b/integration/spark-parity/test_rs_value.py
@@ -57,6 +57,33 @@ def test_rs_value_nodata_pixel_is_null(tmp_path):
compare(sql, sedona, spark, expected=[(None,)])
+@pytest.mark.parametrize(
+ "dtype,nodata,pixel",
+ [
+ pytest.param("uint8", -9999.0, 0, id="below-range"),
+ pytest.param("uint8", 256.0, 255, id="above-range"),
+ pytest.param("int16", float("nan"), 0, id="nan"),
+ pytest.param("int32", 0.5, 0, id="fraction"),
+ ],
+)
+def test_rs_value_unrepresentable_nodata_matches_no_pixel(
+ dtype, nodata, pixel, tmp_path
+):
+ """A file nodata the band dtype cannot hold exactly matches no pixel on
+ both engines, so the pixel a saturating cast would turn it into (-9999 on
+ uint8 into 0) reads back verbatim rather than NULL."""
+ sedona, spark = _engines(
+ "val_unrep_src",
+ tmp_path,
+ dtype=dtype,
+ bands=1,
+ nodata=nodata,
+ plants={(1, 1): pixel},
+ )
+ sql = "SELECT RS_Value(rast, ST_GeomFromWKT('POINT(103 495.5)'), 1) FROM val_unrep_src"
+ compare(sql, sedona, spark, expected=float(pixel))
+
+
def test_rs_value_outside_extent_is_null(tmp_path):
"""A point outside the raster's extent reads back NULL from both engines."""
sedona, spark = _engines("val_out_src", tmp_path)
diff --git a/python/sedonadb/python/sedonadb/raster_testing.py b/python/sedonadb/python/sedonadb/raster_testing.py
index 0f3266238..fa2335119 100644
--- a/python/sedonadb/python/sedonadb/raster_testing.py
+++ b/python/sedonadb/python/sedonadb/raster_testing.py
@@ -229,10 +229,11 @@ def write_geotiff(
data's shape (rasterio's `transform.from_bounds`) — or `gdal_transform` —
GDAL-order `(origin_x, scale_x, skew_x, origin_y, skew_y, scale_y)`, for
grids a bbox cannot express (skew, south-up).
- `nodata` (optional) becomes the per-band nodata of every band. `crs`
- (optional) is any CRS rasterio accepts; parity fixtures stay CRS-less
- unless an engine requires one, and then use the same CRS everywhere so
- nothing reprojects.
+ `nodata` (optional) becomes the per-band nodata of every band. It may be
+ one the dtype cannot hold (-9999 or NaN on uint8): TIFFTAG_GDAL_NODATA is
+ ASCII, so real files carry such values. `crs` (optional) is any CRS
+ rasterio accepts; parity fixtures stay CRS-less unless an engine requires
+ one, and then use the same CRS everywhere so nothing reprojects.
"""
import rasterio
@@ -246,9 +247,13 @@ def write_geotiff(
count=bands,
dtype=str(data.dtype),
transform=_resolve_transform(bbox, gdal_transform, width=width, height=height),
- nodata=nodata,
crs=crs,
) as dst:
+ # The setter, unlike rasterio.open's `nodata=`, does not reject a
+ # value outside the dtype's range; for any other value the file is
+ # byte-identical.
+ if nodata is not None:
+ dst.nodata = nodata
dst.write(data)
diff --git a/python/sedonadb/tests/functions/test_rs_value.py b/python/sedonadb/tests/functions/test_rs_value.py
index 8131d3541..d13908b44 100644
--- a/python/sedonadb/tests/functions/test_rs_value.py
+++ b/python/sedonadb/tests/functions/test_rs_value.py
@@ -17,8 +17,8 @@
"""RS_Value / RS_Values / RS_BandNoDataValue parity.
-Every test defines the raster once (numpy array + GDAL geotransform, written
-to a CRS-less GeoTIFF) and samples it through SedonaDB and a rasterio
+Every test defines the raster once (numpy array + bbox, written to a
+CRS-less GeoTIFF) and samples it through SedonaDB and a rasterio
reference. The fixture bakes a band nodata into the GeoTIFF *and* plants a
pixel valued at that nodata, so one sweep pins all three None rules together:
nodata-valued pixels sample as None, out-of-extent points sample as None,
@@ -41,9 +41,9 @@ from sedonadb.raster_testing import (
pytest.importorskip("rasterio")
-# GDAL-order geotransform: origin (100, 500), 2-wide by 3-tall north-up
-# pixels; with a 7x6 raster the extent is x in [100, 114], y in [482, 500].
-GDAL_TRANSFORM = (100.0, 2.0, 0.0, 500.0, 0.0, -3.0)
+# The extent (minx, miny, maxx, maxy): a 7x6 raster over it has north-up
+# pixels 2 wide by 3 tall with the origin at (100, 500).
+BBOX = (100.0, 482.0, 114.0, 500.0)
BANDS, HEIGHT, WIDTH = 2, 6, 7
# The pixel planted with the band's own nodata value; sampling it is None.
@@ -78,7 +78,7 @@ def _write_fixture(tmp_path, dtype, *, nodata):
random_raster_data(
dtype, bands=BANDS, height=HEIGHT, width=WIDTH, plants=plants
),
- gdal_transform=GDAL_TRANSFORM,
+ bbox=BBOX,
nodata=nodata,
)
return tiff
@@ -278,7 +278,7 @@ def test_rs_band_nodata_matches_comparators(con, tmp_path, dtype):
write_geotiff(
without_nodata,
random_raster_data(dtype, bands=BANDS, height=HEIGHT, width=WIDTH),
- gdal_transform=GDAL_TRANSFORM,
+ bbox=BBOX,
)
for band in (1, 2):
@@ -289,3 +289,45 @@ def test_rs_band_nodata_matches_comparators(con, tmp_path, dtype):
)
assert _sedonadb_band_nodata(con, without_nodata, band=band) is None
assert _rasterio_band_nodata(without_nodata, band=band) is None
+
+
+# A nodata a GeoTIFF can declare but the band dtype cannot hold exactly, and
+# the real pixel value a saturating cast into the dtype would turn it into.
+UNREPRESENTABLE_NODATA = [
+ pytest.param("uint8", -9999.0, 0, id="below_range"),
+ pytest.param("uint8", 256.0, 255, id="above_range"),
+ pytest.param("int16", float("nan"), 0, id="nan"),
+ pytest.param("int32", 0.5, 0, id="fraction"),
+]
+
+
+@pytest.mark.parametrize("loader", ["RS_FromPath(path)", "RS_FromGDALRaster(content)"])
+@pytest.mark.parametrize(("dtype", "nodata", "saturated"), UNREPRESENTABLE_NODATA)
+def test_unrepresentable_file_nodata_is_dropped(
+ con, tmp_path, loader, dtype, nodata, saturated
+):
+ """A file nodata the band dtype cannot hold exactly matches no pixel, so
+ the band reads as having none: RS_BandNoDataValue is NULL and every pixel
+ samples verbatim, including the planted one a saturating cast would have
+ matched (-9999 on uint8 used to turn every 0 pixel into nodata). Covers
+ both the out-db (RS_FromPath) and in-db (RS_FromGDALRaster) loaders."""
+ data = random_raster_data(
+ dtype, bands=1, height=HEIGHT, width=WIDTH, plants={NODATA_PLANT: saturated}
+ )
+ tiff = tmp_path / "unrepresentable.tif"
+ write_geotiff(tiff, data, bbox=BBOX, nodata=nodata)
+ points = [pixel_center(row, col) for row in range(HEIGHT) for col in range(WIDTH)]
+ wkt = "MULTIPOINT (" + ", ".join(f"{x!r} {y!r}" for x, y in points) + ")"
+ con.create_data_frame(
+ pa.table({"path": [str(tiff)], "content": [tiff.read_bytes()], "wkt": [wkt]})
+ ).to_view("unrepresentable_src", overwrite=True)
+
+ got = con.sql(
+ "SELECT RS_BandNoDataValue(r, 1) AS nodata, "
+ "RS_Values(r, ST_GeomFromText(wkt), 1) AS pixels "
+ f"FROM (SELECT {loader} AS r, wkt FROM unrepresentable_src)"
+ ).to_arrow_table()
+
+ expected = [float(v) for v in data[0].ravel()]
+ assert _rasterio_values(tiff, points) == expected
+ assert got.to_pylist() == [{"nodata": None, "pixels": expected}]
diff --git a/python/sedonadb/tests/test_raster_testing.py b/python/sedonadb/tests/test_raster_testing.py
index 33ffe6bf4..21a8ccb91 100644
--- a/python/sedonadb/tests/test_raster_testing.py
+++ b/python/sedonadb/tests/test_raster_testing.py
@@ -22,6 +22,8 @@ bbox-constructed `DecodedRaster`s), so a wrong bbox-derived geotransform
would agree with itself there.
"""
+import re
+
import numpy as np
import pytest
@@ -57,6 +59,25 @@ def test_decoded_raster_write_geotiff_requires_uniform_nodata(tmp_path):
raster.write_geotiff(tmp_path / "nonuniform.tif")
+@pytest.mark.parametrize(
+ ("dtype", "nodata"),
+ [("uint8", -9999.0), ("uint8", 256.0), ("int16", float("nan")), ("int32", 0.5)],
+)
+def test_write_geotiff_writes_nodata_the_dtype_cannot_hold(tmp_path, dtype, nodata):
+ """The declared nodata lands in the file even when the dtype cannot hold
+ it. rasterio's `nodatavals` reports None for an out-of-range one, so read
+ GDAL's own view of the band back through a VRT copy."""
+ pytest.importorskip("rasterio")
+ import rasterio.shutil
+
+ path = tmp_path / "unrepresentable.tif"
+ write_geotiff(path, np.zeros((1, 6, 7), dtype=dtype), bbox=BBOX, nodata=nodata)
+ rasterio.shutil.copy(str(path), str(tmp_path / "copy.vrt"), driver="VRT")
+ vrt = (tmp_path / "copy.vrt").read_text()
+ (written,) = re.findall(r"<NoDataValue>(.*)</NoDataValue>", vrt)
+ np.testing.assert_equal(float(written), nodata) # NaN equals NaN here
+
+
def test_write_geotiff_bbox_places_the_grid(tmp_path):
pytest.importorskip("rasterio")
path = tmp_path / "bbox.tif"
diff --git a/rust/sedona-raster-gdal/src/gdal_common.rs b/rust/sedona-raster-gdal/src/gdal_common.rs
index 86f6bbc8a..fb3d3efe5 100644
--- a/rust/sedona-raster-gdal/src/gdal_common.rs
+++ b/rust/sedona-raster-gdal/src/gdal_common.rs
@@ -25,7 +25,7 @@ use sedona_gdal::raster::types::DatasetOptions;
use sedona_gdal::raster::types::GdalDataType;
use sedona_raster::geo_transform::GeoTransform;
-use sedona_raster::traits::{RasterRef, is_spatial_dim_pair};
+use sedona_raster::traits::{RasterRef, is_spatial_dim_pair, nodata_f64_to_bytes};
use sedona_schema::raster::BandDataType;
use datafusion_common::{DataFusionError, Result, exec_datafusion_err, exec_err};
@@ -418,6 +418,14 @@ pub fn nodata_bytes_to_f64(nodata_bytes: Option<&[u8]>, band_type: &BandDataType
}
/// Read a GDAL band's nodata value into a byte vector using the band's native type.
+///
+/// GDAL reports nodata as an `f64` (for GeoTIFF, the ASCII `TIFFTAG_GDAL_NODATA`
+/// tag), so a file can declare a value its band type cannot hold: -9999 or NaN on
+/// a UInt8 band, or 0.5 on an Int32 band. No pixel can equal such a value, so the
+/// band reads as having no nodata. Packing it into the band type instead would
+/// saturate -9999 to 0 and turn every real 0 pixel into nodata. Sedona Spark
+/// matches no pixel either, and GDAL's own mask band ignores an out-of-range
+/// nodata the same way (though it truncates an in-range fraction such as 0.5).
pub fn band_nodata_to_bytes(band: &RasterBand<'_>) -> Result<Option<Vec<u8>>> {
let band_type = gdal_to_band_data_type(band.band_type())?;
@@ -430,7 +438,7 @@ pub fn band_nodata_to_bytes(band: &RasterBand<'_>) -> Result<Option<Vec<u8>>> {
.map(|nodata| nodata.to_le_bytes().to_vec()),
_ => band
.no_data_value()
- .map(|nodata| nodata_f64_to_bytes(nodata, &band_type)),
+ .and_then(|nodata| nodata_f64_to_bytes(nodata, &band_type).ok()),
})
}
@@ -465,22 +473,6 @@ pub fn set_band_nodata_from_bytes(
}
}
-/// Convert a f64 nodata value into a byte vector appropriate for the given band type.
-pub fn nodata_f64_to_bytes(nodata: f64, band_type: &BandDataType) -> Vec<u8> {
- match band_type {
- BandDataType::UInt8 => vec![(nodata as u8)],
- BandDataType::Int8 => (nodata as i8).to_le_bytes().to_vec(),
- BandDataType::UInt16 => (nodata as u16).to_le_bytes().to_vec(),
- BandDataType::Int16 => (nodata as i16).to_le_bytes().to_vec(),
- BandDataType::UInt32 => (nodata as u32).to_le_bytes().to_vec(),
- BandDataType::Int32 => (nodata as i32).to_le_bytes().to_vec(),
- BandDataType::UInt64 => (nodata as u64).to_le_bytes().to_vec(),
- BandDataType::Int64 => (nodata as i64).to_le_bytes().to_vec(),
- BandDataType::Float32 => (nodata as f32).to_le_bytes().to_vec(),
- BandDataType::Float64 => nodata.to_le_bytes().to_vec(),
- }
-}
-
/// Open an out-db raster source as a GDAL dataset, normalizing the URL to a GDAL VSI path if needed.
pub fn open_gdal_dataset(gdal: &Gdal, url: &str, open_options: Option<&[&str]>) -> Result<Dataset> {
let normalized_url = normalize_outdb_source_path(url);
@@ -735,6 +727,44 @@ mod tests {
assert!(bytes_to_f64(&[1, 2, 3], &BandDataType::UInt8).is_err());
}
+ #[test]
+ fn test_band_nodata_to_bytes_drops_unrepresentable_nodata() {
+ // GDAL reports the nodata as an f64; one the band type can't hold
+ // exactly reads back as no nodata instead of saturating into a real
+ // pixel value (-9999 on UInt8 used to become 0).
+ with_gdal(|gdal| {
+ let driver = gdal.get_driver_by_name("MEM").unwrap();
+ let uint8 = driver.create_with_band_type::<u8>("", 1, 1, 1).unwrap();
+ let int16 = driver.create_with_band_type::<i16>("", 1, 1, 1).unwrap();
+ let float32 = driver.create_with_band_type::<f32>("", 1, 1, 1).unwrap();
+ for (dataset, nodata, expected) in [
+ (&uint8, -9999.0, None),
+ (&uint8, 256.0, None),
+ (&uint8, f64::NAN, None),
+ (&uint8, 0.5, None),
+ (&uint8, 0.0, Some(vec![0u8])),
+ (&uint8, 255.0, Some(vec![255u8])),
+ (&int16, -32769.0, None),
+ (&int16, -32768.0, Some(i16::MIN.to_le_bytes().to_vec())),
+ // A Float32 band rounds to the nearest f32, as GDAL does.
+ (&float32, 0.1, Some(0.1f32.to_le_bytes().to_vec())),
+ ] {
+ let band = dataset.rasterband(1).unwrap();
+ band.set_no_data_value(Some(nodata)).unwrap();
+ // GDAL keeps whatever was declared; the conversion decides.
+ assert!(band.no_data_value().is_some());
+ assert_eq!(
+ band_nodata_to_bytes(&band)?,
+ expected,
+ "{:?} nodata {nodata}",
+ band.band_type()
+ );
+ }
+ Ok(())
+ })
+ .unwrap();
+ }
+
#[test]
fn test_normalize_outdb_source_path_local_and_vsi_paths() {
assert_eq!(
diff --git a/rust/sedona-raster-gdal/src/lib.rs b/rust/sedona-raster-gdal/src/lib.rs
index fe3d4f3fb..25fa5eba5 100644
--- a/rust/sedona-raster-gdal/src/lib.rs
+++ b/rust/sedona-raster-gdal/src/lib.rs
@@ -48,7 +48,7 @@ mod utils;
// Re-export main dataset conversion functions
pub use gdal_common::{
GdalBandLayout, GdalBandPlan, band_data_type_to_gdal, bytes_to_f64, gdal_to_band_data_type,
- gdal_type_byte_size, nodata_bytes_to_f64, nodata_f64_to_bytes,
+ gdal_type_byte_size, nodata_bytes_to_f64,
};
pub use raster_loader::{GDAL_FORMAT, GdalLoader};
pub use rs_as_geotiff::rs_as_geotiff_udf;