-
Notifications
You must be signed in to change notification settings - Fork 44
Add ST_Relate implementation using GEOS #691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f0e8e3b
1966baa
1cfb7c9
6233f5c
882377c
5096780
98d8047
c6623a4
cedfd67
5136544
c0b1a02
c5f171e
9646893
40d256e
4b467fc
156993f
64359d7
e178a68
f609f38
89295ea
8a25cce
0aed5ea
22c3761
6dc2a3d
da305c3
f2ab05b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| // 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::StringBuilder; | ||
| use arrow_schema::DataType; | ||
| use datafusion_common::error::Result; | ||
| use datafusion_common::DataFusionError; | ||
| use datafusion_expr::ColumnarValue; | ||
| use geos::Geom; | ||
| use sedona_expr::{ | ||
| item_crs::ItemCrsKernel, | ||
| scalar_udf::{ScalarKernelRef, SedonaScalarKernel}, | ||
| }; | ||
| use sedona_schema::{datatypes::SedonaType, matchers::ArgMatcher}; | ||
|
|
||
| use crate::executor::GeosExecutor; | ||
|
|
||
| /// ST_Relate implementation using GEOS | ||
| pub fn st_relate_impl() -> Vec<ScalarKernelRef> { | ||
| ItemCrsKernel::wrap_impl(STRelate {}) | ||
| } | ||
|
|
||
| #[derive(Debug)] | ||
| struct STRelate {} | ||
|
|
||
| impl SedonaScalarKernel for STRelate { | ||
| fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> { | ||
| let matcher = ArgMatcher::new( | ||
| vec![ArgMatcher::is_geometry(), ArgMatcher::is_geometry()], | ||
| SedonaType::Arrow(DataType::Utf8), | ||
| ); | ||
|
|
||
| matcher.match_args(args) | ||
| } | ||
|
|
||
| fn invoke_batch( | ||
| &self, | ||
| arg_types: &[SedonaType], | ||
| args: &[ColumnarValue], | ||
| ) -> Result<ColumnarValue> { | ||
| let executor = GeosExecutor::new(arg_types, args); | ||
|
|
||
| // ST_Relate returns a 9-char DE-9IM string per row; 9 bytes * n rows | ||
| let mut builder = | ||
| StringBuilder::with_capacity(executor.num_iterations(), 9 * executor.num_iterations()); | ||
|
|
||
| executor.execute_wkb_wkb_void(|wkb1, wkb2| { | ||
| match (wkb1, wkb2) { | ||
| (Some(g1), Some(g2)) => { | ||
| let relate = g1 | ||
| .relate(g2) | ||
| .map_err(|e| DataFusionError::External(Box::new(e)))?; | ||
|
|
||
| builder.append_value(relate); | ||
| } | ||
| _ => builder.append_null(), | ||
| } | ||
| Ok(()) | ||
| })?; | ||
|
|
||
| executor.finish(Arc::new(builder.finish())) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use arrow_array::{create_array as arrow_array, ArrayRef}; | ||
| use datafusion_common::ScalarValue; | ||
| 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::create::create_array; | ||
| use sedona_testing::testers::ScalarUdfTester; | ||
|
|
||
| use super::*; | ||
|
|
||
| #[rstest] | ||
| fn udf(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType) { | ||
| let udf = SedonaScalarUDF::from_impl("st_relate", st_relate_impl()); | ||
| let tester = ScalarUdfTester::new(udf.into(), vec![sedona_type.clone(), sedona_type]); | ||
| tester.assert_return_type(DataType::Utf8); | ||
|
|
||
| // Two disjoint points — DE-9IM should be "FF0FFF0F2" | ||
| let result = tester | ||
| .invoke_scalar_scalar("POINT (0 0)", "POINT (1 1)") | ||
| .unwrap(); | ||
| tester.assert_scalar_result_equals(result, "FF0FFF0F2"); | ||
|
|
||
| // NULL inputs should return NULL | ||
| let result = tester | ||
| .invoke_scalar_scalar(ScalarValue::Null, ScalarValue::Null) | ||
| .unwrap(); | ||
| assert!(result.is_null()); | ||
|
|
||
| // Array inputs | ||
| let lhs = create_array( | ||
| &[ | ||
| Some("POLYGON ((0 0, 0 1, 1 1, 1 0, 0 0))"), | ||
| Some("POINT (0.5 0.5)"), | ||
| None, | ||
| ], | ||
| &WKB_GEOMETRY, | ||
| ); | ||
| let rhs = create_array( | ||
| &[ | ||
| Some("POINT (0.5 0.5)"), | ||
| Some("POLYGON ((0 0, 0 1, 1 1, 1 0, 0 0))"), | ||
| Some("POINT (0 0)"), | ||
| ], | ||
| &WKB_GEOMETRY, | ||
| ); | ||
|
|
||
| // actual values from GEOS | ||
| let expected: ArrayRef = arrow_array!(Utf8, [Some("0F2FF1FF2"), Some("0FFFFF212"), None]); | ||
| assert_array_equal(&tester.invoke_array_array(lhs, rhs).unwrap(), &expected); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| --- | ||
| # 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_Relate | ||
| description: Returns the DE-9IM intersection matrix string for two geometries. | ||
| kernels: | ||
| - returns: string | ||
| args: [geometry, geometry] | ||
| --- | ||
| ## Description | ||
| Returns the DE-9IM (Dimensionally Extended 9-Intersection Model) intersection matrix | ||
| as a 9-character string describing the spatial relationship between two geometries. | ||
|
|
||
| ## Examples | ||
| ```sql | ||
| SELECT ST_Relate( | ||
| ST_GeomFromWKT('POINT(0 0)'), | ||
| ST_GeomFromWKT('POINT(1 1)') | ||
| ); | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -442,3 +442,56 @@ def test_st_overlaps(eng, geom1, geom2, expected): | |
| f"SELECT ST_Overlaps({geom_or_null(geom1)}, {geom_or_null(geom2)})", | ||
| expected, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("eng", [SedonaDB, PostGIS]) | ||
| @pytest.mark.parametrize( | ||
| ("geom1", "geom2", "expected"), | ||
| [ | ||
| (None, None, None), | ||
| ("POINT (0 0)", None, None), | ||
| (None, "POINT (0 0)", None), | ||
| ("POINT (0 0)", "POINT (1 1)", "FF0FFF0F2"), | ||
| ("POINT (0 0)", "POINT (0 0)", "0FFFFFFF2"), | ||
| ("POINT (0 0)", "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))", "F0FFFF212"), | ||
| ("POINT (0.5 0.5)", "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))", "0FFFFF212"), | ||
| ( | ||
| "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))", | ||
| "POLYGON ((5 5, 6 5, 6 6, 5 6, 5 5))", | ||
| "FF2FF1212", | ||
| ), | ||
| ( | ||
| "POLYGON ((0 0, 2 0, 2 2, 0 2, 0 0))", | ||
| "POLYGON ((1 1, 3 1, 3 3, 1 3, 1 1))", | ||
| "212101212", | ||
| ), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to add some more edge cases. (We are usually more comprehensive for the Python tests, since they are very concise to write) Could we add the following test cases? (These are just the inputs, you need to add the expected output values. I find the easiest way is to just run the tests, and copy-paste the outputs that come out. If both our sedonaDB implementation and PostGIS agree, we're good!) If you come up with any other interesting edge cases, you're welcome to add them too.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion! I’ll add these edge cases to the Python tests and determine the expected outputs by comparing SedonaDB and PostGIS results. |
||
| ("POINT (0 0)", "LINESTRING (0 0, 1 1)", "F0FFFF102"), | ||
| ("LINESTRING (0 0, 2 2)", "LINESTRING (1 1, 3 3)", "1010F0102"), | ||
| ( | ||
| "GEOMETRYCOLLECTION (POINT (0 0), LINESTRING (0 0, 1 1))", | ||
| "POINT (0 0)", | ||
| "FF10F0FF2", | ||
| ), | ||
| ( | ||
| "POLYGON ((0 0, 2 0, 2 2, 0 2, 0 0))", | ||
| "POLYGON ((2 0, 4 0, 4 2, 2 2, 2 0))", | ||
| "FF2F11212", | ||
| ), # touching polygons | ||
| ( | ||
| "POLYGON ((0 0, 4 0, 4 4, 0 4, 0 0))", | ||
| "POLYGON ((1 1, 2 1, 2 2, 1 2, 1 1))", | ||
| "212FF1FF2", | ||
| ), # polygon containment | ||
| ( | ||
| "POLYGON ((0 0, 6 0, 6 6, 0 6, 0 0), (2 2, 4 2, 4 4, 2 4, 2 2))", | ||
| "POINT (1 1)", | ||
| "0F2FF1FF2", | ||
| ), # point in a polygon hole | ||
| ], | ||
| ) | ||
| def test_st_relate(eng, geom1, geom2, expected): | ||
| eng = eng.create_or_skip() | ||
| eng.assert_query_result( | ||
| f"SELECT ST_Relate({geom_or_null(geom1)}, {geom_or_null(geom2)})", | ||
| expected, | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the actual rust implementation. We're going to need to add Rust tests in this file as well as python integration tests. You can take a look at this example PR (#288) to guide you if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oki, let me check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are still missing rust and python tests. Both of these are very important to have before we merge. The example PR has examples of these as well.
The rust tests should exist in this file, starting with:
The new python tests for st_relate should go in test_predicates.py (since st_relate is a predicate). Make sure to read my comment (#288 (comment)) about how to iterate on developing Python integration tests
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the guidance! I’ll add the Rust tests in the module using
#[cfg(test)] mod tests {}and implement the Python tests forst_relateintest_predicates.py,following the approach in #288.