Skip to content

test(integration): RS_Envelope, RS_ConvexHull, RS_PixelAs* parity (item-level-CRS geometry in the harness) - #1286

Open
james-willis wants to merge 2 commits into
apache:mainfrom
james-willis:jw/parity-geom-outputs
Open

test(integration): RS_Envelope, RS_ConvexHull, RS_PixelAs* parity (item-level-CRS geometry in the harness)#1286
james-willis wants to merge 2 commits into
apache:mainfrom
james-willis:jw/parity-geom-outputs

Conversation

@james-willis

Copy link
Copy Markdown
Contributor

Completes parity coverage of the geometry-returning raster functions, closing out the batch #1264 unblocked: RS_Envelope, RS_ConvexHull, RS_PixelAsPoint, RS_PixelAsCentroid, RS_PixelAsPolygon — one file per function, every case probed on both engines first.

Harness: item-level-CRS geometry (first commit)

These five functions return geometry whose CRS can vary per row, which SedonaDB represents as struct<item: geoarrow.wkb, crs> — the tuple stringifier could not cast it ("Unsupported cast from struct"). result_to_tuples now unwraps the struct and renders the geometry child as WKT like any other geometry column (parent nulls propagated via pc.struct_field); the per-item crs field has no counterpart in the other engine's result and stays covered through the RS_CRS suite. RS_WorldToRasterCoord was unaffected because its pixel-space output carries no CRS.

What the probes found

Mostly good news — four of the five functions agree exactly, anchored with exact WKT:

  • RS_ConvexHull: identical ring on both engines (clockwise from the upper-left corner), with and without a raster CRS.
  • RS_PixelAsPoint / Centroid / Polygon (in grid): both engines read the pixel coordinate 1-based; corners, centroids, and footprints match string-for-string.
  • RS_PixelAsCentroid / Polygon (out of grid): both engines extrapolate along the geotransform, identically — anchored.

Two divergences, xfail-cataloged:

  • RS_Envelope ring order: same rectangle, opposite winding — SedonaDB counter-clockwise from the lower-left, Sedona Spark clockwise. (Spark's envelope winding also differs from Spark's own convex-hull winding.)
  • RS_PixelAsPoint out of grid: SedonaDB extrapolates ((0, 0)POINT (98 503)); Sedona Spark raises IndexOutOfBoundsException ("Specified pixel coordinates (0, 0) do not lie in the raster") — even though Spark's own RS_PixelAsCentroid and RS_PixelAsPolygon extrapolate happily. An internal inconsistency on the Spark side.

Full suite: 188 passed, 103 xfailed.

With this, every dual-engine RS_ function has a parity file except RS_Values (list-column results; its coordinate-array overload also has no SQL spelling both engines parse) and RS_AsGeoTiff (binary output, no shared round-trip SQL).

SedonaDB returns geometry whose CRS can vary per row (RS_Envelope,
RS_PixelAsPoint, ...) as struct<item: geoarrow.wkb, crs>, which the
tuple stringifier could not cast. Render the geometry child as WKT like
any other geometry column; the per-item crs field has no counterpart in
other engines' results and stays covered through RS_CRS.

@paleolimbot paleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integration tests look great! I'm not sold on the handling of item crs in the tuples converter, but also our current handling is not great either.

Comment on lines +293 to +298
Geometry columns are rendered as WKT strings — including SedonaDB's
item-level-CRS geometry (``struct<item: geoarrow.wkb, crs>``, returned
by RS_Envelope and friends whose output CRS can vary per row), which is
unwrapped to its geometry child first; the per-item crs field has no
counterpart in other engines' results and is asserted through RS_CRS
coverage instead. List columns (e.g. the `List<Double>` returned by

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this particular context, item crs could come back as {"item": <wkt>, "crs": <crs>} or (<wkt>, <crs>). I am not sure that ditching the CRS is a good idea for this specific function (you could always issue a separate query with just the geometry field if this is what you wanted to test?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants