feat(extensions): add VariantGet for path extraction from variant arrays - #1206
Open
nssalian wants to merge 3 commits into
Open
feat(extensions): add VariantGet for path extraction from variant arrays#1206nssalian wants to merge 3 commits into
nssalian wants to merge 3 commits into
Conversation
nssalian
marked this pull request as ready for review
August 16, 2026 01:37
nssalian
marked this pull request as draft
August 16, 2026 02:07
nssalian
marked this pull request as ready for review
August 16, 2026 03:06
zeroshade
requested changes
Aug 17, 2026
Comment on lines
+416
to
+431
| func (n *nullTracker) apply(arr arrow.Array) { | ||
| if arr == nil || arr.NullN() == 0 { | ||
| return | ||
| } | ||
| if n.valid == nil { | ||
| n.valid = make([]bool, n.length) | ||
| for i := range n.valid { | ||
| n.valid[i] = true | ||
| } | ||
| } | ||
| for i := 0; i < n.length; i++ { | ||
| if arr.IsNull(i) { | ||
| n.valid[i] = false | ||
| } | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
why do this instead of keeping the bitmap and then using BitmapOr? That would likely be simpler and more performant
Contributor
Author
There was a problem hiding this comment.
Dropped the []bool for a bitmap. Kept as BitmapAndAlloc on validity (1=valid), which is the validity-space equivalent of OR-ing null masks - commented on nullTracker.merge. Happy to spell it as BitmapOr on null-masks if you'd prefer
zeroshade
requested changes
Aug 17, 2026
zeroshade
left a comment
Member
There was a problem hiding this comment.
Some edge case tests that are missing which should probably get added:
- Mixed shredded/residual rows at root and nested levels
- Supported
AsTypeconversion matrix - Corrupted object metadata versus genuinely missing keys
- Field-on-scalar versus index-on-scalar semantics
- Index greater than
math.MaxUint32 - A missing path with a nested
AsType - Empty-string object key in the path
- Sliced and zero-length inputs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Reading one field from a variant column reassembles the whole value per row (
VariantArray.Value) then navigates to the field in Go. When the variant is shredded, that field is often already a typed column, so the full reassembly is wasted work.What changes are included in this PR?
VariantGet(input, GetOptions{Path, AsType, Safe, Mem}): extracts a path by following the shreddedtyped_valuecolumns as far as possible and only reassembling the residualvalueper-row for the rest. Mirrors the arrow-rs variant_getAsType == nilreturns aVariantArraypointing at the path; set it to get a typed array. Also extracts the variant-leaf -> typed-builder cast matrix fromshreddedPrimitiveBuilder.tryTypedinto a sharedappendVariantToTypedBuilderso the writer andVariantGetshare it (tryTypedbehavior unchanged).Follow-ups (out of scope here):
takekernel lives inarrow/compute, whicharrow/extensionscannot import (cycle); needs a local gather to push indices into the shredded columns.AsTypeoutput - struct/list target types returnarrow.ErrNotImplemented; would need a recursive per-field builder.Are these changes tested?
variant_get_test.gocovers the extraction paths (typed and variant output, nested path, array index, missing field, fallback, null rows, dictionary-encoded metadata, nested-type rejection).variant_get_internal_test.goadds a white-box check that the perfect-shredding fast path fires and aCheckedAllocatorleak check.Are there any user-facing changes?
Yes - new exported API:
VariantGet,GetOptions,VariantPath,VariantPathElement,VariantPathField,VariantPathIndex. Additive only.