paleolimbot commented on code in PR #33: URL: https://github.com/apache/sedona-db/pull/33#discussion_r2330473797
########## rust/sedona-geo/src/st_centroid.rs: ########## @@ -0,0 +1,150 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::sync::Arc; + +use arrow_array::builder::BinaryBuilder; +use datafusion_common::error::Result; +use datafusion_expr::ColumnarValue; +use geo_generic_alg::Centroid; +use sedona_expr::scalar_udf::{ArgMatcher, ScalarKernelRef, SedonaScalarKernel}; +use sedona_functions::executor::WkbExecutor; +use sedona_functions::st_isempty::is_wkb_empty; +use sedona_schema::datatypes::{SedonaType, WKB_GEOMETRY}; +use wkb::reader::Wkb; + +use geo_traits::Dimensions; +use sedona_geometry::wkb_factory; + +/// ST_Centroid() implementation using centroid extraction +pub fn st_centroid_impl() -> ScalarKernelRef { + Arc::new(STCentroid {}) +} + +#[derive(Debug)] +struct STCentroid {} + +impl SedonaScalarKernel for STCentroid { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + let matcher = ArgMatcher::new(vec![ArgMatcher::is_geometry()], WKB_GEOMETRY); + + matcher.match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = WkbExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity(executor.num_iterations(), 1024); + executor.execute_wkb_void(|maybe_wkb| { + match maybe_wkb { + Some(wkb) => { + let centroid_wkb = invoke_scalar(&wkb)?; + builder.append_value(¢roid_wkb); + } + _ => builder.append_null(), + } + + Ok(()) + })?; + + executor.finish(Arc::new(builder.finish())) + } +} + +fn invoke_scalar(wkb: &Wkb) -> Result<Vec<u8>> { + // Check for empty geometries first - they should return POINT EMPTY + if is_wkb_empty(wkb)? { + let mut empty_point_wkb = Vec::new(); + wkb_factory::write_wkb_empty_point(&mut empty_point_wkb, Dimensions::Xy) + .map_err(|e| datafusion_common::error::DataFusionError::External(Box::new(e)))?; + return Ok(empty_point_wkb); + } + + // Use Centroid trait directly on WKB, similar to how st_area uses unsigned_area() + if let Some(centroid_point) = wkb.centroid() { + // Extract coordinates from the centroid point + let x = centroid_point.x(); + let y = centroid_point.y(); + + wkb_factory::wkb_point((x, y)) + .map_err(|e| datafusion_common::error::DataFusionError::External(Box::new(e))) + } else { + // This should not happen for non-empty geometries, return POINT EMPTY as fallback + let mut empty_point_wkb = Vec::new(); + wkb_factory::write_wkb_empty_point(&mut empty_point_wkb, Dimensions::Xy) + .map_err(|e| datafusion_common::error::DataFusionError::External(Box::new(e)))?; + Ok(empty_point_wkb) + } +} + +#[cfg(test)] +mod tests { + use arrow_array::{Array, BinaryArray}; + use datafusion_common::scalar::ScalarValue; + use rstest::rstest; + use sedona_functions::register::stubs::st_centroid_udf; + use sedona_schema::datatypes::{WKB_GEOMETRY, WKB_VIEW_GEOMETRY}; + use sedona_testing::testers::ScalarUdfTester; + use wkb::reader::Wkb; + + use super::*; + + #[rstest] + fn udf(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType) { + let mut udf = st_centroid_udf(); + udf.add_kernel(st_centroid_impl()); + let tester = ScalarUdfTester::new(udf.into(), vec![sedona_type]); + + assert_eq!(tester.return_type().unwrap(), WKB_GEOMETRY); + + // Test with a polygon + let result = tester + .invoke_wkb_scalar(Some("POLYGON ((0 0, 2 0, 2 2, 0 2, 0 0))")) + .unwrap(); + + if let ScalarValue::Binary(Some(wkb_data)) = result { + let wkb = Wkb::try_new(&wkb_data).unwrap(); + if let Some(centroid_point) = wkb.centroid() { + assert_eq!(centroid_point.x(), 1.0); + assert_eq!(centroid_point.y(), 1.0); + } else { + panic!("Expected centroid point"); + } + } else { + panic!("Expected Binary result"); + } Review Comment: ```suggestion let result = tester .invoke_scalar("POLYGON ((0 0, 2 0, 2 2, 0 2, 0 0))") .unwrap(); tester.assert_scalar_result(result, "POINT (1 1)"); ``` ########## rust/sedona-geo/src/st_centroid.rs: ########## @@ -0,0 +1,150 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::sync::Arc; + +use arrow_array::builder::BinaryBuilder; +use datafusion_common::error::Result; +use datafusion_expr::ColumnarValue; +use geo_generic_alg::Centroid; +use sedona_expr::scalar_udf::{ArgMatcher, ScalarKernelRef, SedonaScalarKernel}; +use sedona_functions::executor::WkbExecutor; +use sedona_functions::st_isempty::is_wkb_empty; +use sedona_schema::datatypes::{SedonaType, WKB_GEOMETRY}; +use wkb::reader::Wkb; + +use geo_traits::Dimensions; +use sedona_geometry::wkb_factory; + +/// ST_Centroid() implementation using centroid extraction +pub fn st_centroid_impl() -> ScalarKernelRef { + Arc::new(STCentroid {}) +} + +#[derive(Debug)] +struct STCentroid {} + +impl SedonaScalarKernel for STCentroid { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + let matcher = ArgMatcher::new(vec![ArgMatcher::is_geometry()], WKB_GEOMETRY); + + matcher.match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = WkbExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity(executor.num_iterations(), 1024); Review Comment: I believe we have a named constant in `sedona-geometry/src/wkb-factory` for the minimum probable WKB element size per iteration. (I think most of the functions that have geometry output are using it) ########## rust/sedona-geo/src/st_centroid.rs: ########## @@ -0,0 +1,150 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::sync::Arc; + +use arrow_array::builder::BinaryBuilder; +use datafusion_common::error::Result; +use datafusion_expr::ColumnarValue; +use geo_generic_alg::Centroid; +use sedona_expr::scalar_udf::{ArgMatcher, ScalarKernelRef, SedonaScalarKernel}; +use sedona_functions::executor::WkbExecutor; +use sedona_functions::st_isempty::is_wkb_empty; +use sedona_schema::datatypes::{SedonaType, WKB_GEOMETRY}; +use wkb::reader::Wkb; + +use geo_traits::Dimensions; +use sedona_geometry::wkb_factory; + +/// ST_Centroid() implementation using centroid extraction +pub fn st_centroid_impl() -> ScalarKernelRef { + Arc::new(STCentroid {}) +} + +#[derive(Debug)] +struct STCentroid {} + +impl SedonaScalarKernel for STCentroid { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + let matcher = ArgMatcher::new(vec![ArgMatcher::is_geometry()], WKB_GEOMETRY); + + matcher.match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = WkbExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity(executor.num_iterations(), 1024); + executor.execute_wkb_void(|maybe_wkb| { + match maybe_wkb { + Some(wkb) => { + let centroid_wkb = invoke_scalar(&wkb)?; + builder.append_value(¢roid_wkb); + } + _ => builder.append_null(), + } + + Ok(()) + })?; + + executor.finish(Arc::new(builder.finish())) + } +} + +fn invoke_scalar(wkb: &Wkb) -> Result<Vec<u8>> { + // Check for empty geometries first - they should return POINT EMPTY + if is_wkb_empty(wkb)? { + let mut empty_point_wkb = Vec::new(); + wkb_factory::write_wkb_empty_point(&mut empty_point_wkb, Dimensions::Xy) + .map_err(|e| datafusion_common::error::DataFusionError::External(Box::new(e)))?; + return Ok(empty_point_wkb); + } + + // Use Centroid trait directly on WKB, similar to how st_area uses unsigned_area() + if let Some(centroid_point) = wkb.centroid() { + // Extract coordinates from the centroid point + let x = centroid_point.x(); + let y = centroid_point.y(); + + wkb_factory::wkb_point((x, y)) + .map_err(|e| datafusion_common::error::DataFusionError::External(Box::new(e))) + } else { + // This should not happen for non-empty geometries, return POINT EMPTY as fallback + let mut empty_point_wkb = Vec::new(); + wkb_factory::write_wkb_empty_point(&mut empty_point_wkb, Dimensions::Xy) + .map_err(|e| datafusion_common::error::DataFusionError::External(Box::new(e)))?; + Ok(empty_point_wkb) + } +} + +#[cfg(test)] +mod tests { + use arrow_array::{Array, BinaryArray}; + use datafusion_common::scalar::ScalarValue; + use rstest::rstest; + use sedona_functions::register::stubs::st_centroid_udf; + use sedona_schema::datatypes::{WKB_GEOMETRY, WKB_VIEW_GEOMETRY}; + use sedona_testing::testers::ScalarUdfTester; + use wkb::reader::Wkb; + + use super::*; + + #[rstest] + fn udf(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType) { + let mut udf = st_centroid_udf(); + udf.add_kernel(st_centroid_impl()); + let tester = ScalarUdfTester::new(udf.into(), vec![sedona_type]); + + assert_eq!(tester.return_type().unwrap(), WKB_GEOMETRY); + + // Test with a polygon + let result = tester + .invoke_wkb_scalar(Some("POLYGON ((0 0, 2 0, 2 2, 0 2, 0 0))")) + .unwrap(); + + if let ScalarValue::Binary(Some(wkb_data)) = result { + let wkb = Wkb::try_new(&wkb_data).unwrap(); + if let Some(centroid_point) = wkb.centroid() { + assert_eq!(centroid_point.x(), 1.0); + assert_eq!(centroid_point.y(), 1.0); + } else { + panic!("Expected centroid point"); + } + } else { + panic!("Expected Binary result"); + } + + // Test with array + let input_wkt = vec![ + Some("POINT(1 2)"), + None, + Some("POLYGON ((0 0, 2 0, 2 2, 0 2, 0 0))"), + ]; + let result_array = tester.invoke_wkb_array(input_wkt).unwrap(); + let binary_array = result_array.as_any().downcast_ref::<BinaryArray>().unwrap(); + + // First element: POINT(1 2) - centroid should be (1, 2) + assert!(!binary_array.value(0).is_empty()); + // Second element: NULL + assert!(binary_array.is_null(1)); + // Third element: POLYGON centroid should be (1, 1) + assert!(!binary_array.value(2).is_empty()); Review Comment: ```suggestion assert_array_equal(&result_array, &create_array(&[Some("POINT (1 2)"), None, Some("POINT (1 1)")]); ``` ########## rust/sedona-geo/src/st_centroid.rs: ########## @@ -0,0 +1,150 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::sync::Arc; + +use arrow_array::builder::BinaryBuilder; +use datafusion_common::error::Result; +use datafusion_expr::ColumnarValue; +use geo_generic_alg::Centroid; +use sedona_expr::scalar_udf::{ArgMatcher, ScalarKernelRef, SedonaScalarKernel}; +use sedona_functions::executor::WkbExecutor; +use sedona_functions::st_isempty::is_wkb_empty; +use sedona_schema::datatypes::{SedonaType, WKB_GEOMETRY}; +use wkb::reader::Wkb; + +use geo_traits::Dimensions; +use sedona_geometry::wkb_factory; + +/// ST_Centroid() implementation using centroid extraction +pub fn st_centroid_impl() -> ScalarKernelRef { + Arc::new(STCentroid {}) +} + +#[derive(Debug)] +struct STCentroid {} + +impl SedonaScalarKernel for STCentroid { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + let matcher = ArgMatcher::new(vec![ArgMatcher::is_geometry()], WKB_GEOMETRY); + + matcher.match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = WkbExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity(executor.num_iterations(), 1024); + executor.execute_wkb_void(|maybe_wkb| { + match maybe_wkb { + Some(wkb) => { + let centroid_wkb = invoke_scalar(&wkb)?; + builder.append_value(¢roid_wkb); + } + _ => builder.append_null(), + } + + Ok(()) + })?; + + executor.finish(Arc::new(builder.finish())) + } +} + +fn invoke_scalar(wkb: &Wkb) -> Result<Vec<u8>> { + // Check for empty geometries first - they should return POINT EMPTY + if is_wkb_empty(wkb)? { + let mut empty_point_wkb = Vec::new(); + wkb_factory::write_wkb_empty_point(&mut empty_point_wkb, Dimensions::Xy) + .map_err(|e| datafusion_common::error::DataFusionError::External(Box::new(e)))?; + return Ok(empty_point_wkb); + } + + // Use Centroid trait directly on WKB, similar to how st_area uses unsigned_area() + if let Some(centroid_point) = wkb.centroid() { + // Extract coordinates from the centroid point + let x = centroid_point.x(); + let y = centroid_point.y(); Review Comment: Not a blocker here because GEOS doesn't do this either, but DuckDB's centroid calculation includes support for Z and M coordinates. ########## rust/sedona-geo/src/st_centroid.rs: ########## @@ -0,0 +1,150 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::sync::Arc; + +use arrow_array::builder::BinaryBuilder; +use datafusion_common::error::Result; +use datafusion_expr::ColumnarValue; +use geo_generic_alg::Centroid; +use sedona_expr::scalar_udf::{ArgMatcher, ScalarKernelRef, SedonaScalarKernel}; +use sedona_functions::executor::WkbExecutor; +use sedona_functions::st_isempty::is_wkb_empty; +use sedona_schema::datatypes::{SedonaType, WKB_GEOMETRY}; +use wkb::reader::Wkb; + +use geo_traits::Dimensions; +use sedona_geometry::wkb_factory; + +/// ST_Centroid() implementation using centroid extraction +pub fn st_centroid_impl() -> ScalarKernelRef { + Arc::new(STCentroid {}) +} + +#[derive(Debug)] +struct STCentroid {} + +impl SedonaScalarKernel for STCentroid { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + let matcher = ArgMatcher::new(vec![ArgMatcher::is_geometry()], WKB_GEOMETRY); + + matcher.match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = WkbExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity(executor.num_iterations(), 1024); + executor.execute_wkb_void(|maybe_wkb| { + match maybe_wkb { + Some(wkb) => { + let centroid_wkb = invoke_scalar(&wkb)?; + builder.append_value(¢roid_wkb); + } + _ => builder.append_null(), + } + + Ok(()) + })?; + + executor.finish(Arc::new(builder.finish())) + } +} + +fn invoke_scalar(wkb: &Wkb) -> Result<Vec<u8>> { + // Check for empty geometries first - they should return POINT EMPTY + if is_wkb_empty(wkb)? { + let mut empty_point_wkb = Vec::new(); + wkb_factory::write_wkb_empty_point(&mut empty_point_wkb, Dimensions::Xy) + .map_err(|e| datafusion_common::error::DataFusionError::External(Box::new(e)))?; + return Ok(empty_point_wkb); + } + + // Use Centroid trait directly on WKB, similar to how st_area uses unsigned_area() + if let Some(centroid_point) = wkb.centroid() { + // Extract coordinates from the centroid point + let x = centroid_point.x(); + let y = centroid_point.y(); + + wkb_factory::wkb_point((x, y)) + .map_err(|e| datafusion_common::error::DataFusionError::External(Box::new(e))) + } else { + // This should not happen for non-empty geometries, return POINT EMPTY as fallback + let mut empty_point_wkb = Vec::new(); + wkb_factory::write_wkb_empty_point(&mut empty_point_wkb, Dimensions::Xy) + .map_err(|e| datafusion_common::error::DataFusionError::External(Box::new(e)))?; + Ok(empty_point_wkb) + } Review Comment: If there was an error calculating the centroid I think this should be passed on to the user with an `exec_err!()` instead of ignored ########## rust/sedona-functions/src/st_isempty.rs: ########## @@ -87,6 +87,10 @@ impl SedonaScalarKernel for STIsEmpty { } } +pub fn is_wkb_empty(item: &Wkb) -> Result<bool> { + invoke_scalar(item) +} + Review Comment: If we need this for something other than a scalar function, we can move it (and its tets) to `sedona-geometry/src/is_empty.rs`. This can also operate on `&impl GeometryTrait` rather than `&Wkb`. ########## rust/sedona-geo/src/st_length.rs: ########## @@ -0,0 +1,113 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::sync::Arc; + +use arrow_array::builder::Float64Builder; +use arrow_schema::DataType; +use datafusion_common::error::Result; +use datafusion_expr::ColumnarValue; +use geo_generic_alg::algorithm::{line_measures::Euclidean, LengthMeasurableExt}; +use sedona_expr::scalar_udf::{ArgMatcher, ScalarKernelRef, SedonaScalarKernel}; +use sedona_functions::executor::WkbExecutor; +use sedona_schema::datatypes::SedonaType; +use wkb::reader::Wkb; + +/// ST_Length() implementation using [LengthMeasurableExt] with Euclidean metric +pub fn st_length_impl() -> ScalarKernelRef { + Arc::new(STLength {}) +} + +#[derive(Debug)] +struct STLength {} + +impl SedonaScalarKernel for STLength { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + let matcher = ArgMatcher::new( + vec![ArgMatcher::is_geometry()], + SedonaType::Arrow(DataType::Float64), + ); + + matcher.match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = WkbExecutor::new(arg_types, args); + let mut builder = Float64Builder::with_capacity(executor.num_iterations()); + executor.execute_wkb_void(|maybe_wkb| { + match maybe_wkb { + Some(wkb) => { + builder.append_value(invoke_scalar(&wkb)?); + } + _ => builder.append_null(), + } + + Ok(()) + })?; + + executor.finish(Arc::new(builder.finish())) + } +} + +fn invoke_scalar(wkb: &Wkb) -> Result<f64> { + Ok(wkb.length_ext(&Euclidean)) +} + +#[cfg(test)] +mod tests { + use arrow_array::{create_array, ArrayRef}; + use datafusion_common::scalar::ScalarValue; + use rstest::rstest; + use sedona_functions::register::stubs::st_length_udf; + use sedona_schema::datatypes::{WKB_GEOMETRY, WKB_VIEW_GEOMETRY}; + use sedona_testing::testers::ScalarUdfTester; + + use super::*; + + #[rstest] + fn udf(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType) { + let mut udf = st_length_udf(); + udf.add_kernel(st_length_impl()); + let tester = ScalarUdfTester::new(udf.into(), vec![sedona_type]); + + assert_eq!( + tester.return_type().unwrap(), + SedonaType::Arrow(DataType::Float64) + ); + + // Test with a line string + assert_eq!( + tester + .invoke_wkb_scalar(Some("LINESTRING (0 0, 3 4)")) + .unwrap(), + ScalarValue::Float64(Some(5.0)) + ); + + let input_wkt = vec![ + Some("POINT(1 2)"), // Point should have 0 length + None, + Some("LINESTRING (0 0, 3 4)"), // Should have length 5 + Some("LINESTRING (0 0, 1 0, 1 1)"), // Should have length 2 + ]; Review Comment: Are polygons handled correctly here? ########## rust/sedona-geo/Cargo.toml: ########## @@ -48,6 +48,7 @@ sedona-expr = { path = "../sedona-expr" } sedona-functions = { path = "../sedona-functions" } sedona-geometry = { path = "../sedona-geometry" } sedona-schema = { path = "../sedona-schema" } +sedona-testing = { path = "../sedona-testing" } Review Comment: I think you can remove this! ########## .github/workflows/rust.yml: ########## @@ -99,6 +104,18 @@ jobs: # Update this key to force a new cache prefix-key: "rust-${{ matrix.name }}-v1" + - name: Free Disk Space (Ubuntu) + uses: jlumbroso/free-disk-space@main + with: Review Comment: Can you try bumping the `prefix-key` just above here instead of this change? (If that doesn't work feel free to bring this back!) -- 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]
