james-willis opened a new issue, #3311: URL: https://github.com/apache/sedona/issues/3311
## Expected behavior Per the [RS_SetBandNoDataValue docs](https://sedona.apache.org/latest/api/sql/Raster-Operators/RS_SetBandNoDataValue/): > Passing a `null` value for `noDataValue` will remove the no data value and that will ensure all pixels are included in functions rather than excluded as no data. So `RS_SetBandNoDataValue(raster, 1, NULL)` should return the input raster with band 1's no-data value removed. The common Java implementation (`RasterBandEditors.setBandNoDataValue`) actually implements this: its `noDataValue == null` branch strips the no-data value from the sample dimension and returns the cloned raster. This is also the semantics of PostGIS [`ST_SetBandNoDataValue`](https://postgis.net/docs/RT_ST_SetBandNoDataValue.html) ("To mark a band as having no nodata value, set the nodata value = NULL"), and the semantics SedonaDB adopted in apache/sedona-db#1158, so Sedona Spark is currently inconsistent with both. ## Actual behavior From Spark SQL, `RS_SetBandNoDataValue(raster, 1, NULL)` (and the 2-argument form with a NULL no-data value) returns a NULL raster. The documented "clear the no-data value" code path is unreachable from SQL. Cause: the `RS_SetBandNoDataValue` catalyst expression wraps the Java function with `inferrableFunction2/3/4` (`InferrableFunctionConverter.scala`), and those evaluators return `null` as soon as *any* argument is null — the wrapped Java function is never invoked, so its `noDataValue == null` branch never runs. The existing test in `rasteralgebraTest.scala` ("Passed RS_SetBandNoDataValue with raster") cannot distinguish the two behaviors: ```scala val actualNull = df.selectExpr("RS_BandNoDataValue(RS_SetBandNoDataValue(raster, 1, null))").first().get(0) assertNull(actualNull) ``` `RS_BandNoDataValue` returns null both when the raster's no-data value was cleared **and** when the raster itself is null, so the assertion passes under either semantics. ## Proposed fix The codebase already has a pattern for this: `InferrableFunction.allowRightNull` (`InferredExpression.scala`), used by `ST_MakePolygon`, `ST_CollectionExtract`, etc., which lets the trailing argument be null while still null-propagating on a null first argument. It currently only exists for arity 2. - Add an arity-3 variant (allowing the third argument to be null, still returning null if the raster or band index is null). - Use it for the 2- and 3-argument overloads of `RS_SetBandNoDataValue`. - Leave the 4-argument `replace` overload null-propagating: replacing pixel values with a null no-data value is meaningless, and the docs can note that clearing requires the 2-/3-argument form. - Strengthen the test so it distinguishes clearing from null-propagation (assert the returned raster itself is non-null, then that `RS_BandNoDataValue` on it is null). I have a patch ready and will open a PR. Related (not duplicates): #1269 and #1148 both stem from confusion around the `replace` variant and null no-data values, which this inconsistency feeds into. ## Steps to reproduce ```sql SELECT RS_SetBandNoDataValue( RS_SetBandNoDataValue( RS_MakeEmptyRaster(1, 20, 20, 2, 22, 2, 3, 1, 1, 0), -999), NULL) IS NULL; -- returns true; expected: a non-null raster with no no-data value on band 1 ``` ## Sedona version master (2.0.0-SNAPSHOT); behavior dates back to the introduction of the function (v1.5.0) ## Apache Spark version 3.5 (applies to all supported versions — the affected code is in `spark/common`) ## API type SQL ## Scala version 2.12 ## JRE version 17 ## Environment any ## Existing issues - [x] I searched the existing issues and did not find a duplicate. -- 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]
