feat(vortex-spatial): support GeoArrow Geometry unions - #9368
feat(vortex-spatial): support GeoArrow Geometry unions#9368HarukiMoriarty wants to merge 2 commits into
Conversation
|
Three things need fixing before this lands, and one structural question about where the decode belongs. Everything after that is style.
|
|
A second pass turned up two more correctness problems and three structural ones, same commit. The unknown-type-ID fallback keeps the original offset
(false, false) => fallback_type_id,but the offsets are copied straight across with no matching substitution: let arrow_offsets = parts.offsets.as_slice::<i32>().to_vec();So the row leaves with a type ID naming one child and an offset that indexed a different one. Arrow validates dense offsets against the child the type ID selects, so Null slots are allowed to hold garbage:
|
2de8987 to
c198e3e
Compare
Merging this PR will improve performance by 13.85%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | words_gather_scalar[65536] |
9.4 µs | 8.3 µs | +13.85% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing nemo/geo-union (bcac46e) with nemo/dense-union (7459934)
Footnotes
-
89 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Signed-off-by: Nemo Yu <zyu379@wisc.edu>
c198e3e to
c3710a2
Compare
Selector-only kernels reorder and repeat per-child offsets, so a dense union must be compacted before it can be exported as an Arrow dense union. That compaction lived in the Geometry extension type even though nothing in it was geometric; it now sits with the encoding whose invariant creates the need, as `dense_union::compact_for_arrow`. Cleanups folded in along the way: - Drop the export's trailing null-verification loop, which re-read the union it had just built to assert a property `take` already produced. - Stop masking every sparse child on export: compaction already gathers a null index for invalid rows, so the mask was `children x len` of wasted work. - Materialize child lengths and null buffers once on import instead of calling `UnionArray::child` and `Array::is_valid` per row. - Skip the extension execute and Arrow export for empty children. - Canonicalize unselected variants to a constant of their default value, matching `UnionArray::constant`, and drop the `AllOr` match for one pass. - Share `with_selectors`, `tag_lookup` and `union_variants` rather than repeating them across the four compute kernels and two modules. - Drop the variant-dtype validation that `ExtDType::try_new` already runs. - Reuse the crate's `AllOr` scatter idiom in `mixed_geometry_boxes` and return corners like its sibling `row_boxes`. - Split the fused selector test into rstest cases and add the take and mask conformance suites. Signed-off-by: Nemo Yu <zyu379@wisc.edu>
Rationale for this change
GeoArrow Geometry columns use dense unions to mix geometry kinds and dimensions. Vortex needs a logical Geometry extension over its canonical Union dtype while retaining Arrow compact children through the external DenseUnion physical encoding.
What changes are included in this PR?
What APIs are changed? Are there any user-facing changes?
Adds the public Geometry and GeometryData spatial extension types and registers GeoArrow Geometry import/export in vortex_spatial::initialize. GeometryCollection fields are accepted as part of the standard schema, but selected GeometryCollection values remain unsupported.