-
Couldn't load subscription status.
- Fork 29
perf: ST_Buffer implementation using geo
#233
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
base: main
Are you sure you want to change the base?
Conversation
| } | ||
|
|
||
| #[test] | ||
| fn test_empty_geometry() { |
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.
The test suite here is the same as the geos buffer test suite plus this new function, which I also copied over to geos st_buffer to be sure it works the same.
| // PostGIS returns POLYGON EMPTY for all empty geometries | ||
| let is_empty = is_geometry_empty(wkb).map_err(|e| DataFusionError::External(Box::new(e)))?; | ||
| if is_empty { |
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.
I was running into an error here with POINT EMPTY since geo apparently doesn't support it.
sedona-db/rust/sedona-geo/src/to_geo.rs
Lines 62 to 66 in 3d75664
| not_impl_err!( | |
| "geo kernel implementation on {}, {}, or {} not supported", | |
| "MULTIPOINT with EMPTY child", | |
| "POINT EMPTY", | |
| "GEOMETRYCOLLECTION" |
My current workaround is to use WKBExecutor here instead of GeoTypesExecutor and use our native is_geometry_empty check.
Wondering if there's a better way we can handle empty points in our item_to_geometry() method. The docstring for geo's try_to_point() function we are in there says returning None represents an empty point. Though returning None is not a safe option, so I can't think of anything better than this workaround atm.
|
Are there any convenient existing ways to benchmark this against the |
This should work: # on main
cargo bench -- st_buffer
# switch to your branch
cargo bench -- st_buffer |
closes #232