Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions c/sedona-geos/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ mod st_buffer;
mod st_centroid;
mod st_convexhull;
mod st_dwithin;
mod st_isring;
mod st_isvalid;
mod st_length;
mod st_perimeter;
Expand Down
10 changes: 6 additions & 4 deletions c/sedona-geos/src/register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@
// under the License.
use sedona_expr::scalar_udf::ScalarKernelRef;

use crate::st_convexhull::st_convex_hull_impl;
use crate::st_isvalid::st_is_valid_impl;
use crate::{
distance::st_distance_impl, st_area::st_area_impl, st_buffer::st_buffer_impl,
st_centroid::st_centroid_impl, st_dwithin::st_dwithin_impl, st_length::st_length_impl,
st_centroid::st_centroid_impl, st_convexhull::st_convex_hull_impl, st_dwithin::st_dwithin_impl,
st_isring::st_is_ring_impl, st_isvalid::st_is_valid_impl, st_length::st_length_impl,
st_perimeter::st_perimeter_impl,
};

Expand All @@ -42,14 +41,17 @@ pub fn scalar_kernels() -> Vec<(&'static str, ScalarKernelRef)> {
("st_convexhull", st_convex_hull_impl()),
("st_coveredby", st_covered_by_impl()),
("st_covers", st_covers_impl()),
("st_crosses", st_crosses_impl()),
("st_difference", st_difference_impl()),
("st_disjoint", st_disjoint_impl()),
("st_distance", st_distance_impl()),
("st_dwithin", st_dwithin_impl()),
("st_equals", st_equals_impl()),
("st_length", st_length_impl()),
("st_intersection", st_intersection_impl()),
("st_intersects", st_intersects_impl()),
("st_isring", st_is_ring_impl()),
("st_length", st_length_impl()),
("st_overlaps", st_overlaps_impl()),
("st_perimeter", st_perimeter_impl()),
("st_symdifference", st_sym_difference_impl()),
("st_touches", st_touches_impl()),
Expand Down
179 changes: 179 additions & 0 deletions c/sedona-geos/src/st_isring.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
// 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::BooleanBuilder;
use arrow_schema::DataType;
use datafusion_common::{error::Result, DataFusionError};
use datafusion_expr::ColumnarValue;
use geos::{Geom, GeometryTypes};
use sedona_expr::scalar_udf::{ScalarKernelRef, SedonaScalarKernel};
use sedona_schema::{datatypes::SedonaType, matchers::ArgMatcher};

use crate::executor::GeosExecutor;

/// ST_IsRing() implementation using the geos crate
pub fn st_is_ring_impl() -> ScalarKernelRef {
Arc::new(STIsRing {})
}

#[derive(Debug)]
struct STIsRing {}

impl SedonaScalarKernel for STIsRing {
fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> {
let matcher = ArgMatcher::new(
vec![ArgMatcher::is_geometry()],
SedonaType::Arrow(DataType::Boolean),
);

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 = BooleanBuilder::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(geos_geom: &geos::Geometry) -> Result<bool> {
// Check if geometry is empty - (PostGIS compatibility)
let is_empty = geos_geom.is_empty().map_err(|e| {
DataFusionError::Execution(format!("Failed to check if geometry is a ring: {e}"))
})?;

if is_empty {
return Ok(false);
}

// Check if geometry is a LineString - (PostGIS compatibility)
if geos_geom.geometry_type() != GeometryTypes::LineString {
return Err(DataFusionError::Execution(
"ST_IsRing() should only be called on a linear feature".to_string(),
));
}

geos_geom.is_ring().map_err(|e| {
DataFusionError::Execution(format!("Failed to check if geometry is a ring: {e}"))
})
}

#[cfg(test)]
mod tests {
use arrow_array::{create_array as arrow_array, ArrayRef};
use rstest::rstest;
use sedona_expr::scalar_udf::SedonaScalarUDF;
use sedona_schema::datatypes::{WKB_GEOMETRY, 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)] sedona_type: SedonaType) {
let udf = SedonaScalarUDF::from_kernel("st_isring", st_is_ring_impl());
let tester = ScalarUdfTester::new(udf.into(), vec![sedona_type]);
tester.assert_return_type(DataType::Boolean);

// Valid ring (closed + simple) - square
let result = tester
.invoke_scalar("LINESTRING(0 0, 0 1, 1 1, 1 0, 0 0)")
.unwrap();
tester.assert_scalar_result_equals(result, true);

// Valid ring (closed + simple) - triangle
let result = tester
.invoke_scalar("LINESTRING(0 0, 1 0, 1 1, 0 0)")
.unwrap();
tester.assert_scalar_result_equals(result, true);

// Non-LineString types should throw errors (PostGIS compatibility)

// Point (not a linestring) - should error
let result = tester.invoke_scalar("POINT(21 52)");
assert!(result.is_err());
assert!(result
.unwrap_err()
.to_string()
.contains("should only be called on a linear feature"));

// Polygon (not a linestring) - should error
let result = tester.invoke_scalar("POLYGON((0 0, 0 5, 5 5, 5 0, 0 0))");
assert!(result.is_err());
assert!(result
.unwrap_err()
.to_string()
.contains("should only be called on a linear feature"));

// MultiLineString (collection) - should error
let result = tester.invoke_scalar("MULTILINESTRING((0 0, 0 1, 1 1, 1 0, 0 0))");
assert!(result.is_err());
assert!(result
.unwrap_err()
.to_string()
.contains("should only be called on a linear feature"));

// GeometryCollection - should error
let result =
tester.invoke_scalar("GEOMETRYCOLLECTION(LINESTRING(0 0, 0 1, 1 1, 1 0, 0 0))");
assert!(result.is_err());
assert!(result
.unwrap_err()
.to_string()
.contains("should only be called on a linear feature"));

let input_wkt = vec![
Some("LINESTRING(0 0, 0 1, 1 1, 1 0, 0 0)"), // Valid ring => true
Some("LINESTRING(0 0, 0 1, 1 0, 1 1, 0 0)"), // Self-intersecting => false
Some("LINESTRING(0 0, 2 2)"), // Not closed => false
Some("LINESTRING EMPTY"), // Empty => false
Some("POINT EMPTY"), // Empty => false
None, // NULL => null
];

let expected: ArrayRef = arrow_array!(
Boolean,
[
Some(true),
Some(false),
Some(false),
Some(false),
Some(false),
None
]
);

assert_array_equal(&tester.invoke_wkb_array(input_wkt).unwrap(), &expected);
}
}
57 changes: 56 additions & 1 deletion python/sedonadb/tests/functions/test_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# under the License.
import pytest
import shapely
from sedonadb.testing import geom_or_null, PostGIS, SedonaDB, val_or_null
from sedonadb.testing import PostGIS, SedonaDB, geom_or_null, val_or_null


@pytest.mark.parametrize("eng", [SedonaDB, PostGIS])
Expand Down Expand Up @@ -661,6 +661,61 @@ def test_st_isclosed(eng, geom, expected):
eng.assert_query_result(f"SELECT ST_IsClosed({geom_or_null(geom)})", expected)


@pytest.mark.parametrize("eng", [SedonaDB, PostGIS])
@pytest.mark.parametrize(
("geom", "expected"),
[
(None, None),
# Valid rings (closed + simple)
("LINESTRING(0 0, 0 1, 1 1, 1 0, 0 0)", True),
("LINESTRING(0 0, 1 0, 1 1, 0 0)", True),
("LINESTRING(0 0, 2 2, 1 2, 0 0)", True),
# Closed but self-intersecting - bowtie shape (not simple)
("LINESTRING(0 0, 0 1, 1 0, 1 1, 0 0)", False),
# Not closed
("LINESTRING(0 0, 1 1)", False),
("LINESTRING(2 0, 2 2, 3 3)", False),
("LINESTRING(0 0, 2 2)", False),
# Empty geometries
("LINESTRING EMPTY", False),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
("LINESTRING EMPTY", False),
("LINESTRING EMPTY", False),
# Collections of linestrings that would have been considered rings
("MULTILINESTRING((0 0, 0 1, 1 1, 1 0, 0 0))", False),
("GEOMETRYCOLLECTION(LINESTRING(0 0, 0 1, 1 1, 1 0, 0 0))", False),

Let's add these two cases. Returning False is the natural behavior for the geos's is_ring() method, which agrees with DuckDB

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Added these under test_st_isring_non_linestring as PostGis is throwing error on these.

("POINT EMPTY", False),
("POLYGON EMPTY", False),
("MULTIPOLYGON EMPTY", False),
("GEOMETRYCOLLECTION EMPTY", False),
],
)
def test_st_isring(eng, geom, expected):
"""Test ST_IsRing with LineString geometries.

ST_IsRing returns true if the geometry is a closed and simple LineString.
"""
eng = eng.create_or_skip()
eng.assert_query_result(f"SELECT ST_IsRing({geom_or_null(geom)})", expected)


@pytest.mark.parametrize("eng", [SedonaDB, PostGIS])
@pytest.mark.parametrize(
("geom"),
[
"POINT(0 0)",
"MULTIPOINT((0 0), (1 1))",
"POLYGON((0 0, 1 0, 1 1, 0 1, 0 0))",
"MULTILINESTRING((0 0, 0 1, 1 1, 1 0, 0 0))",
"GEOMETRYCOLLECTION(LINESTRING(0 0, 0 1, 1 1, 1 0, 0 0))",
],
)
def test_st_isring_non_linestring_error(eng, geom):
"""Test that ST_IsRing throws errors for non-LineString non-empty geometries.

Both SedonaDB and PostGIS throw errors when ST_IsRing is called on
non-LineString geometry types (PostGIS compatibility).
"""
eng = eng.create_or_skip()

with pytest.raises(Exception, match="linear|linestring"):
eng.assert_query_result(f"SELECT ST_IsRing(ST_GeomFromText('{geom}'))", None)


@pytest.mark.parametrize("eng", [SedonaDB, PostGIS])
@pytest.mark.parametrize(
("geom", "expected"),
Expand Down
Loading