dragonknightk opened a new pull request, #12231: URL: https://github.com/apache/arrow-rs/pull/12231
# Which issue does this PR close? - Closes #11835 # Rationale for this change `shift`, `merge_n` and `union_extract` could write a null into a union child or a run-end encoded values child whose field is not nullable, and the result still passed `validate_full`. Issue 10992 settled that a kernel returns an error in this case. These kernels now follow that rule, as `take` and `nullif` already do. # What changes are included in this PR? - `shift` builds a union or run-end encoded result with `take` and null indices for the vacated rows. `take` stores those nulls in a union field that can hold them, or returns an error when no field can. - `union_extract` builds the null rows of a union or run-end encoded target the same way, in the four places that called `new_null_array` or zipped with it. - `merge_n` returns an error before merging when the arrays are run-end encoded with a non-nullable values field and an index is `None`. The check is in `merge_n` because `MutableArrayData` also writes nulls into such a child when a union or struct parent masks them, and those results are valid. These kernels reuse `take` rather than changing `new_null_array` or `ArrayData::new_null`. Neither can return an error, so making `ArrayData::new_null` pick a nullable union field, as the closed PR 11288 did, turns a union with no nullable field into a panic. `take` already implements the nullability rule from 10992 and returns an error. For `shift` this means an O(n) index array and a `take` call, but only for union and run-end encoded arrays; other data types keep their current code path. # Are these changes tested? Yes. New unit tests in `window.rs`, `union_extract.rs` and `merge.rs` cover a union with no nullable field, a union whose first field is not nullable but a later one is, and a run-end encoded array with a nullable or non-nullable values field, across the affected branches of each kernel. A `merge_n` test also checks that a sparse union with a non-nullable run-end encoded field still merges a null into its nullable field, and that a run-end encoded array with a nullable values field or no `None` index still merges. The error and union cases fail without this change. # Are there any significant user-facing changes? For a union whose first field is not nullable, `shift` and `union_extract` now put new nulls in the first field that can hold one, so the type ids of those rows differ from before; previously these nulls went into the non-nullable first field and the result was invalid. When the first field is nullable, the result is the same as before. # AI usage disclosure Human-led contribution. Grok Bot, working for dragonknightk, assisted with this change. All submitted content was reviewed by a human before submission. Grok Bot drafted the code changes, the unit tests and this description. (๑•̀ㅂ•́)و✧ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
