Skip to content

fix(arrow-data): don't panic on dictionary key overflow in interleave/concat - #10675

Open
okhsunrog wants to merge 1 commit into
apache:mainfrom
okhsunrog:fix-dictionary-key-overflow-panic
Open

fix(arrow-data): don't panic on dictionary key overflow in interleave/concat#10675
okhsunrog wants to merge 1 commit into
apache:mainfrom
okhsunrog:fix-dictionary-key-overflow-panic

Conversation

@okhsunrog

Copy link
Copy Markdown

Which issue does this PR close?

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.

MutableArrayData::new / with_capacities unconditionally concatenate
dictionaries whose values aren't the same underlying buffer, and
.expect() the result. When the combined dictionary genuinely exceeds
the key type's range (e.g. merging independently dictionary-encoded
partitions with UInt16 keys), this panics instead of surfacing a
catchable error. The same construction is used recursively for
dictionaries nested inside List/FixedSizeList/Struct/RunEndEncoded/
Union children, so the panic isn't limited to top-level dictionary
arrays.

Add fallible try_new / try_with_capacities that return
Err(ArrowError::DictionaryKeyOverflowError) instead of panicking, and
switch arrow-select's interleave_fallback and concat_fallback (the
paths reached for dictionary arrays that can't/shouldn't be merged),
including their recursive descent into nested children, to use them.
new()/with_capacities() keep their existing panicking behavior for all
other callers.
@github-actions github-actions Bot added arrow Changes to the arrow crate arrow-select arrow-data labels Aug 13, 2026
@okhsunrog

Copy link
Copy Markdown
Author

Also probably related to #10553

@okhsunrog

Copy link
Copy Markdown
Author

@Jefffrey could you review this one please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate arrow-data arrow-select

Projects

None yet

Development

Successfully merging this pull request may close these issues.

interleave/concat panic instead of erroring on genuine dictionary key overflow for Utf8View/BinaryView dictionaries

1 participant