ump45nose opened a new pull request, #11693: URL: https://github.com/apache/arrow-rs/pull/11693
# Which issue does this PR close? - Closes #11686. # Rationale for this change `MutableArrayData::extend` on sparse union `ArrayData` with a nonzero offset took the type ids from the right rows but copied the children's values from the rows before the offset. So each type id was paired with the wrong value. A sparse union's children are not sliced by the parent's offset, and `array.buffer::<i8>(0)` applies the offset only to the type ids. # What changes are included in this PR? `build_extend_sparse` in `arrow-data/src/transform/union.rs` adds `array.offset()` to `start` before it extends the children, the same way #10934 did for structs and #11636 did for fixed size lists. The overflow checks match those two fixes. Dense unions are not changed, because their offsets buffer already gives each row's child index. # Are these changes tested? Yes. `test_extend_sparse_union_accounts_for_parent_offset` in `arrow-data/src/data.rs` (next to the struct and fixed size list tests) builds a sparse union with two children and offset 1, extends it, and checks the type ids and both children. Without the fix, child `a` comes back as `[10, 20]` instead of `[20, 30]`. I also ran `cargo test -p arrow-data`, `cargo test -p arrow --test array_transform`, `cargo test -p arrow-select --lib` and `cargo clippy -p arrow-data --all-targets -- -D warnings`. # Are there any significant user-facing changes? No. AI disclosure: I used an AI coding assistant to write the fix, the test and this description, and I reviewed all of it. The part worth a reviewer's look is that the type ids keep using the already offset buffer while only the children get the added offset. -- 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]
