feat(compute): add list_element - #1138
Conversation
693b7a0 to
a592af7
Compare
zeroshade
left a comment
There was a problem hiding this comment.
The list_element implementation has broad test coverage and the compute, array, and targeted race suites pass. However, I reproduced a blocking public API panic for an unsupported scalar first argument; this should return a dispatch/type error instead.
| return nil, fmt.Errorf("%w: list_element scalar output type %s is not supported", arrow.ErrNotImplemented, listType.Elem()) | ||
| } | ||
|
|
||
| listValue := args[0].(*ScalarDatum).Value.(scalar.ListScalar) |
There was a problem hiding this comment.
Blocking: This unchecked assertion makes unsupported scalar inputs panic instead of returning the normal dispatch/type error. For example, ListElement(ctx, ScalarDatum(Int32(7)), ScalarDatum(Int64(0))) panics with *scalar.Int32 is not scalar.ListScalar; the equivalent unsupported array input returns an error. Please guard the list-like/type assertion and either fall through to normal kernel dispatch or return arrow.ErrType, with a regression test for a non-list scalar argument.
Rationale for this change
Arrow-Go supports the list, large-list, list-view, and fixed-size-list array types, but callers currently need to read offsets and child arrays themselves to select one element from every row. A list_element kernel provides the common nested-data operation directly through compute.
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?
Yes. This adds the public compute.ListElement function. Existing functions and APIs are unchanged.