From e9c08d902a5ceaed7f4c9a9a065236421964fa2e Mon Sep 17 00:00:00 2001 From: Ajay Padwal Date: Sat, 20 Jun 2026 22:44:04 +0530 Subject: [PATCH 1/7] feat: add ST_NumPoints alias for ST_NPoints Closes part of #200. --- rust/sedona-functions/src/st_points.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rust/sedona-functions/src/st_points.rs b/rust/sedona-functions/src/st_points.rs index 15c98e711..ca928eabf 100644 --- a/rust/sedona-functions/src/st_points.rs +++ b/rust/sedona-functions/src/st_points.rs @@ -115,6 +115,7 @@ pub fn st_npoints_udf() -> SedonaScalarUDF { ItemCrsKernel::wrap_impl(vec![Arc::new(STNPoints)]), Volatility::Immutable, ) + .with_aliases(vec!["st_numpoints".to_string()]) } #[derive(Debug)] @@ -296,6 +297,12 @@ mod tests { assert!(st_npoints_udf.documentation().is_none()); } + #[test] + fn npoints_aliases() { + let udf: ScalarUDF = st_npoints_udf().into(); + assert!(udf.aliases().contains(&"st_numpoints".to_string())); + } + #[rstest] fn udf(#[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType) { use arrow_array::UInt64Array; From bcd306b7c9471a34f8a616ab0b6d4ede2f2a6ce0 Mon Sep 17 00:00:00 2001 From: Ajay Padwal Date: Sat, 20 Jun 2026 23:15:05 +0530 Subject: [PATCH 2/7] docs: mention ST_NumPoints alias in st_npoints.qmd --- docs/reference/sql/st_npoints.qmd | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/reference/sql/st_npoints.qmd b/docs/reference/sql/st_npoints.qmd index 9a7a3ea1d..5572f55ba 100644 --- a/docs/reference/sql/st_npoints.qmd +++ b/docs/reference/sql/st_npoints.qmd @@ -29,6 +29,8 @@ kernels: Returns the total number of coordinate points in the geometry, counting all vertices across all components. +This function also has the alias `ST_NumPoints`. + ## Examples ```sql From 8d8db4cdf2ab35782bf4c94c683aab2d06005e21 Mon Sep 17 00:00:00 2001 From: Ajay Padwal Date: Sun, 21 Jun 2026 21:59:25 +0530 Subject: [PATCH 3/7] docs: use 'Aliases:' pattern in st_npoints.qmd per review feedback --- docs/reference/sql/st_npoints.qmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/sql/st_npoints.qmd b/docs/reference/sql/st_npoints.qmd index 5572f55ba..f7543f94b 100644 --- a/docs/reference/sql/st_npoints.qmd +++ b/docs/reference/sql/st_npoints.qmd @@ -29,7 +29,7 @@ kernels: Returns the total number of coordinate points in the geometry, counting all vertices across all components. -This function also has the alias `ST_NumPoints`. +Aliases: `ST_NumPoints`. ## Examples From c0ca6bb0a79f395b37e2a5b7fc800bb704419f41 Mon Sep 17 00:00:00 2001 From: Ajay Padwal Date: Mon, 22 Jun 2026 11:17:07 +0530 Subject: [PATCH 4/7] feat: remove GEOS ST_NumPoints impl; update tests for alias behavior ST_NumPoints is now an alias for ST_NPoints (registered via with_aliases in rust/sedona-functions). The separate GEOS implementation in c/sedona-geos was LineString-only (returned NULL for other types). Remove it to avoid the duplicate registration conflict. Update test_st_numpoints expected values to reflect alias behavior: ST_NumPoints now counts vertices across all geometry types, matching ST_NPoints and PostGIS documentation. --- c/sedona-geos/src/lib.rs | 1 - c/sedona-geos/src/register.rs | 1 - c/sedona-geos/src/st_numpoints.rs | 146 ------------------ .../tests/functions/test_functions.py | 22 +-- 4 files changed, 11 insertions(+), 159 deletions(-) delete mode 100644 c/sedona-geos/src/st_numpoints.rs diff --git a/c/sedona-geos/src/lib.rs b/c/sedona-geos/src/lib.rs index 9973f4437..29c4d293b 100644 --- a/c/sedona-geos/src/lib.rs +++ b/c/sedona-geos/src/lib.rs @@ -41,7 +41,6 @@ mod st_minimumclearance_line; mod st_normalize; mod st_nrings; mod st_numinteriorrings; -mod st_numpoints; mod st_perimeter; mod st_polygonize; mod st_polygonize_agg; diff --git a/c/sedona-geos/src/register.rs b/c/sedona-geos/src/register.rs index bc48a14eb..390d7420e 100644 --- a/c/sedona-geos/src/register.rs +++ b/c/sedona-geos/src/register.rs @@ -70,7 +70,6 @@ pub fn scalar_kernels() -> Vec<(&'static str, Vec)> { "st_normalize" => crate::st_normalize::st_normalize_impl, "st_nrings" => crate::st_nrings::st_nrings_impl, "st_numinteriorrings" => crate::st_numinteriorrings::st_num_interior_rings_impl, - "st_numpoints" => crate::st_numpoints::st_num_points_impl, "st_overlaps" => crate::binary_predicates::st_overlaps_impl, "st_perimeter" => crate::st_perimeter::st_perimeter_impl, "st_polygonize" => crate::st_polygonize::st_polygonize_impl, diff --git a/c/sedona-geos/src/st_numpoints.rs b/c/sedona-geos/src/st_numpoints.rs deleted file mode 100644 index 4f65435e4..000000000 --- a/c/sedona-geos/src/st_numpoints.rs +++ /dev/null @@ -1,146 +0,0 @@ -// 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 crate::executor::GeosExecutor; -use arrow_array::builder::Int32Builder; -use arrow_schema::DataType; -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_schema::{datatypes::SedonaType, matchers::ArgMatcher}; - -pub fn st_num_points_impl() -> Vec { - ItemCrsKernel::wrap_impl(STNumPoints {}) -} - -#[derive(Debug)] -struct STNumPoints {} - -impl SedonaScalarKernel for STNumPoints { - fn return_type(&self, args: &[SedonaType]) -> Result> { - let matcher = ArgMatcher::new( - vec![ArgMatcher::is_geometry_or_geography()], - SedonaType::Arrow(DataType::Int32), - ); - - matcher.match_args(args) - } - - fn invoke_batch( - &self, - arg_types: &[SedonaType], - args: &[ColumnarValue], - ) -> Result { - let executor = GeosExecutor::new(arg_types, args); - let mut builder = Int32Builder::with_capacity(executor.num_iterations()); - executor.execute_wkb_void(|maybe_geom| { - match maybe_geom { - None => builder.append_null(), - Some(geom) => { - let res = invoke_scalar(&geom)?; - match res { - Some(n) => builder.append_value(n), - None => builder.append_null(), - } - } - } - Ok(()) - })?; - - executor.finish(Arc::new(builder.finish())) - } -} - -fn invoke_scalar(geom: &Geometry) -> Result> { - match geom - .geometry_type() - .map_err(|e| DataFusionError::Execution(format!("Failed to get geometry type: {e}")))? - { - GeometryTypes::LineString => { - let count = geom.get_num_points().map_err(|e| { - DataFusionError::Execution(format!("Failed to get num points: {e}")) - })?; - Ok(Some(count as i32)) - } - _ => Ok(None), - } -} - -#[cfg(test)] -mod tests { - use std::sync::Arc; - - use arrow_array::{ArrayRef, Int32Array}; - use arrow_schema::DataType; - use datafusion_common::ScalarValue; - use rstest::rstest; - use sedona_expr::scalar_udf::SedonaScalarUDF; - use sedona_schema::datatypes::{ - SedonaType, WKB_GEOGRAPHY, WKB_GEOGRAPHY_ITEM_CRS, WKB_GEOMETRY, WKB_GEOMETRY_ITEM_CRS, - WKB_VIEW_GEOGRAPHY, WKB_VIEW_GEOMETRY, - }; - use sedona_testing::compare::assert_array_equal; - use sedona_testing::testers::ScalarUdfTester; - - use super::*; - - #[rstest] - fn udf( - #[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY, WKB_GEOGRAPHY, WKB_VIEW_GEOGRAPHY, WKB_GEOMETRY_ITEM_CRS.clone(), WKB_GEOGRAPHY_ITEM_CRS.clone())] - sedona_type: SedonaType, - ) { - let udf = SedonaScalarUDF::from_impl("st_numpoints", st_num_points_impl()); - let tester = ScalarUdfTester::new(udf.into(), vec![sedona_type]); - tester.assert_return_type(DataType::Int32); - let result = tester.invoke_scalar("LINESTRING (1 2, 3 4)").unwrap(); - tester.assert_scalar_result_equals(result, 2_i32); - - let result = tester.invoke_scalar(ScalarValue::Null).unwrap(); - assert!(result.is_null()); - - let input_wkt = vec![ - None, - Some("POINT (1 2)"), - Some("LINESTRING (0 0, 1 1, 2 2)"), - Some("POLYGON EMPTY"), - Some("LINESTRING(0 0, 1 1, 1 2, 2 2)"), - Some("MULTILINESTRING ((0 0, 1 1), (2 2, 3 3))"), - Some("POLYGON ((0 0, 4 0, 4 4, 0 4, 0 0))"), - Some("GEOMETRYCOLLECTION (POINT (1 2), LINESTRING (0 0, 1 1))"), - ]; - - let expected: ArrayRef = Arc::new(Int32Array::from(vec![ - None, - None, - Some(3), - None, - Some(4), - None, - None, - None, - ])); - - let result = tester.invoke_wkb_array(input_wkt).unwrap(); - assert_array_equal(&result, &expected); - } -} diff --git a/python/sedonadb/tests/functions/test_functions.py b/python/sedonadb/tests/functions/test_functions.py index 95c36c0ae..1caba408a 100644 --- a/python/sedonadb/tests/functions/test_functions.py +++ b/python/sedonadb/tests/functions/test_functions.py @@ -3772,23 +3772,23 @@ def test_st_numinteriorrings_basic(eng, geom, expected): ("geom", "expected"), [ (None, None), - ("POINT EMPTY", None), + ("POINT EMPTY", 0), ("LINESTRING EMPTY", 0), - ("POLYGON EMPTY", None), - ("MULTIPOINT EMPTY", None), - ("MULTILINESTRING EMPTY", None), - ("MULTIPOLYGON EMPTY", None), - ("GEOMETRYCOLLECTION EMPTY", None), - ("POINT (1 2)", None), + ("POLYGON EMPTY", 0), + ("MULTIPOINT EMPTY", 0), + ("MULTILINESTRING EMPTY", 0), + ("MULTIPOLYGON EMPTY", 0), + ("GEOMETRYCOLLECTION EMPTY", 0), + ("POINT (1 2)", 1), ("LINESTRING (0 0, 1 1, 2 2)", 3), ("LINESTRING (0 0, 1 1, 0 0)", 3), ("LINESTRING Z (0 0 0, 1 1 1, 2 2 2, 3 3 3)", 4), ("LINESTRING M (0 0 0, 1 1 1, 2 2 2, 3 3 3)", 4), ("LINESTRING ZM (0 0 0 2, 1 1 1 4)", 2), - ("POLYGON ((0 0, 4 0, 4 4, 0 4, 0 0))", None), - ("MULTILINESTRING ((0 0, 0 1, 1 1, 0 0),(0 0, 1 1))", None), - ("GEOMETRYCOLLECTION (LINESTRING (0 0, 0 1, 1 1, 0 0))", None), - ("POLYGON ((0 0,6 0,6 6,0 6,0 0),(2 2,4 2,4 4,2 4,2 2))", None), + ("POLYGON ((0 0, 4 0, 4 4, 0 4, 0 0))", 5), + ("MULTILINESTRING ((0 0, 0 1, 1 1, 0 0),(0 0, 1 1))", 6), + ("GEOMETRYCOLLECTION (LINESTRING (0 0, 0 1, 1 1, 0 0))", 4), + ("POLYGON ((0 0,6 0,6 6,0 6,0 0),(2 2,4 2,4 4,2 4,2 2))", 10), ], ) def test_st_numpoints(eng, geom, expected): From b5510a273c1fa8cc7e8349c2b755c20555b5d7c4 Mon Sep 17 00:00:00 2001 From: Ajay Padwal Date: Mon, 22 Jun 2026 19:24:47 +0530 Subject: [PATCH 5/7] ci: re-trigger CI (transient crates.io network failure) From c1377f9eda5e38f4094e862c00931baa8c95ffda Mon Sep 17 00:00:00 2001 From: Ajay Padwal Date: Tue, 23 Jun 2026 00:51:10 +0530 Subject: [PATCH 6/7] test: fold ST_NumPoints into ST_NPoints test; fix geography test expectations Per review feedback: remove standalone test_st_numpoints from test_functions.py and add ST_NumPoints as a third assertion in test_st_points, keeping both aliases in sync automatically. Also update test_geog_mechanical_transforms.py ST_NumPoints expected values to reflect alias behavior (all geometry types return counts, not just LineStrings). --- .../tests/functions/test_functions.py | 36 +++---------------- .../test_geog_mechanical_transforms.py | 20 +++++------ 2 files changed, 14 insertions(+), 42 deletions(-) diff --git a/python/sedonadb/tests/functions/test_functions.py b/python/sedonadb/tests/functions/test_functions.py index 1caba408a..09369407e 100644 --- a/python/sedonadb/tests/functions/test_functions.py +++ b/python/sedonadb/tests/functions/test_functions.py @@ -2614,6 +2614,10 @@ def test_st_points(eng, geometry, expected, expected_n): f"SELECT ST_NPoints({geom_or_null(geometry)})", expected_n, ) + eng.assert_query_result( + f"SELECT ST_NumPoints({geom_or_null(geometry)})", + expected_n, + ) @pytest.mark.parametrize("eng", [SedonaDB, PostGIS]) @@ -3767,38 +3771,6 @@ def test_st_numinteriorrings_basic(eng, geom, expected): ) -@pytest.mark.parametrize("eng", [SedonaDB, PostGIS]) -@pytest.mark.parametrize( - ("geom", "expected"), - [ - (None, None), - ("POINT EMPTY", 0), - ("LINESTRING EMPTY", 0), - ("POLYGON EMPTY", 0), - ("MULTIPOINT EMPTY", 0), - ("MULTILINESTRING EMPTY", 0), - ("MULTIPOLYGON EMPTY", 0), - ("GEOMETRYCOLLECTION EMPTY", 0), - ("POINT (1 2)", 1), - ("LINESTRING (0 0, 1 1, 2 2)", 3), - ("LINESTRING (0 0, 1 1, 0 0)", 3), - ("LINESTRING Z (0 0 0, 1 1 1, 2 2 2, 3 3 3)", 4), - ("LINESTRING M (0 0 0, 1 1 1, 2 2 2, 3 3 3)", 4), - ("LINESTRING ZM (0 0 0 2, 1 1 1 4)", 2), - ("POLYGON ((0 0, 4 0, 4 4, 0 4, 0 0))", 5), - ("MULTILINESTRING ((0 0, 0 1, 1 1, 0 0),(0 0, 1 1))", 6), - ("GEOMETRYCOLLECTION (LINESTRING (0 0, 0 1, 1 1, 0 0))", 4), - ("POLYGON ((0 0,6 0,6 6,0 6,0 0),(2 2,4 2,4 4,2 4,2 2))", 10), - ], -) -def test_st_numpoints(eng, geom, expected): - eng = eng.create_or_skip() - eng.assert_query_result( - f"SELECT ST_NumPoints({geom_or_null(geom)})", - expected, - ) - - @pytest.mark.parametrize("eng", [SedonaDB, PostGIS]) @pytest.mark.parametrize( ("geom", "expected"), diff --git a/python/sedonadb/tests/geography/test_geog_mechanical_transforms.py b/python/sedonadb/tests/geography/test_geog_mechanical_transforms.py index ba391a284..54ba4a75e 100644 --- a/python/sedonadb/tests/geography/test_geog_mechanical_transforms.py +++ b/python/sedonadb/tests/geography/test_geog_mechanical_transforms.py @@ -638,28 +638,28 @@ def test_st_numinteriorrings(eng, geog, expected): ("geog", "expected"), [ pytest.param(None, None, id="null"), - pytest.param("POINT EMPTY", None, id="point_empty"), + pytest.param("POINT EMPTY", 0, id="point_empty"), pytest.param("LINESTRING EMPTY", 0, id="linestring_empty"), - pytest.param("POLYGON EMPTY", None, id="polygon_empty"), - pytest.param("MULTIPOINT EMPTY", None, id="multipoint_empty"), - pytest.param("MULTILINESTRING EMPTY", None, id="multilinestring_empty"), - pytest.param("MULTIPOLYGON EMPTY", None, id="multipolygon_empty"), - pytest.param("GEOMETRYCOLLECTION EMPTY", None, id="geometrycollection_empty"), - pytest.param("POINT (1 2)", None, id="point"), + pytest.param("POLYGON EMPTY", 0, id="polygon_empty"), + pytest.param("MULTIPOINT EMPTY", 0, id="multipoint_empty"), + pytest.param("MULTILINESTRING EMPTY", 0, id="multilinestring_empty"), + pytest.param("MULTIPOLYGON EMPTY", 0, id="multipolygon_empty"), + pytest.param("GEOMETRYCOLLECTION EMPTY", 0, id="geometrycollection_empty"), + pytest.param("POINT (1 2)", 1, id="point"), pytest.param("LINESTRING (0 0, 1 1, 2 2)", 3, id="linestring"), pytest.param("LINESTRING (0 0, 1 1, 0 0)", 3, id="linestring_closed"), pytest.param("LINESTRING Z (0 0 0, 1 1 1, 2 2 2, 3 3 3)", 4, id="linestring_z"), pytest.param("LINESTRING M (0 0 0, 1 1 1, 2 2 2, 3 3 3)", 4, id="linestring_m"), pytest.param("LINESTRING ZM (0 0 0 2, 1 1 1 4)", 2, id="linestring_zm"), - pytest.param("POLYGON ((0 0, 4 0, 4 4, 0 4, 0 0))", None, id="polygon"), + pytest.param("POLYGON ((0 0, 4 0, 4 4, 0 4, 0 0))", 5, id="polygon"), pytest.param( "MULTILINESTRING ((0 0, 0 1, 1 1, 0 0),(0 0, 1 1))", - None, + 6, id="multilinestring", ), pytest.param( "GEOMETRYCOLLECTION (LINESTRING (0 0, 0 1, 1 1, 0 0))", - None, + 4, id="geometrycollection", ), ], From 8256d8bf4485a6f97bcdbbfbe0cb5be2d2f62487 Mon Sep 17 00:00:00 2001 From: Ajay Padwal Date: Tue, 23 Jun 2026 01:55:28 +0530 Subject: [PATCH 7/7] test: guard ST_NumPoints assertion to SedonaDB only in test_st_points PostGIS treats ST_NumPoints as LineString-only in practice, returning NULL for other geometry types, even though its documentation lists it as an alias for ST_NPoints. Skip the ST_NumPoints assertion on PostGIS to avoid false failures. --- python/sedonadb/tests/functions/test_functions.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/python/sedonadb/tests/functions/test_functions.py b/python/sedonadb/tests/functions/test_functions.py index 09369407e..b949dafd2 100644 --- a/python/sedonadb/tests/functions/test_functions.py +++ b/python/sedonadb/tests/functions/test_functions.py @@ -2605,6 +2605,7 @@ def test_st_pointm(eng, x, y, m, expected): ], ) def test_st_points(eng, geometry, expected, expected_n): + is_postgis = eng is PostGIS eng = eng.create_or_skip() eng.assert_query_result( f"SELECT ST_Points({geom_or_null(geometry)})", @@ -2614,10 +2615,13 @@ def test_st_points(eng, geometry, expected, expected_n): f"SELECT ST_NPoints({geom_or_null(geometry)})", expected_n, ) - eng.assert_query_result( - f"SELECT ST_NumPoints({geom_or_null(geometry)})", - expected_n, - ) + if not is_postgis: + # ST_NumPoints is an alias for ST_NPoints in SedonaDB. + # PostGIS still treats ST_NumPoints as LineString-only despite documentation. + eng.assert_query_result( + f"SELECT ST_NumPoints({geom_or_null(geometry)})", + expected_n, + ) @pytest.mark.parametrize("eng", [SedonaDB, PostGIS])