paleolimbot commented on code in PR #990: URL: https://github.com/apache/sedona-db/pull/990#discussion_r3464538254
########## c/sedona-geos/src/st_delaunaytriangles.rs: ########## @@ -0,0 +1,234 @@ +// 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 arrow_schema::DataType; +use datafusion_common::{cast::as_boolean_array, cast::as_float64_array, DataFusionError, Result}; +use datafusion_expr::ColumnarValue; +use geos::{Geom, Geometry}; +use sedona_expr::{ + item_crs::ItemCrsKernel, + scalar_udf::{ScalarKernelRef, SedonaScalarKernel}, +}; +use sedona_geometry::wkb_factory::WKB_MIN_PROBABLE_BYTES; +use sedona_schema::{ + datatypes::{SedonaType, WKB_GEOMETRY}, + matchers::ArgMatcher, +}; + +use crate::executor::GeosExecutor; +use crate::geos_to_wkb::write_geos_geometry; + +fn invoke_scalar( + geom: &Geometry, + tolerance: f64, + only_edges: bool, + writer: &mut impl std::io::Write, +) -> Result<()> { + let result = geom + .delaunay_triangulation(tolerance, only_edges) + .map_err(|e| DataFusionError::Execution(format!("ST_DelaunayTriangles failed: {e}")))?; + write_geos_geometry(&result, writer)?; + Ok(()) +} + +// ── 1-arg: ST_DelaunayTriangles(geom) ──────────────────────────────────────── + +pub fn st_delaunay_triangles_impl() -> Vec<ScalarKernelRef> { + ItemCrsKernel::wrap_impl(STDelaunayTriangles) +} + +#[derive(Debug)] +struct STDelaunayTriangles; + +impl SedonaScalarKernel for STDelaunayTriangles { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + ArgMatcher::new(vec![ArgMatcher::is_geometry()], WKB_GEOMETRY).match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = GeosExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity( + executor.num_iterations(), + WKB_MIN_PROBABLE_BYTES * executor.num_iterations(), + ); + executor.execute_wkb_void(|maybe_geom| { + match maybe_geom { + Some(geom) => { + invoke_scalar(&geom, 0.0, false, &mut builder)?; + builder.append_value([]); + } + None => builder.append_null(), + } + Ok(()) + })?; + executor.finish(Arc::new(builder.finish())) + } +} + +// ── 2-arg: ST_DelaunayTriangles(geom, tolerance) ───────────────────────────── + +pub fn st_delaunay_triangles_tolerance_impl() -> Vec<ScalarKernelRef> { + ItemCrsKernel::wrap_impl(STDelaunayTrianglesWithTolerance) +} + +#[derive(Debug)] +struct STDelaunayTrianglesWithTolerance; + +impl SedonaScalarKernel for STDelaunayTrianglesWithTolerance { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + ArgMatcher::new( + vec![ArgMatcher::is_geometry(), ArgMatcher::is_numeric()], + WKB_GEOMETRY, + ) + .match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = GeosExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity( + executor.num_iterations(), + WKB_MIN_PROBABLE_BYTES * executor.num_iterations(), + ); + let tol_value = args[1] + .cast_to(&DataType::Float64, None)? + .to_array(executor.num_iterations())?; + let tol_array = as_float64_array(&tol_value)?; + let mut tol_iter = tol_array.iter(); + executor.execute_wkb_void(|maybe_geom| { + match (maybe_geom, tol_iter.next().unwrap()) { + (Some(geom), Some(tol)) => { + invoke_scalar(&geom, tol, false, &mut builder)?; + builder.append_value([]); + } + _ => builder.append_null(), + } + Ok(()) + })?; + executor.finish(Arc::new(builder.finish())) + } +} + +// ── 3-arg: ST_DelaunayTriangles(geom, tolerance, flags) ────────────────────── +// flags=0 → polygon output (default), flags=1 → multilinestring edges only + +pub fn st_delaunay_triangles_flags_impl() -> Vec<ScalarKernelRef> { + ItemCrsKernel::wrap_impl(STDelaunayTrianglesWithFlags) +} + +#[derive(Debug)] +struct STDelaunayTrianglesWithFlags; + +impl SedonaScalarKernel for STDelaunayTrianglesWithFlags { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + ArgMatcher::new( + vec![ + ArgMatcher::is_geometry(), + ArgMatcher::is_numeric(), + ArgMatcher::is_boolean(), Review Comment: This works because 0 or 1 are the two values we accept, but probably this is better as `is_integer`. ########## c/sedona-geos/src/st_exteriorring.rs: ########## @@ -0,0 +1,153 @@ +// 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, DataFusionError}; +use datafusion_expr::ColumnarValue; +use geos::{Geom, Geometry, GeometryTypes}; +use sedona_expr::{ + item_crs::ItemCrsKernel, + scalar_udf::{ScalarKernelRef, SedonaScalarKernel}, +}; +use sedona_geometry::wkb_factory::WKB_MIN_PROBABLE_BYTES; +use sedona_schema::{ + datatypes::{SedonaType, WKB_GEOGRAPHY, WKB_GEOMETRY}, + matchers::ArgMatcher, +}; + +use crate::executor::GeosExecutor; + +/// ST_ExteriorRing() implementation using the geos crate +/// +/// Returns the exterior ring of a Polygon, or NULL for non-polygon geometries. +pub fn st_exterior_ring_impl() -> Vec<ScalarKernelRef> { + ItemCrsKernel::wrap_impl(vec![ + Arc::new(STExteriorRing { + matcher: ArgMatcher::new(vec![ArgMatcher::is_geometry()], WKB_GEOMETRY), + }), + Arc::new(STExteriorRing { + matcher: ArgMatcher::new(vec![ArgMatcher::is_geography()], WKB_GEOGRAPHY), + }), + ]) +} + +#[derive(Debug)] +struct STExteriorRing { + matcher: ArgMatcher, +} + +impl SedonaScalarKernel for STExteriorRing { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + self.matcher.match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = GeosExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity( + executor.num_iterations(), + WKB_MIN_PROBABLE_BYTES * executor.num_iterations(), + ); + executor.execute_wkb_void(|maybe_geom| { + match maybe_geom { + Some(geom) => match invoke_scalar(&geom)? { + Some(wkb) => { + builder.append_value(&wkb); + } + None => builder.append_null(), + }, + _ => builder.append_null(), + } + Ok(()) + })?; + + executor.finish(Arc::new(builder.finish())) + } +} + +fn invoke_scalar(geom: &Geometry) -> Result<Option<Vec<u8>>> { + let geom_type = geom + .geometry_type() + .map_err(|e| DataFusionError::Execution(format!("Failed to get geometry type: {e}")))?; + + if geom_type != GeometryTypes::Polygon { + return Ok(None); + } + + let ring = geom + .get_exterior_ring() + .map_err(|e| DataFusionError::Execution(format!("ST_ExteriorRing failed: {e}")))?; + + let wkb = ring + .to_wkb() + .map_err(|e| DataFusionError::Execution(format!("Failed to write WKB: {e}")))?; + + Ok(Some(wkb)) +} + +#[cfg(test)] +mod tests { + use datafusion_common::ScalarValue; + use rstest::rstest; + use sedona_expr::scalar_udf::SedonaScalarUDF; + use sedona_schema::datatypes::{ + WKB_GEOGRAPHY, WKB_GEOGRAPHY_ITEM_CRS, WKB_GEOMETRY, WKB_GEOMETRY_ITEM_CRS, + }; + use sedona_testing::testers::ScalarUdfTester; + + use super::*; + + #[rstest] + fn udf(#[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType) { + let udf = SedonaScalarUDF::from_impl("st_exteriorring", st_exterior_ring_impl()); + let tester = ScalarUdfTester::new(udf.into(), vec![sedona_type.clone()]); + + tester.assert_return_type(sedona_type.clone()); + + let result = tester + .invoke_scalar("POLYGON ((0 0, 4 0, 4 4, 0 4, 0 0))") + .unwrap(); + tester.assert_scalar_result_equals(result, "LINEARRING (0 0, 4 0, 4 4, 0 4, 0 0)"); + + // non-polygon returns null + let result = tester.invoke_scalar("POINT (1 2)").unwrap(); + assert!(result.is_null()); + + let result = tester.invoke_scalar(ScalarValue::Null).unwrap(); + assert!(result.is_null()); + } + + #[rstest] + fn udf_invoke_item_crs( + #[values(WKB_GEOMETRY_ITEM_CRS.clone(), WKB_GEOGRAPHY_ITEM_CRS.clone())] + sedona_type: SedonaType, + ) { + let udf = SedonaScalarUDF::from_impl("st_exteriorring", st_exterior_ring_impl()); + let tester = ScalarUdfTester::new(udf.into(), vec![sedona_type.clone()]); + tester.assert_return_type(sedona_type); + + let result = tester + .invoke_scalar("POLYGON ((0 0, 4 0, 4 4, 0 4, 0 0))") + .unwrap(); + tester.assert_scalar_result_equals(result, "LINEARRING (0 0, 4 0, 4 4, 0 4, 0 0)"); Review Comment: This probably works if the wkt crate accepts LINEARRING, but LINESTRING is probably better: ```suggestion tester.assert_scalar_result_equals(result, "LINESTRING (0 0, 4 0, 4 4, 0 4, 0 0)"); ``` ########## c/sedona-geos/src/st_delaunaytriangles.rs: ########## @@ -0,0 +1,234 @@ +// 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 arrow_schema::DataType; +use datafusion_common::{cast::as_boolean_array, cast::as_float64_array, DataFusionError, Result}; +use datafusion_expr::ColumnarValue; +use geos::{Geom, Geometry}; +use sedona_expr::{ + item_crs::ItemCrsKernel, + scalar_udf::{ScalarKernelRef, SedonaScalarKernel}, +}; +use sedona_geometry::wkb_factory::WKB_MIN_PROBABLE_BYTES; +use sedona_schema::{ + datatypes::{SedonaType, WKB_GEOMETRY}, + matchers::ArgMatcher, +}; + +use crate::executor::GeosExecutor; +use crate::geos_to_wkb::write_geos_geometry; + +fn invoke_scalar( + geom: &Geometry, + tolerance: f64, + only_edges: bool, + writer: &mut impl std::io::Write, +) -> Result<()> { + let result = geom + .delaunay_triangulation(tolerance, only_edges) + .map_err(|e| DataFusionError::Execution(format!("ST_DelaunayTriangles failed: {e}")))?; + write_geos_geometry(&result, writer)?; + Ok(()) +} + +// ── 1-arg: ST_DelaunayTriangles(geom) ──────────────────────────────────────── + +pub fn st_delaunay_triangles_impl() -> Vec<ScalarKernelRef> { + ItemCrsKernel::wrap_impl(STDelaunayTriangles) +} + +#[derive(Debug)] +struct STDelaunayTriangles; + +impl SedonaScalarKernel for STDelaunayTriangles { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + ArgMatcher::new(vec![ArgMatcher::is_geometry()], WKB_GEOMETRY).match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = GeosExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity( + executor.num_iterations(), + WKB_MIN_PROBABLE_BYTES * executor.num_iterations(), + ); + executor.execute_wkb_void(|maybe_geom| { + match maybe_geom { + Some(geom) => { + invoke_scalar(&geom, 0.0, false, &mut builder)?; + builder.append_value([]); + } + None => builder.append_null(), + } + Ok(()) + })?; + executor.finish(Arc::new(builder.finish())) + } +} + +// ── 2-arg: ST_DelaunayTriangles(geom, tolerance) ───────────────────────────── + +pub fn st_delaunay_triangles_tolerance_impl() -> Vec<ScalarKernelRef> { + ItemCrsKernel::wrap_impl(STDelaunayTrianglesWithTolerance) +} + +#[derive(Debug)] +struct STDelaunayTrianglesWithTolerance; + +impl SedonaScalarKernel for STDelaunayTrianglesWithTolerance { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + ArgMatcher::new( + vec![ArgMatcher::is_geometry(), ArgMatcher::is_numeric()], + WKB_GEOMETRY, + ) + .match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = GeosExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity( + executor.num_iterations(), + WKB_MIN_PROBABLE_BYTES * executor.num_iterations(), + ); + let tol_value = args[1] + .cast_to(&DataType::Float64, None)? + .to_array(executor.num_iterations())?; + let tol_array = as_float64_array(&tol_value)?; + let mut tol_iter = tol_array.iter(); + executor.execute_wkb_void(|maybe_geom| { + match (maybe_geom, tol_iter.next().unwrap()) { + (Some(geom), Some(tol)) => { + invoke_scalar(&geom, tol, false, &mut builder)?; + builder.append_value([]); + } + _ => builder.append_null(), + } + Ok(()) + })?; + executor.finish(Arc::new(builder.finish())) + } +} + +// ── 3-arg: ST_DelaunayTriangles(geom, tolerance, flags) ────────────────────── +// flags=0 → polygon output (default), flags=1 → multilinestring edges only + +pub fn st_delaunay_triangles_flags_impl() -> Vec<ScalarKernelRef> { + ItemCrsKernel::wrap_impl(STDelaunayTrianglesWithFlags) +} + +#[derive(Debug)] +struct STDelaunayTrianglesWithFlags; + +impl SedonaScalarKernel for STDelaunayTrianglesWithFlags { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + ArgMatcher::new( + vec![ + ArgMatcher::is_geometry(), + ArgMatcher::is_numeric(), + ArgMatcher::is_boolean(), + ], + WKB_GEOMETRY, + ) + .match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = GeosExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity( + executor.num_iterations(), + WKB_MIN_PROBABLE_BYTES * executor.num_iterations(), + ); + let tol_value = args[1] + .cast_to(&DataType::Float64, None)? + .to_array(executor.num_iterations())?; + let tol_array = as_float64_array(&tol_value)?; + let mut tol_iter = tol_array.iter(); + + let flags_value = args[2] + .cast_to(&DataType::Boolean, None)? + .to_array(executor.num_iterations())?; + let flags_array = as_boolean_array(&flags_value)?; + let mut flags_iter = flags_array.iter(); + + executor.execute_wkb_void(|maybe_geom| { + match ( + maybe_geom, + tol_iter.next().unwrap(), + flags_iter.next().unwrap(), + ) { + (Some(geom), Some(tol), Some(only_edges)) => { + invoke_scalar(&geom, tol, only_edges, &mut builder)?; + builder.append_value([]); + } + _ => builder.append_null(), + } + Ok(()) + })?; + executor.finish(Arc::new(builder.finish())) + } +} + +#[cfg(test)] +mod tests { + use datafusion_common::ScalarValue; + use rstest::rstest; + use sedona_expr::scalar_udf::SedonaScalarUDF; + use sedona_schema::datatypes::{WKB_GEOMETRY, WKB_GEOMETRY_ITEM_CRS}; + use sedona_testing::testers::ScalarUdfTester; + + use super::*; + + #[rstest] + fn udf_no_tolerance(#[values(WKB_GEOMETRY)] sedona_type: SedonaType) { + let udf = SedonaScalarUDF::from_impl("st_delaunaytriangles", st_delaunay_triangles_impl()); + let tester = ScalarUdfTester::new(udf.into(), vec![sedona_type.clone()]); + Review Comment: Now that you have the flags version, you should check to make sure it works (because the Rust tests' scope is to cover the branches of the Rust implementation) ########## docs/reference/sql/st_exteriorring.qmd: ########## @@ -0,0 +1,41 @@ +--- +# 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. + +title: ST_ExteriorRing +description: Returns the exterior ring of a polygon as a linear ring geometry. +kernels: + - returns: geometry + args: [geometry] + - returns: geography + args: [geography] +--- + +## Description + +Returns the exterior ring (outer boundary) of a polygon as a `LINEARRING`. Returns NULL for Review Comment: ```suggestion Returns the exterior ring (outer boundary) of a polygon as a `LINESTRING`. Returns NULL for ``` ########## c/sedona-geos/src/st_exteriorring.rs: ########## @@ -0,0 +1,153 @@ +// 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, DataFusionError}; +use datafusion_expr::ColumnarValue; +use geos::{Geom, Geometry, GeometryTypes}; +use sedona_expr::{ + item_crs::ItemCrsKernel, + scalar_udf::{ScalarKernelRef, SedonaScalarKernel}, +}; +use sedona_geometry::wkb_factory::WKB_MIN_PROBABLE_BYTES; +use sedona_schema::{ + datatypes::{SedonaType, WKB_GEOGRAPHY, WKB_GEOMETRY}, + matchers::ArgMatcher, +}; + +use crate::executor::GeosExecutor; + +/// ST_ExteriorRing() implementation using the geos crate +/// +/// Returns the exterior ring of a Polygon, or NULL for non-polygon geometries. +pub fn st_exterior_ring_impl() -> Vec<ScalarKernelRef> { + ItemCrsKernel::wrap_impl(vec![ + Arc::new(STExteriorRing { + matcher: ArgMatcher::new(vec![ArgMatcher::is_geometry()], WKB_GEOMETRY), + }), + Arc::new(STExteriorRing { + matcher: ArgMatcher::new(vec![ArgMatcher::is_geography()], WKB_GEOGRAPHY), + }), + ]) +} + +#[derive(Debug)] +struct STExteriorRing { + matcher: ArgMatcher, +} + +impl SedonaScalarKernel for STExteriorRing { + fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { + self.matcher.match_args(args) + } + + fn invoke_batch( + &self, + arg_types: &[SedonaType], + args: &[ColumnarValue], + ) -> Result<ColumnarValue> { + let executor = GeosExecutor::new(arg_types, args); + let mut builder = BinaryBuilder::with_capacity( + executor.num_iterations(), + WKB_MIN_PROBABLE_BYTES * executor.num_iterations(), + ); + executor.execute_wkb_void(|maybe_geom| { + match maybe_geom { + Some(geom) => match invoke_scalar(&geom)? { + Some(wkb) => { + builder.append_value(&wkb); + } + None => builder.append_null(), + }, + _ => builder.append_null(), + } + Ok(()) + })?; + + executor.finish(Arc::new(builder.finish())) + } +} + +fn invoke_scalar(geom: &Geometry) -> Result<Option<Vec<u8>>> { + let geom_type = geom + .geometry_type() + .map_err(|e| DataFusionError::Execution(format!("Failed to get geometry type: {e}")))?; Review Comment: Our other GEOS functions have an `invoke_scalar()` that accept a `mut impl Write` so that the GEOS output can be written to the output builder in place (rather than allocating a wkb byte buffer, then copying it into a new wkb byte buffer). I believe all the other functions that return a geometry do this but we may have missed some. ########## c/sedona-geos/src/st_pointonsurface.rs: ########## @@ -0,0 +1,137 @@ +// 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, DataFusionError}; +use datafusion_expr::ColumnarValue; +use geos::{Geom, Geometry}; +use sedona_expr::{ + item_crs::ItemCrsKernel, + scalar_udf::{ScalarKernelRef, SedonaScalarKernel}, +}; +use sedona_geometry::wkb_factory::WKB_MIN_PROBABLE_BYTES; +use sedona_schema::{ + datatypes::{SedonaType, WKB_GEOGRAPHY, WKB_GEOMETRY}, + matchers::ArgMatcher, +}; + +use crate::executor::GeosExecutor; +use crate::geos_to_wkb::write_geos_geometry; + +/// ST_PointOnSurface() implementation using the geos crate +pub fn st_point_on_surface_impl() -> Vec<ScalarKernelRef> { + ItemCrsKernel::wrap_impl(vec![ + Arc::new(STPointOnSurface { + matcher: ArgMatcher::new(vec![ArgMatcher::is_geometry()], WKB_GEOMETRY), + }), + Arc::new(STPointOnSurface { + matcher: ArgMatcher::new(vec![ArgMatcher::is_geography()], WKB_GEOGRAPHY), + }), Review Comment: ```suggestion ``` This one shouldn't apply to geography, since the algorithm is very much a planar one and may not be correct. We also implement this for geography already. ########## docs/reference/sql/st_exteriorring.qmd: ########## @@ -0,0 +1,41 @@ +--- +# 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. + +title: ST_ExteriorRing +description: Returns the exterior ring of a polygon as a linear ring geometry. Review Comment: ```suggestion description: Returns the exterior ring of a polygon as a linestring geometry. ``` ########## docs/reference/sql/st_delaunaytriangles.qmd: ########## @@ -0,0 +1,46 @@ +--- +# 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. + +title: ST_DelaunayTriangles +description: Returns a geometry collection of Delaunay triangles covering the vertices of the input geometry. +kernels: + - returns: geometry + args: [geometry] + - returns: geometry + args: + - geometry + - name: tolerance + type: float64 + description: Snap-rounding tolerance for vertices. Use 0.0 for exact computation. +--- + +## Description + +Computes a Delaunay triangulation of the vertices of the input geometry. The result is a +`GEOMETRYCOLLECTION` of `POLYGON` triangles. An optional tolerance value can be specified to +snap nearby vertices together before triangulation. Review Comment: ```suggestion Computes a Delaunay triangulation of the vertices of the input geometry. The result is a `GEOMETRYCOLLECTION` of `POLYGON` triangles unless the `flags` parameter is set to request `MULTILINESTRING` output. An optional tolerance value can be specified to snap nearby vertices together before triangulation. ``` ########## docs/reference/sql/st_delaunaytriangles.qmd: ########## @@ -0,0 +1,46 @@ +--- +# 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. + +title: ST_DelaunayTriangles +description: Returns a geometry collection of Delaunay triangles covering the vertices of the input geometry. +kernels: + - returns: geometry + args: [geometry] + - returns: geometry + args: + - geometry + - name: tolerance + type: float64 + description: Snap-rounding tolerance for vertices. Use 0.0 for exact computation. Review Comment: Can you add the flags kernel here too now that it's implemented? ########## python/sedonadb/tests/functions/test_functions.py: ########## @@ -750,6 +750,51 @@ def test_st_buffer_style_parameters( ) [email protected]("eng", [SedonaDB, PostGIS]) [email protected]( + ("geom", "expected"), + [ + (None, None), + ("LINESTRING (0 0, 1 0, 1 1, 0 0)", "POLYGON ((0 0, 1 1, 1 0, 0 0))"), + ( + "MULTILINESTRING ((0 0, 1 0, 1 1, 0 0), (2 2, 3 2, 3 3, 2 2))", + "MULTIPOLYGON (((1 1, 1 0, 0 0, 1 1)), ((3 3, 3 2, 2 2, 3 3)))", + ), + ], +) +def test_st_buildarea(eng, geom, expected): + eng = eng.create_or_skip() + eng.assert_query_result(f"SELECT ST_BuildArea({geom_or_null(geom)})", expected) + + [email protected]("eng", [SedonaDB, PostGIS]) [email protected]( + ("geom", "sedona_expected", "postgis_expected"), + [ + # Both engines return an empty geometry for empty linework, not NULL. + # SedonaDB returns GEOMETRYCOLLECTION EMPTY; PostGIS returns POLYGON EMPTY. + ("LINESTRING EMPTY", "GEOMETRYCOLLECTION EMPTY", "POLYGON EMPTY"), + ("MULTILINESTRING EMPTY", "GEOMETRYCOLLECTION EMPTY", "POLYGON EMPTY"), + ], +) +def test_st_buildarea_empty_linework(eng, geom, sedona_expected, postgis_expected): + is_postgis = eng is PostGIS + eng = eng.create_or_skip() + expected = postgis_expected if is_postgis else sedona_expected + eng.assert_query_result(f"SELECT ST_BuildArea({geom_or_null(geom)})", expected) + + [email protected]("eng", [SedonaDB, PostGIS]) +def test_st_buildarea_non_linework(eng): + """POINT input forms no closed ring: PostGIS returns NULL, SedonaDB returns GEOMETRYCOLLECTION EMPTY.""" Review Comment: We should catch this case and return NULL as well ########## docs/reference/sql/st_delaunaytriangles.qmd: ########## @@ -0,0 +1,46 @@ +--- +# 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. + +title: ST_DelaunayTriangles +description: Returns a geometry collection of Delaunay triangles covering the vertices of the input geometry. +kernels: + - returns: geometry + args: [geometry] + - returns: geometry + args: + - geometry + - name: tolerance + type: float64 + description: Snap-rounding tolerance for vertices. Use 0.0 for exact computation. +--- + +## Description + +Computes a Delaunay triangulation of the vertices of the input geometry. The result is a +`GEOMETRYCOLLECTION` of `POLYGON` triangles. An optional tolerance value can be specified to +snap nearby vertices together before triangulation. + +## Examples + +```sql +SELECT ST_DelaunayTriangles(ST_GeomFromWKT('MULTIPOINT ((0 0), (1 0), (0 1))')); +``` + +```sql +SELECT ST_DelaunayTriangles(ST_GeomFromWKT('MULTIPOINT ((0 0), (1 0), (0 1))'), 0.0); +``` Review Comment: A flags example would be good, too -- 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]
