feat(rust/sedona-functions): Implement ST_AsEWKB with item CRS support#535
feat(rust/sedona-functions): Implement ST_AsEWKB with item CRS support#535paleolimbot merged 18 commits intoapache:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the ST_AsEWKB function with support for CRS (Coordinate Reference System) metadata, enabling EWKB (Extended Well-Known Binary) output that includes SRID information.
Changes:
- Adds EWKB writer implementation with SRID support for all geometry types
- Implements
ST_AsEWKBscalar UDF with both type-level and item-level CRS support - Adds comprehensive test coverage for EWKB serialization across geometry types and dimensions
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rust/sedona-geometry/src/lib.rs | Exports new ewkb_factory module |
| rust/sedona-geometry/src/ewkb_factory.rs | Implements EWKB writer with SRID encoding for all geometry types |
| rust/sedona-functions/src/st_asewkb.rs | Implements ST_AsEWKB UDF with CRS support |
| rust/sedona-functions/src/register.rs | Registers ST_AsEWKB function in default function set |
| rust/sedona-functions/src/lib.rs | Declares st_asewkb module |
| rust/sedona-functions/src/executor.rs | Adds support for iterating over item CRS struct types |
| python/sedonadb/tests/functions/test_wkb.py | Adds parametrized tests comparing SedonaDB and PostGIS EWKB output |
| python/sedonadb/python/sedonadb/testing.py | Extends geom_or_null helper to support SRID parameter |
| python/sedonadb/pyproject.toml | Adds pandas version constraint as workaround |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| } | ||
| } | ||
|
|
||
| const ROUNDTRIP_CASES: [&str; 56] = [ |
There was a problem hiding this comment.
Can we add a test case that have very large SRID values and nested GeometryCollection?
There was a problem hiding this comment.
Good call! A few questionable things were happening for CRSes that couldn't be cleanly exported as an SRID
Kontinuation
left a comment
There was a problem hiding this comment.
Looks good. Hope that georust/wkb will directly support writing EWKB. It supports parsing EWKB so writing EWKB is probably a reasonable feature to have in georust/wkb.
| ScalarValue::Struct(s) | ||
| if s.fields().len() == 2 | ||
| && s.fields()[0].name() == "item" | ||
| && s.fields()[1].name() == "crs" => |
There was a problem hiding this comment.
If I remember it correctly, this pattern for decomposing fields in struct type representing item with crs has appeared many times in the code base. Maybe we can define a helper method for doing this.
Agreed! I can make a PR with roughly this at some point.
Agreed! I opened a follow-up for this ( #546 ). It was tricky to implement a helper initially because there are slightly different requirements in various places but I think we're close to the point where we've collected all the patterns we'll need. |
This PR implements an EWKB writer and uses it to implement ST_AsEWKB.