paleolimbot commented on code in PR #533:
URL: https://github.com/apache/sedona-db/pull/533#discussion_r2713372737
##########
rust/sedona-functions/src/st_geomfromwkb.rs:
##########
@@ -77,6 +92,29 @@ fn doc(name: &str, out_type_name: &str) -> Documentation {
.build()
}
+/// Documentation for `ST_GeomFromWKBUnchecked()`.
+///
+/// Parameterized for reuse if `ST_GeogFromWKBUnchecked()` is implemented in
the future.
+fn doc_unchecked(name: &str, out_type_name: &str) -> Documentation {
Review Comment:
Yes, it's currently being updated manually. It is also in a state of flux
right now where we're trying to separate the pages from each other in a way
that each page can have a bit more detail, but still generate the main page.
For now having the doc reference like you did here is perfect.
##########
python/sedonadb/tests/functions/test_functions.py:
##########
@@ -1668,6 +1669,49 @@ def test_st_geomfromwkb(eng, geom):
eng.assert_query_result(f"SELECT ST_GeomFromWKB({wkb})", expected)
+# `ST_GeomFromWKBUnchecked` is not available in PostGIS
[email protected]("eng", [SedonaDB])
[email protected](
+ ("geom"),
+ [
+ "POINT (1 1)",
+ "POINT EMPTY",
+ "LINESTRING EMPTY",
+ "POLYGON EMPTY",
+ "GEOMETRYCOLLECTION EMPTY",
+ "POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))",
+ "MULTILINESTRING ((0 0, 1 1), (1 1, 2 2))",
+ "GEOMETRYCOLLECTION (POINT (0 0), LINESTRING (0 0, 1 1), POLYGON ((0
0, 0 1, 1 1, 1 0, 0 0)))",
+ ],
+)
+def test_st_geomfromwkbunchecked(eng, geom):
+ eng = eng.create_or_skip()
+
+ expected = geom
+ if geom == "POINT EMPTY":
+ # arrow-c returns POINT (nan nan) instead of POINT EMPTY
+ expected = "POINT (nan nan)"
+
+ wkb = shapely.from_wkt(geom).wkb
+ wkb = "0x" + wkb.hex()
+
+ eng.assert_query_result(f"SELECT ST_GeomFromWKBUnchecked({wkb})", expected)
+
+
[email protected]("eng", [SedonaDB])
+def test_st_geomfromwkbunchecked_invalid_wkb(eng):
+ eng = eng.create_or_skip()
+
+ # Invalid WKB payload can still convert to geometry column
+ eng.assert_query_result(
+ "SELECT ST_AsBinary(ST_GeomFromWKBUnchecked(0x01))", b"\x01"
+ )
+
+ # TODO: Test error case after error message check is supported in
`assert_query_result`
+ # Using invalid WKB elsewhere may result in undefined behavior.
+ # eng.assert_query_result("SELECT
ST_AsText(ST_GeomFromWKBUnchecked(0x01))", None)
Review Comment:
I think sometimes we do:
```python
with pytest.raises(lib.SedonaError, match="foofy"):
eng.execute_and_collect("SELECT ...")
```
Would that work here? If not I think it is OK to skip this check for now.
--
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]
