zhuqi-lucas opened a new pull request, #10414:
URL: https://github.com/apache/arrow-rs/pull/10414
# 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>>`:
```text
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](https://github.com/apache/arrow-rs/blob/main/arrow-row/src/list.rs#L259-L320)),
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:
```rust
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 #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 https://github.com/apache/datafusion/pull/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.
--
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]
