You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "Kontinuation (via GitHub)" <gi...@apache.org> on 2026/01/20 12:16:47 UTC

[PR] fix(sedona-raster-functions): Handle array inputs in coordinate conversion functions [sedona-db]

Kontinuation opened a new pull request, #532:
URL: https://github.com/apache/sedona-db/pull/532

   Previously, RS_RasterToWorldCoord and RS_WorldToRasterCoord only handled scalar inputs and would error with 'Expected scalar ... argument' when array inputs were passed.
   
   This commit fixes both functions to properly handle array inputs by:
   - Using into_array() to expand scalar inputs to arrays
   - Using arrow_cast::cast() for type conversion
   - Processing all values in a loop with proper null handling
   
   Added test cases for array input handling.


-- 
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] fix(sedona-raster-functions): Handle array inputs in coordinate conversion functions [sedona-db]

Posted by "Kontinuation (via GitHub)" <gi...@apache.org>.
Kontinuation commented on code in PR #532:
URL: https://github.com/apache/sedona-db/pull/532#discussion_r2711250041


##########
rust/sedona-raster-functions/src/rs_rastercoordinate.rs:
##########
@@ -139,17 +141,35 @@ impl SedonaScalarKernel for RsCoordinateMapper {
         let executor = RasterExecutor::new(arg_types, args);
         let mut builder = Int64Builder::with_capacity(executor.num_iterations());
 
-        let coord_opt = extract_scalar_coord(&args[1], &args[2])?;
-        executor.execute_raster_void(|_i, raster_opt| {
-            match (raster_opt, coord_opt) {
-                (Some(raster), (Some(x), Some(y))) => {
+        // Expand world x and y coordinate parameters to arrays and cast to Float64
+        let world_x_array = args[1].clone().cast_to(&DataType::Float64, None)?;
+        let world_x_array = world_x_array.into_array(executor.num_iterations())?;
+        let world_x_array = world_x_array.as_primitive::<Float64Type>();

Review Comment:
   Use as_float64_array to replace `as_primitive`.



-- 
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] fix(sedona-raster-functions): Handle array inputs in coordinate conversion functions [sedona-db]

Posted by "Kontinuation (via GitHub)" <gi...@apache.org>.
Kontinuation commented on code in PR #532:
URL: https://github.com/apache/sedona-db/pull/532#discussion_r2711251247


##########
rust/sedona-raster-functions/src/rs_rastercoordinate.rs:
##########
@@ -139,17 +141,35 @@ impl SedonaScalarKernel for RsCoordinateMapper {
         let executor = RasterExecutor::new(arg_types, args);
         let mut builder = Int64Builder::with_capacity(executor.num_iterations());
 
-        let coord_opt = extract_scalar_coord(&args[1], &args[2])?;
-        executor.execute_raster_void(|_i, raster_opt| {
-            match (raster_opt, coord_opt) {
-                (Some(raster), (Some(x), Some(y))) => {
+        // Expand world x and y coordinate parameters to arrays and cast to Float64
+        let world_x_array = args[1].clone().cast_to(&DataType::Float64, None)?;
+        let world_x_array = world_x_array.into_array(executor.num_iterations())?;
+        let world_x_array = world_x_array.as_primitive::<Float64Type>();
+        let world_y_array = args[2].clone().cast_to(&DataType::Float64, None)?;
+        let world_y_array = world_y_array.into_array(executor.num_iterations())?;
+        let world_y_array = world_y_array.as_primitive::<Float64Type>();
+
+        executor.execute_raster_void(|i, raster_opt| {
+            let x_opt = if world_x_array.is_null(i) {
+                None
+            } else {
+                Some(world_x_array.value(i))
+            };
+            let y_opt = if world_y_array.is_null(i) {
+                None
+            } else {
+                Some(world_y_array.value(i))
+            };

Review Comment:
   Switched to `.next().unwrap()`.



-- 
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] fix(sedona-raster-functions): Handle array inputs in coordinate conversion functions [sedona-db]

Posted by "Kontinuation (via GitHub)" <gi...@apache.org>.
Kontinuation merged PR #532:
URL: https://github.com/apache/sedona-db/pull/532


-- 
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] fix(sedona-raster-functions): Handle array inputs in coordinate conversion functions [sedona-db]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #532:
URL: https://github.com/apache/sedona-db/pull/532#discussion_r2709268691


##########
rust/sedona-raster-functions/src/rs_rastercoordinate.rs:
##########
@@ -139,17 +141,35 @@ impl SedonaScalarKernel for RsCoordinateMapper {
         let executor = RasterExecutor::new(arg_types, args);
         let mut builder = Int64Builder::with_capacity(executor.num_iterations());
 
-        let coord_opt = extract_scalar_coord(&args[1], &args[2])?;
-        executor.execute_raster_void(|_i, raster_opt| {
-            match (raster_opt, coord_opt) {
-                (Some(raster), (Some(x), Some(y))) => {
+        // Expand world x and y coordinate parameters to arrays and cast to Float64
+        let world_x_array = args[1].clone().cast_to(&DataType::Float64, None)?;
+        let world_x_array = world_x_array.into_array(executor.num_iterations())?;
+        let world_x_array = world_x_array.as_primitive::<Float64Type>();

Review Comment:
   Totally optional, but I have mostly been using DataFusion's helpers for these so far (`as_float64_array()`), which return errors in debug mode.



##########
rust/sedona-raster-functions/src/rs_rastercoordinate.rs:
##########
@@ -139,17 +141,35 @@ impl SedonaScalarKernel for RsCoordinateMapper {
         let executor = RasterExecutor::new(arg_types, args);
         let mut builder = Int64Builder::with_capacity(executor.num_iterations());
 
-        let coord_opt = extract_scalar_coord(&args[1], &args[2])?;
-        executor.execute_raster_void(|_i, raster_opt| {
-            match (raster_opt, coord_opt) {
-                (Some(raster), (Some(x), Some(y))) => {
+        // Expand world x and y coordinate parameters to arrays and cast to Float64
+        let world_x_array = args[1].clone().cast_to(&DataType::Float64, None)?;
+        let world_x_array = world_x_array.into_array(executor.num_iterations())?;
+        let world_x_array = world_x_array.as_primitive::<Float64Type>();
+        let world_y_array = args[2].clone().cast_to(&DataType::Float64, None)?;
+        let world_y_array = world_y_array.into_array(executor.num_iterations())?;
+        let world_y_array = world_y_array.as_primitive::<Float64Type>();
+
+        executor.execute_raster_void(|i, raster_opt| {
+            let x_opt = if world_x_array.is_null(i) {
+                None
+            } else {
+                Some(world_x_array.value(i))
+            };
+            let y_opt = if world_y_array.is_null(i) {
+                None
+            } else {
+                Some(world_y_array.value(i))
+            };

Review Comment:
   Also optional, but so far we have mostly implemented this by resolving the iterator before the loop and calling `x_opt.next().unwrap()`. In theory the iterator could be faster than calling `is_null()` and `value()` in a loop but probably isn't in practice.



-- 
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