okhsunrog opened a new pull request, #10675: URL: https://github.com/apache/arrow-rs/pull/10675
# Which issue does this PR close? - Closes #10674. # Rationale for this change `interleave()` and `concat()` document a `Result` return type, but for `Dictionary<K, Utf8View>`/`Dictionary<K, BinaryView>` arrays (top-level or nested inside a `List`/`FixedSizeList`/`Struct`/`RunEndEncoded`/`Union`), a genuine dictionary key overflow currently panics instead of returning `Err`. This happens because the fallback path builds a `MutableArrayData` directly and `.expect()`s the dictionary-concat result, and `MutableArrayData::new`/`with_capacities` have no fallible variant. See #10674 for the full analysis and a minimal repro. # What changes are included in this PR? - Add `MutableArrayData::try_new` / `try_with_capacities`, fallible variants of `new` / `with_capacities` that return `Err(ArrowError::DictionaryKeyOverflowError)` instead of panicking on dictionary key overflow. `new()`/`with_capacities()` keep their existing panicking behavior (now implemented as `try_with_capacities(..).expect(..)`) for the many existing callers that rely on infallibility. - Recursive child construction inside `try_with_capacities` (for `List`/`LargeList`/`Map`/`ListView`/`LargeListView`/`FixedSizeList`/`Struct`/`RunEndEncoded`/`Union`) also uses the fallible variants and propagates errors with `?`, so dictionaries nested inside container types are covered, not just top-level dictionary arrays. - Switch `arrow-select`'s `interleave_fallback` and `concat_fallback` (the paths reached for dictionary arrays whose values can't/shouldn't be merged) to the fallible constructors. # Are these changes tested? Yes: - `concat_string_view_dictionary_overflow_returns_err` / `test_interleave_string_view_dictionary_overflow_returns_err`: top-level `Dictionary<UInt8, Utf8View>` overflow returns `Err` instead of panicking. - `concat_nested_dictionary_overflow_returns_err` / `test_interleave_nested_dictionary_overflow_returns_err`: same overflow nested inside a `FixedSizeList`, exercising the recursive child construction. Full `arrow-data`/`arrow-select` test suites pass (410 tests), `cargo fmt --check` and `cargo clippy --all-targets -- -D warnings` are clean for both crates. # Are there any user-facing changes? No breaking changes. `MutableArrayData::new`/`with_capacities` keep their documented panicking behavior and signatures. Two new public fallible methods are added (`try_new`, `try_with_capacities`). `interleave()`/`concat()` keep their existing `Result` signature -- the only visible change is that a specific previously-panicking input (genuine dictionary key overflow on `Utf8View`/`BinaryView` dictionaries) now returns `Err(ArrowError::DictionaryKeyOverflowError)` instead. -- 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]
