feat(rust/sedona-functions): implement ST_GeoHash - #1163
Conversation
Adds a native st_geohash(geometry, precision) scalar UDF that returns the base32 geohash string of a geometry, matching Apache Sedona Spark's ST_GeoHash semantics: - Non-point geometries hash the center of their bounding box (GeometryGeoHashEncoder.calculate) - Returns null when the bounding box is not fully contained in [-180, 180] x [-90, 90] - Precision <= 0 returns an empty string; precision is capped at 20 (PointGeoHashEncoder.calculateGeoHash) Expected test values are taken from Sedona's TestStGeoHash.scala.
The R package generates roxygen docs from the qmd description; bare
square brackets become Rd \link{} targets and fail R CMD check.
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Thank you!
Can you add integration tests to Python that verify this returns the same values as PostGIS?
| // Longitude can take values in [-180, 180]; latitude can take values in [-90, 90] | ||
| if x.lo() < -180.0 || y.lo() < -90.0 || x.hi() > 180.0 || y.hi() > 90.0 { | ||
| return Ok(None); | ||
| } |
There was a problem hiding this comment.
This would be better normalizing to the -180, 180 range and validating that we have lon/lat CRS (you could check what PostGIS does for this case...we have a "get geographic params" to check if this is a CRS we know is in longitude/latitude units, or maybe we only want this if it's bona-fide WGS84). This is easy for type level CRS but sort of a pain for item level CRS.
There was a problem hiding this comment.
postgis will error here, sedona spark wil return null. I kept the spark behavior here
I believe we already check for lon/lat CRS before this point
There was a problem hiding this comment.
If we've already checked for a lon/lat CRS, normalizing to the -180...180 range is less likely to introduce a dropped filter row by accident (while maintaining the property of leniency to avoid failures in the middle of large processing).
Repoint the sedona-db git dependencies from apache main @ 1245541b to james-willis/sedona-db @ d18b1dee (same base commit plus the ST_GeoHash implementation; still DataFusion 52.5.0). The fork URL keeps the pin reachable if upstream squash-merges. Verified end to end: ST_GeoHash output matches pygeohash at precisions 5 and 12.
2 participants
What changes were proposed in this PR?
Implement
ST_GeoHash(geometry, precision)returning the base-32 geohash string of the geometry, matching Apache Sedona Spark semantics:precision <= 0returns an empty string; precision caps at 20 (PointGeoHashEncoder.java:29-32)0123456789bcdefghjkmnpqrstuvwxyzalphabetThe encoder is implemented by hand (~40 lines) — no new crate dependency.
Follows crate conventions:
SedonaScalarUDF::new+ItemCrsKernel::wrap_impl,WkbExecutor,ArgMatcher::is_geometry() + is_integer()with per-row Int64-cast precision (modeled onst_geometryn), Utf8 return viaStringBuilder(modeled onst_geometrytype).Two deliberate deviations from Sedona Spark, documented in code:
minx=0, maxx=-1) and accidentally hashes empty geometries as (-0.5, -0.5); returning null is PostGIS-consistent.st_xmin/st_yminare geometry-only for the same planar-bbox reason.Why are the changes needed?
ST_GeoHashis used in Sedona Spark pipelines (e.g. for spatial bucketing/partitioning keys); sedona-db currently has no implementation, so such SQL fails with an unknown-function error.How was this patch tested?
8 new tests in
st_geohash.rswith expected values taken from Sedona Spark'sTestStGeoHash.scala(each vector cited in a comment), including WKB_VIEW_GEOMETRY and ITEM_CRS variants, precision capping, out-of-range nulls, and pole/antimeridian edge cases.cargo test -p sedona-functionspasses (513 tests), clippy and fmt clean.Did this PR include necessary documentation updates?
Function-level rustdoc included; happy to add a SQL reference doc page if maintainers point me at the right template.