emilk opened a new pull request, #10748: URL: https://github.com/apache/arrow-rs/pull/10748
# Which issue does this PR close? No issue; found while reviewing https://github.com/apache/arrow-rs/pull/10730. # Rationale for this change `try_binary` skipped null handling when `a.null_count() == 0 && b.null_count() == 0`. That is a *physical* count. A `RunArray` never has a null buffer of its own, so an array whose values contain nulls took the no-nulls fast path and silently produced a result with no nulls at all: ``` values [Some(10), None, Some(30)] + [1, 1, 1] => [11, 1, 31] // row 1 should be null ``` A `DictionaryArray` with nullable values has the same problem. `binary` is unaffected: it has no such fast path. # What changes are included in this PR? Gate the fast path on `is_nullable`, which accounts for logical nulls. That check is allowed to be conservative, so an empty union of the logical nulls now falls back to the no-nulls path instead of `unwrap`ping. # Are these changes tested? Yes, a new test for a `RunArray` with nulls in its values. It fails on `main`. # Are there any user-facing changes? `try_binary` returns the correct nulls for run and dictionary arrays. No API change. -- 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]
