yongster opened a new pull request, #10909: URL: https://github.com/apache/arrow-rs/pull/10909
# Which issue does this PR close? - Closes #10908. # Rationale for this change `take` leaves a null in the output when the index at that slot is null. That is already true for primitive arrays, dictionaries, lists, and sparse unions. Two arms did not: 1. **Dense `UnionArray`** — `take_native` fills a null index with offset `0`, and `PrimitiveArray::try_new(..., None)` dropped the index null bitmap. The fake offset was then used to take from the child, so the first child value was copied into that slot. 2. **`RunEndEncoded`** — `take_run` called `get_physical_indices(logical_indices.values())`. A null integer slot still has a backing value (typically `0`), so the lookup used logical index 0 and produced the first run's value. Examples on current `main`: - Dense union `[1, 2, 3]` with indices `[0, null, 2]` became `[1, 1, 3]` - REE `[10, 10, 99, …]` with the same indices became `[10, 10, 99]` - Sparse union and primitive `take` already return `[1, null, 3]` / `[10, null, 99]` # What changes are included in this PR? - Dense union keeps the index null bitmap on the taken offsets, so `take` of each child writes a null instead of reading offset 0. - `take_run` maps only valid indices to physical runs. A null logical index becomes a null run; consecutive nulls are merged. - Add regression tests for both arms, including a dense vs sparse comparison. # Are these changes tested? Yes. I ran: ```text cargo test -p arrow-select --lib -- take cargo clippy -p arrow-select --all-targets --all-features -- -D warnings cargo fmt -- arrow-select/src/take.rs ``` -- 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]
