Skip to content

arrow-row: Fix decode_fixed_size_list to apply the corrected_type step for dictionary children - #10414

Merged
Jefffrey merged 4 commits into
apache:mainfrom
zhuqi-lucas:fix/decode-fsl-dict
Jul 24, 2026
Merged

arrow-row: Fix decode_fixed_size_list to apply the corrected_type step for dictionary children#10414
Jefffrey merged 4 commits into
apache:mainfrom
zhuqi-lucas:fix/decode-fsl-dict

Conversation

@zhuqi-lucas

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #10413.

Rationale for this change

RowConverter::convert_rows used to fail with a schema-mismatch InvalidArgumentError on any target type that contained a FixedSizeList<Dictionary<K, V>>:

InvalidArgumentError("FixedSizeListArray expected data type Dictionary(Int32, Utf8) got Utf8 for \"item\"")

decode_fixed_size_list was building the returned array with the declared element_field (which still carried Dictionary<...>) while the children came from converter.convert_raw, which returns the flattened (non-dictionary) type. try_new_with_length validates the child type against element_field and returned Err.

The other list-like decoders — List, LargeList, ListView, LargeListView, Map — already reconcile declared-vs-actual via a corrected_type step (see list::decode<L> at list.rs:~260 on main), and Codec::Struct in lib.rs does the same via corrected_fields. decode_fixed_size_list was the only list-like decoder that skipped this pattern.

What changes are included in this PR?

  • arrow-row/src/list.rs::decode_fixed_size_list: adopt the corrected_type step, mirroring the shape of every other list-like decoder in the file.
  • arrow-row/src/lib.rs::tests::test_fixed_size_list_of_dictionaries_round_trips: regression test that reproduces the original failure on one row ["a", "b"] of FixedSizeList<Dictionary<Int32, Utf8>, 2>.

Diff on the decoder is ~10 lines:

let mut children = unsafe { converter.convert_raw(&mut child_rows, validate_utf8) }?;
assert_eq!(children.len(), 1);

// Since `RowConverter` flattens certain data types (i.e. `Dictionary`),
// we need to use the child's actual data type rather than the declared
// `element_field`'s. Mirrors the `corrected_type` logic in `decode<L>`
// above and `Codec::Struct`'s `corrected_fields` in `lib.rs`.
let corrected_element_field = Arc::new(
    element_field
        .as_ref()
        .clone()
        .with_data_type(children[0].data_type().clone()),
);

FixedSizeListArray::try_new_with_length(
    corrected_element_field,
    *size,
    children.pop().unwrap(),
    nulls,
    num_rows,
)

Are these changes tested?

Yes:

  • test_fixed_size_list_of_dictionaries_round_trips — regression test in arrow-row/src/lib.rs. Pre-fix it panics at the first convert_rows(...).unwrap() (matching the report in arrow-row: decode_fixed_size_list panics on FixedSizeList<Dictionary> (missing corrected_type) #10413); post-fix it returns a FixedSizeList<Utf8> (same flattened shape produced today for List<Dictionary<...>>) whose values are "a", "b".
  • All existing arrow-row tests continue to pass (cargo test -p arrow-row).
  • cargo clippy -p arrow-row --all-targets -- -D warnings clean.

Are there any user-facing changes?

Behaviour change (bug fix): RowConverter::convert_rows now succeeds where it previously returned an InvalidArgumentError for FixedSizeList<Dictionary<...>> (and any type containing one, e.g. FSL<Struct<Dict>>, List<FSL<Dict>>). The returned array flattens the dictionary child to its native representation — same behaviour today's List<Dict> / LargeList<Dict> / Map<Dict> decoders exhibit. Callers that need the declared dictionary type back can re-encode after convert_rows (that's the documented contract).

No public API surface changes.

Downstream context

DataFusion hit this in apache/datafusion#23523 (nested-type support in GroupValuesColumn) where a GROUP BY fsl_col on FixedSizeList<Dictionary<Int32, Utf8>> would panic on emit. The DataFusion PR is landing a defensive blacklist in the meantime; this fix will let that blacklist be lifted in a follow-up.

…onary children (apache#10413)

Closes apache#10413.

`RowConverter::convert_rows` used to fail with a schema-mismatch
InvalidArgumentError on any target type that contained a
`FixedSizeList<Dictionary<K, V>>`:

    InvalidArgumentError("FixedSizeListArray expected data type
     Dictionary(Int32, Utf8) got Utf8 for \"item\"")

`decode_fixed_size_list` was building the returned array with the
declared `element_field` (which still carried `Dictionary<...>`) while
its children came from `converter.convert_raw`, which returns the
flattened (non-dictionary) type. `try_new_with_length` validates the
child type against `element_field` and returned Err.

The other list-like decoders — `List`, `LargeList`, `ListView`,
`LargeListView`, `Map` — already reconcile declared-vs-actual via a
`corrected_type` step (see `list::decode<L>` at list.rs:~260), and
`Codec::Struct` in `lib.rs` does the same via `corrected_fields`.
`decode_fixed_size_list` is the only list-like decoder that skipped
this pattern. This commit adopts it, matching the shape of every
other list-like decode path in the file.

Test: `test_fixed_size_list_of_dictionaries_round_trips` reproduces
the original failure on one row `["a", "b"]` of
`FixedSizeList<Dictionary<Int32, Utf8>, 2>`. Pre-fix it panicked at
the first `convert_rows(...).unwrap()` (matching the report in
apache#10413); post-fix it returns a `FixedSizeList<Utf8>` (same flattened
shape produced for `List<Dictionary<...>>` today) whose values are
`"a"`, `"b"`.

Downstream context: DataFusion hit this in
apache/datafusion#23523 where a
`GROUP BY fsl_col` on a `FixedSizeList<Dictionary<Int32, Utf8>>`
would panic on emit. The DataFusion PR is landing a defensive
blacklist in the meantime; this fix lets that blacklist be lifted
in a follow-up.
Copilot AI review requested due to automatic review settings July 23, 2026 04:32
@github-actions github-actions Bot added the arrow Changes to the arrow crate label Jul 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes arrow-row decoding for FixedSizeList<Dictionary<..>> by ensuring decode_fixed_size_list uses the actual decoded child data type (post-dictionary-flattening) when constructing the returned FixedSizeListArray, matching the existing “corrected type” behavior of other list-like decoders.

Changes:

  • Update decode_fixed_size_list to rebuild the element Field with the decoded child’s (possibly flattened) DataType before calling FixedSizeListArray::try_new_with_length
  • Add a regression test covering round-tripping FixedSizeList<Dictionary<Int32, Utf8>, 2> through RowConverter::{convert_columns, convert_rows}

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
arrow-row/src/list.rs Applies “corrected type” logic to FixedSizeList decoding to avoid schema mismatch when dictionary children are flattened
arrow-row/src/lib.rs Adds regression test ensuring convert_rows succeeds and returns a self-consistent flattened child type for FixedSizeList<Dictionary<..>>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Jefffrey Jefffrey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the comments seem overly verbose

Comment thread arrow-row/src/list.rs Outdated
Comment thread arrow-row/src/lib.rs Outdated
zhuqi-lucas and others added 2 commits July 23, 2026 14:09
Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
@zhuqi-lucas

Copy link
Copy Markdown
Contributor Author

the comments seem overly verbose

Thanks @Jefffrey for good suggestion, applied now.

@Jefffrey
Jefffrey merged commit a5158c8 into apache:main Jul 24, 2026
14 checks passed
@Jefffrey

Copy link
Copy Markdown
Contributor

thanks @zhuqi-lucas

zhuqi-lucas added a commit to zhuqi-lucas/arrow-datafusion that referenced this pull request Jul 24, 2026
…rgeListView in RowsGroupColumn

Address kosiew's review on apache#23523: `RowsGroupColumn::supports_type`
accepts `ListView<Dictionary<..>>` and `LargeListView<Dictionary<..>>`,
but `encode_array_if_necessary` had no reconstruction arm for either
container. arrow-row's `decode_list_view` applies the dictionary-flatten
`corrected_type` step to the child, so the emitted array came back as
e.g. `ListView<Utf8>` and no longer matched `output_type`:

    SELECT arrow_cast(a, 'ListView(Dictionary(Int32, Utf8))') AS k, COUNT(*)
    FROM (VALUES (['a','b']), (['a','b']), (['c'])) AS t(a)
    GROUP BY k;
    -- expected ListView(Dictionary(Int32, Utf8)) but found ListView(Utf8)

Changes:

- `encode_array_if_necessary`: add `ListView` / `LargeListView` arms
  mirroring the `List` / `LargeList` ones, additionally carrying the
  `sizes` buffer that view-lists have.
- `contains_fsl_with_dictionary`: add the TODO kosiew requested —
  the guard works around apache/arrow-rs#10413, fixed upstream by
  apache/arrow-rs#10414 (merged 2026-07-24, not yet released as of
  arrow 59.1.0); remove the guard when DataFusion upgrades past that.
- `supports_type` doc: replace the now-inaccurate "round-trip cleanly"
  wording — the correction means these containers decode without
  panicking but with a *flattened* child type that `build` / `take_n`
  must re-encode.

Tests (all red without the fix, green with it):

- `build_preserves_list_view_of_dictionary_schema` — the reproducer
  shape, `build` path.
- `take_n_preserves_list_view_of_dictionary_schema` — `take_n` path,
  plus type of the remainder emitted by a subsequent `build`.
- `build_and_take_n_preserve_large_list_view_of_dictionary_schema` —
  same coverage for `LargeListView`.
- `list_view_of_dict_groups_by_logical_value` — group identity across
  distinct dictionary key mappings (3 input rows → 2 groups), matching
  the reproducer's GROUP BY semantics.
@Jefffrey Jefffrey added the bug label Jul 28, 2026
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 bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

arrow-row: decode_fixed_size_list panics on FixedSizeList<Dictionary> (missing corrected_type)

3 participants