diff --git a/rust/geodatafusion/src/udf/native/accessors/is_empty.rs b/rust/geodatafusion/src/udf/native/accessors/is_empty.rs index cfa6bf4..bdc651c 100644 --- a/rust/geodatafusion/src/udf/native/accessors/is_empty.rs +++ b/rust/geodatafusion/src/udf/native/accessors/is_empty.rs @@ -61,7 +61,10 @@ impl ScalarUDFImpl for IsEmpty { Some(DOCUMENTATION.get_or_init(|| { Documentation::builder( DOC_SECTION_OTHER, - "Tests if a geometry is empty. ST_IsEmpty(NULL) is NULL.", + "Tests if a geometry is topologically empty. \ + Multi-geometries and GeometryCollections where every leaf is empty \ + (e.g. GEOMETRYCOLLECTION(POINT EMPTY, POLYGON EMPTY) are reported empty. \ + ST_IsEmpty(NULL) is NULL.", "ST_IsEmpty(geom)", ) .with_argument("geom", "geometry") @@ -70,6 +73,38 @@ impl ScalarUDFImpl for IsEmpty { } } +/// Recursive emptiness predicate. +/// +/// This matches the behavior of PostGIS and JTS. +/// As a cross-reference, the javadoc for JTS `Geometry.isEmpty()` reads as follows: +/// "the emptiness test checks for topological emptiness, +/// not structural emptiness. A collection containing only empty elements is reported as empty." +pub(crate) fn is_geometry_topologically_empty(geom: &impl GeometryTrait) -> bool { + match geom.as_type() { + geo_traits::GeometryType::Point(p) => p.coord().is_none(), + geo_traits::GeometryType::LineString(ls) => ls.num_coords() == 0, + geo_traits::GeometryType::Polygon(p) => { + p.exterior().is_none_or(|ring| ring.num_coords() == 0) + } + geo_traits::GeometryType::MultiPoint(mp) => mp.points().all(|p| p.coord().is_none()), + geo_traits::GeometryType::MultiLineString(mls) => { + mls.line_strings().all(|ls| ls.num_coords() == 0) + } + geo_traits::GeometryType::MultiPolygon(mp) => mp + .polygons() + .all(|p| p.exterior().is_none_or(|ring| ring.num_coords() == 0)), + geo_traits::GeometryType::GeometryCollection(gc) => gc + .geometries() + .all(|child| is_geometry_topologically_empty(&child)), + // Rect/Triangle/Line always carry coordinates and so are never empty. + // NB: geoarrow has no curved-geometry support, so e.g. CIRCULARSTRING EMPTY + // from the PostGIS docs is not representable at the moment, but we can still model them. + geo_traits::GeometryType::Rect(_) + | geo_traits::GeometryType::Triangle(_) + | geo_traits::GeometryType::Line(_) => false, + } +} + fn is_empty_impl(args: ScalarFunctionArgs) -> GeoDataFusionResult { let array = ColumnarValue::values_to_arrays(&args.args)? .into_iter() @@ -92,28 +127,11 @@ fn impl_is_empty<'a>( for item in array.iter() { match item { // A present geometry. - // Emptiness is the absence of coordinates, - // which is distinct from a SQL NULL (handled below). + // Emptiness is recursive so that it's topological rather than just the structure. + // so a collection where every leaf is empty counts as empty itself. Some(geom) => { let geom = geom?; - let is_empty = match geom.as_type() { - geo_traits::GeometryType::Point(p) => p.coord().is_none(), - geo_traits::GeometryType::LineString(ls) => ls.num_coords() == 0, - geo_traits::GeometryType::Polygon(p) => { - p.exterior().is_none_or(|ring| ring.num_coords() == 0) - } - geo_traits::GeometryType::MultiPoint(mp) => mp.num_points() == 0, - geo_traits::GeometryType::MultiLineString(mls) => mls.num_line_strings() == 0, - geo_traits::GeometryType::MultiPolygon(mp) => mp.num_polygons() == 0, - geo_traits::GeometryType::GeometryCollection(gc) => gc.num_geometries() == 0, - // Rect/Triangle/Line always carry coordinates and so are never empty. - // (geoarrow has no curved-geometry support, so e.g. CIRCULARSTRING EMPTY - // from the PostGIS docs is not representable here.) - geo_traits::GeometryType::Rect(_) - | geo_traits::GeometryType::Triangle(_) - | geo_traits::GeometryType::Line(_) => false, - }; - builder.append_value(is_empty); + builder.append_value(is_geometry_topologically_empty(&geom)); } // SQL NULL in, SQL NULL out. // This matches the PostGIS behavior, @@ -134,10 +152,10 @@ mod test { use datafusion::prelude::SessionContext; use super::*; - use crate::udf::native::io::GeomFromText; + use crate::udf::native::io::{GeomFromText, GeomFromWKB}; #[tokio::test] - async fn test_st_isempty() { + async fn test_st_isempty_wkt() { let ctx = SessionContext::new(); ctx.register_udf(IsEmpty::new().into()); ctx.register_udf(GeomFromText::new(Default::default()).into()); @@ -203,6 +221,103 @@ mod test { } } + #[tokio::test] + async fn test_structural_emptiness() { + let ctx = SessionContext::new(); + ctx.register_udf(IsEmpty::new().into()); + ctx.register_udf(GeomFromText::new(Default::default()).into()); + ctx.register_udf(GeomFromWKB::new(Default::default()).into()); + + // Recursive emptiness (PostGIS / JTS topological semantics). + // A collection whose every leaf is empty is itself reported as empty, + // even though it has structurally non-zero children. + let wkt_cases = vec![ + // NB: GeoArrow explicitly denies recursive collections, + // so we only need to test one level. + ( + "GEOMETRYCOLLECTION(POINT EMPTY, POLYGON EMPTY)", + true, + "collection of all-empty atomic children is recursively empty", + ), + ( + "GEOMETRYCOLLECTION(MULTIPOINT EMPTY, MULTIPOLYGON EMPTY)", + true, + "collection of all-empty multi-children is recursively empty", + ), + ( + "GEOMETRYCOLLECTION(POINT(0 0), POLYGON EMPTY)", + false, + "collection with at least one non-empty child is non-empty", + ), + ( + "MULTILINESTRING(EMPTY, EMPTY)", + true, + "multi-geometry where every member is empty is recursively empty", + ), + ( + "MULTILINESTRING(EMPTY, (0 0, 1 1))", + false, + "multi-geometry with at least one non-empty member is non-empty", + ), + ]; + + for (wkt, expected, description) in wkt_cases { + let sql = format!("SELECT ST_IsEmpty(ST_GeomFromText('{}'))", wkt); + let df = ctx + .sql(&sql) + .await + .unwrap_or_else(|_| panic!("Failed to execute SQL for {}", description)); + + let batch = df.collect().await.unwrap().into_iter().next().unwrap(); + let col = batch.column(0).as_boolean(); + + assert_eq!(col.value(0), expected, "Failed on {}: {}", description, wkt); + } + + // The following fixtures fail to parse or crash in WKT form, so we test via WKB + let wkb_cases = vec![ + ( + // MULTIPOINT (EMPTY, EMPTY) + "010400000000000000", + true, + "multi-geometry where every member is empty is recursively empty", + ), + ( + // MULTIPOLYGON (EMPTY, EMPTY) + "010600000000000000", + true, + "multi-geometry where every member is empty is recursively empty", + ), + ( + // MULTIPOINT(EMPTY, (0 0)) + "0104000000020000000101000000000000000000f87f000000000000f87f010100000000000000000000000000000000000000", + false, + "multi-geometry with at least one non-empty member is non-empty", + ), + // This crashes geoarrow-array-0.8.0/src/builder/multipolygon.rs:188:51. + // There's an unconditional unwrap. + // ( + // // MULTIPOLYGON(EMPTY, ((0 0, 1 0, 1 1, 0 0))) + // "0106000000020000000103000000000000000103000000010000000400000000000000000000000000000000000000000000000000f03f0000000000000000000000000000f03f000000000000f03f00000000000000000000000000000000", + // false, + // "multi-geometry with at least one non-empty member is non-empty", + // ), + ]; + + for (wkb, expected, description) in wkb_cases { + let sql = format!("SELECT ST_IsEmpty(ST_GeomFromWKB(X'{}'))", wkb); + let df = ctx + .sql(&sql) + .await + .unwrap_or_else(|_| panic!("Failed to execute SQL for {}", description)); + + let batch = df.collect().await.unwrap().into_iter().next().unwrap(); + let col = batch.column(0).as_boolean(); + + assert_eq!(col.value(0), expected, "Failed on {}: {}", description, wkb); + } + } + #[tokio::test] async fn test_st_isempty_null() { let ctx = SessionContext::new();