fix(arrow): promote FixedSizeList file columns to List by Baunsgaard · Pull Request #3629 · apache/iceberg-rust

Baunsgaard · 2026-11-04T23:23:04Z

Which issue does this PR close?

Closes FixedSizeList columns fail to read with "cannot promote FixedSizeList to List" #3471.

What changes are included in this PR?

PromotePlan now converts a file-side Arrow FixedSizeList to Iceberg List, the same way it already converts LargeList. parquet-rs can surface a 3-level Parquet LIST as FixedSizeList from the embedded Arrow schema; Iceberg has no fixed-size list type, so those scans used to fail with cannot promote.

Null rows stay null with no children (pads from parquet-rs are dropped; Java OptionReader does the same). Arrow cast would keep the pads and fail ListArray::try_new when the element is required.

This is reader encoding, not a new table type and not v4 vector (apache/iceberg#18297). FixedSizeList vs a non-list target is now cannot promote at plan time. That used to fall through to Cast, which unwrapped size-1 lists. A field-id table cannot pair list with primitive; a bad name mapping can.

Not in this PR: #3467 (list-view hints), vector type.

Are these changes tested?

Unit tests in record_batch_transformer.rs:

FixedSizeList<Int32> -> List<Int64> with a null parent (offsets [0, 2, 2]) and child nulls
null parent + required element (does not fail ListArray::try_new)
all-non-null path through RecordBatchTransformer
FixedSizeList -> Int32 rejected at plan time, including size 1

The bug is in PromotePlan after parquet-rs has already built a FixedSizeListArray, so the tests construct that array directly. A Parquet round-trip is omitted to keep the PR small.

AI Disclosure

AI (Cursor) was used to implement the change, write tests, and draft this description. I reviewed the spec (list vs fixed(L)), the LargeList path, and the transformer tests.

Copilot

🟡 Changes recommended

FixedSizeList conversion can panic when retained children exceed the i32 List offset range in either branch, rather than returning a conversion error.

1 open issue

Reject FixedSizeList casts exceeding the List offset range · New

What changed in this PR

This PR addresses #3471 by allowing the Arrow reader’s promotion layer to convert file-side FixedSizeList columns into Iceberg List columns without introducing a new table type.

Changes:

Adds conversion that preserves null lists and removes their padded child values.
Rejects incompatible FixedSizeList promotions at planning time.
Adds tests for child-type promotion, required elements, null rows, and transformer integration.

File	Description
`crates/iceberg/src/arrow/record_batch_transformer.rs`	Adds fixed-size list promotion, compatibility checks, and regression tests.

_{Review effort: Balanced}

Baunsgaard · 2026-11-06T16:58:41Z

+    let Some(nulls) = fixed_size_list.nulls().filter(|n| n.null_count() > 0) else {
+        return Ok(cast(
+            fixed_size_list,
+            &DataType::List(Arc::clone(element_field)),
+        )?);
+    };
+    let size = fixed_size_list.value_length() as usize;


laskoviymishka

As I read the reader path, this gets the #3471 scan past cannot promote.

The offset-overflow panic in list_from_fixed_size_list is what I'm requesting changes for; with a guard before both branches I'm OK with this.

Could you also reword "Java OptionReader does the same" in the description? Java reads a null list from its definition level and never has pads to drop, so only the result matches.

@mbutrovich, could you weigh in on whether PromotePlan should accept FixedSizeList at all before this goes in?

Once panic fixed - it's good to me, but i'll wait sign-up from @mbutrovich to actually merge.

Thanks for a PR!

laskoviymishka · 2026-11-06T08:38:38Z

+    };
+    let size = fixed_size_list.value_length() as usize;
+    let offsets =
+        OffsetBuffer::<i32>::from_lengths(nulls.iter().map(|valid| if valid { size } else { 0 }));


Could we check the retained child count against i32::MAX before the let Some(nulls) split and return an error instead? A batch past that panics in either branch: from_lengths here, and From<FixedSizeListArray> behind the cast path, both via expect("offset overflow"). On main this column was a cannot promote error, and the LargeList arm goes through cast_list, which returns ComputeError. One check on (len() - null_count()) * value_length() covers both branches.

Done. Checking (len - null_count) * value_length against i32::MAX before the nulls split, so both the cast path and from_lengths return Err instead of panicking.

laskoviymishka · 2026-11-06T08:38:38Z

    }

+    #[test]
+    fn promote_fixed_size_list_file_column_to_list() {


Could we add one test that writes a Parquet file with a FixedSizeList column and a null row, then reads it back through ArrowReader? All three tests build the FixedSizeListArray by hand, so nothing exercises the #3471 scan itself or the parquet-rs null-row padding the doc comment relies on. The fixed_size_list_column test in #3471 looks close to reusable, ideally with one nullable and one required element.

Done. Added test_read_fixed_size_list_column_as_list in projection.rs: writes a FixedSizeList with a null row, reads through ArrowReader, and covers both optional and required elements.

Baunsgaard

Addressed the overflow guard and added the Parquet round-trip with a null FixedSizeList row (optional + required elements). Updated the description so the Java OptionReader note is result-only, not pad-dropping.

Happy to wait on @mbutrovich for the PromotePlan review.

Baunsgaard · 2026-11-06T16:57:59Z

+    };
+    let size = fixed_size_list.value_length() as usize;
+    let offsets =
+        OffsetBuffer::<i32>::from_lengths(nulls.iter().map(|valid| if valid { size } else { 0 }));


Done. Checking (len - null_count) * value_length against i32::MAX before the nulls split, so both the cast path and from_lengths return Err instead of panicking.

Baunsgaard · 2026-11-06T16:58:23Z

    }

+    #[test]
+    fn promote_fixed_size_list_file_column_to_list() {


Done. Added test_read_fixed_size_list_column_as_list in projection.rs: writes a FixedSizeList with a null row, reads through ArrowReader, and covers both optional and required elements.

Baunsgaard · 2026-11-06T16:58:41Z

+    let Some(nulls) = fixed_size_list.nulls().filter(|n| n.null_count() > 0) else {
+        return Ok(cast(
+            fixed_size_list,
+            &DataType::List(Arc::clone(element_field)),
+        )?);
+    };
+    let size = fixed_size_list.value_length() as usize;


mbutrovich

Thanks @Baunsgaard for picking up #3471 and for adding the Parquet round trip.

mbutrovich · 2026-11-06T18:49:58Z

+            // File Arrow hints may be List, LargeList, or FixedSizeList. Iceberg has no
+            // fixed-size list; tables are always List (`ToArrowSchemaConverter::list`).
            (
-                DataType::List(source_field) | DataType::LargeList(source_field),
+                DataType::List(source_field)
+                | DataType::LargeList(source_field)
+                | DataType::FixedSizeList(source_field, _),


@laskoviymishka asked whether PromotePlan should accept FixedSizeList at all. I don't think it should. Could we rewrite FixedSizeList to List in the Arrow schema hint instead, the way #3467 does for list views, and drop the PromotePlan arm and list_from_fixed_size_list?

parquet-rs takes the list type from the hint, so a List hint reads the same Parquet LIST through its list reader. I prototyped this as a rewrite at the top of configure_arrow_reader_metadata, before name mapping and fallback IDs are applied, which is where #3467 puts coerce_list_views. I replaced the new FixedSizeList arm in apply with a panic, and test_read_fixed_size_list_column_as_list still passes. A pyarrow-written file without field IDs also reads correctly through both a name mapping and fallback IDs, including FixedSizeList columns nested in a struct and as a map value. So the transformer never sees a FixedSizeList.

That removes three things this PR has to carry. For a batch with a null row, parquet-rs copies the decoded values into a padded child (fixed_size_list_array.rs), and list_from_fixed_size_list then copies them again with filter to drop the pads. With the hint, the list reader uses the decoded values without either copy. The PR would also stop depending on parquet-rs padding null rows, which the doc comment and the filter assume. Finally, the overflow guard isn't needed, because the list reader already returns an error when the child length doesn't fit i32 offsets (list_array.rs).

It also keeps the normalization of file-side Arrow list types in one place. coerce_list_views in #3467 already recurses through FixedSizeList, so this becomes one changed arm there, DataType::FixedSizeList(element, _) => DataType::List(coerce_list_views_or_share(element)), plus a rename. Could we build on #3467 here?

Done, I think it makes more sense to do it this way.

Dropped the PromotePlan arm and list_from_fixed_size_list. coerce_fixed_size_lists rewrites FixedSizeList to List at the top of configure_arrow_reader_metadata, before name mapping and fallback IDs, same placement as #3467.

#3467 is still open, so this PR contains the FixedSizeList rewrite on its own; when list views land, the FixedSizeList arm can fold into that helper.

mbutrovich · 2026-11-06T18:50:49Z

+    /// says so. Iceberg has no such type; the table column is a plain list. Also covers a null
+    /// parent (parquet-rs pads `size` children) with both optional and required elements.
+    #[tokio::test]
+    async fn test_read_fixed_size_list_column_as_list() {


Could this test also cover files without field IDs? Those are where these columns come from in practice. Iceberg's own writers don't produce them: iceberg-rust's ParquetWriter builds its Arrow schema from the Iceberg schema (parquet_writer.rs), and PyIceberg's append casts a FixedSizeListArray to a plain list before writing (pyarrow.py). They get in when existing files are added without a rewrite. PyIceberg's add_files reads the file's Arrow schema, which comes from ARROW:schema (pyarrow.py), accepts FixedSizeListType as a list (pyarrow.py), and sets a default name mapping when the table has none (__init__.py). A pyarrow-written file has no field IDs, so the reader resolves it through that name mapping.

This test writes field IDs at every level, so it only covers the less common shape. These cases would cover the import path:

A file without field IDs, read through a name mapping, with a FixedSizeList column at the top level, inside a struct, as a map value, as a list element, and as the element of another FixedSizeList. Give each a null row, and give the top-level column a required element.

The same file read without a name mapping, so the reader assigns fallback IDs to the top-level columns.

ArrowWriter writes this shape when the Arrow fields carry no PARQUET:field_id metadata, as in test_read_parquet_with_name_mapping_maps_nested_fields, so the test doesn't need pyarrow. At this PR's head, the top-level, struct, and map cases read correctly through both paths in a local test with a pyarrow 25 file. These tests would guard whichever approach lands.

Done. Added test_read_fixed_size_list_without_field_ids: no field IDs, name mapping and fallback IDs, null rows, required top-level element, plus FixedSizeList inside a struct, as a map value, as a list element, and nested FixedSizeList.

parquet-rs picks its list reader from the embedded Arrow schema. Iceberg has no fixed-size list type, so coerce FixedSizeList hints to List before name mapping and fallback IDs. The 3-level LIST reader then handles null rows without PromotePlan conversion or pad filtering.

Baunsgaard

Reworked per the schema-hint approach: FixedSizeList is rewritten to List before the stream reader is built, so PromotePlan never sees it.

Also updated the description and tests for the no-field-ID import path.

Baunsgaard · 2026-11-06T20:33:26Z

+            // File Arrow hints may be List, LargeList, or FixedSizeList. Iceberg has no
+            // fixed-size list; tables are always List (`ToArrowSchemaConverter::list`).
            (
-                DataType::List(source_field) | DataType::LargeList(source_field),
+                DataType::List(source_field)
+                | DataType::LargeList(source_field)
+                | DataType::FixedSizeList(source_field, _),


Done, I think it makes more sense to do it this way.

Dropped the PromotePlan arm and list_from_fixed_size_list. coerce_fixed_size_lists rewrites FixedSizeList to List at the top of configure_arrow_reader_metadata, before name mapping and fallback IDs, same placement as #3467.

#3467 is still open, so this PR contains the FixedSizeList rewrite on its own; when list views land, the FixedSizeList arm can fold into that helper.

Baunsgaard · 2026-11-06T20:34:00Z

+    /// says so. Iceberg has no such type; the table column is a plain list. Also covers a null
+    /// parent (parquet-rs pads `size` children) with both optional and required elements.
+    #[tokio::test]
+    async fn test_read_fixed_size_list_column_as_list() {


Done. Added test_read_fixed_size_list_without_field_ids: no field IDs, name mapping and fallback IDs, null rows, required top-level element, plus FixedSizeList inside a struct, as a map value, as a list element, and nested FixedSizeList.

laskoviymishka

The overflow panic went away with list_from_fixed_size_list, and the ArrowReader round-trip is in, thanks. Could you rewrite the PR description for the schema-hint approach and drop the Java OptionReader line? It still describes the PromotePlan conversion, and the squash commit comes from it.

From my side this is good to go, but I'll wait for @mbutrovich before merge. Matt, could you look at the rework and decide whether it should wait on #3467?

Please check and address all review comments in this PR.

laskoviymishka · 2026-11-08T22:25:50Z

+/// reader is built. That uses the 3-level LIST reader (no null-row padding, no
+/// `PromotePlan` conversion). Returns `None` when the schema has no `FixedSizeList`.
+/// Unchanged fields are shared; nothing is allocated before the first change.
+///
+/// Same placement as the list-view rewrite in #3467: call this before name mapping and
+/// fallback IDs so nested IDs land on the rewritten `List` fields.


Could the doc stick to what the function does? "no null-row padding, no PromotePlan conversion" describes the earlier version of this PR, and #3467 is still open, so the placement note points at code that isn't on main. The pipeline.rs comment has the same #3467 reference.

Suggested change

/// reader is built. That uses the 3-level LIST reader (no null-row padding, no

/// `PromotePlan` conversion). Returns `None` when the schema has no `FixedSizeList`.

/// Unchanged fields are shared; nothing is allocated before the first change.

///

/// Same placement as the list-view rewrite in #3467: call this before name mapping and

/// fallback IDs so nested IDs land on the rewritten `List` fields.

/// reader is built. Returns `None` when the schema has no `FixedSizeList`.

/// Unchanged fields are shared; nothing is allocated before the first change.

///

/// Call this before name mapping and fallback IDs so nested IDs land on the rewritten

/// `List` fields.

laskoviymishka · 2026-11-08T22:25:50Z

    }
+
+    #[test]
+    fn test_coerce_fixed_size_lists_rewrites_nested() {


Could this build the expected schema and assert equality with it? With PARQUET:field_id metadata and a non-nullable element on the input fields, it would pin what name mapping and fallback IDs rely on. Right now it only checks matches!(…, DataType::List(_)), so a rewrite that dropped either would still pass. An Arc::ptr_eq on plain would cover the sharing the doc promises.

laskoviymishka · 2026-11-08T22:25:50Z

+                &Int32Array::from(vec![1, 2, 5, 6]) as &dyn Array
+            );
+            let schema = out.schema();
+            for name in ["s", "m", "list_of_fsl", "nested_fsl"] {


Could assert_top_list compare s, m, list_of_fsl and nested_fsl against expected arrays, like top? The "not FixedSizeList" check can't fail for s and m. With a debug print added, the fallback pass reads s.inner as [null, null, null], since id-less struct children read as absent, and the test still passes. Expecting that null under fallback and the values under name mapping would pin both.

Copilot AI balanced review requested due to automatic review settings November 4, 2026 23:23

Copilot started reviewing on behalf of Baunsgaard November 4, 2026 23:23

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

laskoviymishka requested changes Nov 6, 2026

View reviewed changes

Baunsgaard force-pushed the fix/fixed-size-list-promote branch from 4e3f106 to 6bbf39a Compare November 6, 2026 16:56

Baunsgaard commented Nov 6, 2026

View reviewed changes

mbutrovich self-requested a review November 6, 2026 17:15

mbutrovich suggested changes Nov 6, 2026

View reviewed changes

Baunsgaard commented Nov 6, 2026

View reviewed changes

Baunsgaard force-pushed the fix/fixed-size-list-promote branch from 6bbf39a to aa525f1 Compare November 6, 2026 20:35

Baunsgaard requested review from laskoviymishka and mbutrovich November 6, 2026 20:36

laskoviymishka approved these changes Nov 8, 2026

View reviewed changes

Conversation

Baunsgaard commented Nov 4, 2026

Which issue does this PR close?

What changes are included in this PR?

Are these changes tested?

AI Disclosure

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laskoviymishka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Baunsgaard left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbutrovich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Baunsgaard Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Baunsgaard left a comment

Choose a reason for hiding this comment

Uh oh!

Baunsgaard Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laskoviymishka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Baunsgaard Nov 6, 2026 •

edited

Loading

Baunsgaard Nov 6, 2026 •

edited

Loading