Skip to content

Add FixedSizeList support for recursive struct schema adaptation - #22980

Merged
kosiew merged 17 commits into
apache:mainfrom
kosiew:schema-evolution-02-20835
Jul 25, 2026
Merged

Add FixedSizeList support for recursive struct schema adaptation#22980
kosiew merged 17 commits into
apache:mainfrom
kosiew:schema-evolution-02-20835

Conversation

@kosiew

@kosiew kosiew commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

FixedSizeList containing Struct values was not handled by the existing recursive nested adaptation logic used for schema evolution. As a result, planner-time compatibility checks, nested cast detection, and runtime casting did not support additive struct evolution within FixedSizeList containers.

This change adds FixedSizeList support and verifies planner/runtime parity so that planning allows exactly the cases runtime can adapt while continuing to reject incompatible schema changes.

What changes are included in this PR?

  • Extend cast_column to support recursive casting of FixedSizeList values when source and target list sizes match.

  • Add FixedSizeList handling to:

    • requires_nested_struct_cast
    • validate_data_type_compatibility
  • Implement recursive casting of nested Struct values contained in FixedSizeList.

  • Preserve planner/runtime parity by validating child type compatibility before runtime fallback logic is applied.

  • Add handling for null-parent FixedSizeList entries by masking hidden child values before retrying casts, avoiding failures caused by semantically inaccessible child data.

  • Refactor list and list-view casting helpers to use Arrow AsArray accessors.

Are these changes tested?

Yes.

The following tests were added:

  • test_cast_fixed_size_list_struct
  • test_validate_fixed_size_list_struct_compatibility
  • test_validate_fixed_size_list_struct_missing_non_nullable_field_rejected
  • test_validate_fixed_size_list_struct_size_mismatch_rejected
  • test_cast_fixed_size_list_struct_all_null
  • test_fixed_size_list_struct_planner_runtime_parity_on_incompatible_type
  • test_cast_fixed_size_list_struct_missing_non_nullable_field_runtime_rejected
  • test_cast_fixed_size_list_struct_ignores_hidden_child_values_for_null_parent

Existing coverage in test_requires_nested_struct_cast was also extended to include FixedSizeList cases.

These tests cover:

  • Additive nullable nested-field evolution
  • All-null and partially null list cases
  • Incompatible nested type changes
  • Non-nullable field addition rejection
  • Planner/runtime parity validation

Are there any user-facing changes?

No user-facing changes. This is an internal enhancement to nested schema adaptation and casting behavior for FixedSizeList<Struct> types.

LLM-generated code disclosure

This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed.

kosiew added 3 commits June 16, 2026 19:41
…untime parity

- Updated runtime casting to recognize FixedSizeList when source and target list sizes match, allowing recursive adaptation of the child struct via a new helper.
- Extended planner/runtime parity by updating `validate_data_type_compatibility` and `requires_nested_struct_cast` to handle equal-size FixedSizeList children.

test: add regression coverage for nested field changes

- Added tests for:
- Additive nullable nested-field evolution
- Planner acceptance for compatible cases
- Non-nullable added-field rejection
- All-null column handling
- Planner/runtime parity on incompatible nested type changes
- Introduced `downcast_array!` macro to simplify downcasting of array types and improve error handling with descriptive messages.
- Refactored existing functions `cast_list_column`, `cast_list_view_column`, and `cast_fixed_size_list_column` to utilize the new macro for improved readability and maintainability.
- Added helper function `create_fsl_test_fields` to streamline test field creation for fixed-size list struct tests.
- Updated related tests to use the new helper function for improved clarity and reduced redundancy.
- Prevent validation from accepting unsupported length changes
- Ensure planner behavior aligns with runtime casting
- Add regression test for rejected mismatch case
@github-actions github-actions Bot added the common Related to common crate label Jun 16, 2026
kosiew added 3 commits June 16, 2026 19:57
- Added regression test for null FixedSizeList parent hiding invalid child values.
- Implemented runtime retry mechanism for child slots masked as null when cast fails due to hidden null-parent values.
- Added runtime parity test to ensure missing non-nullable nested fields are still rejected.
- Removed redundant FixedSizeList size-mismatch match arm
- Bound repeated source_values and target_type
- Renamed masked fallback local for better readability
- Inlined single-use mask wrapper
- Added test helpers to reduce duplication in tests
- Reused fields in parity test for efficiency
- Reused s1 and s2 in requires_nested_struct_cast test to minimize duplication
@kosiew
kosiew marked this pull request as ready for review June 16, 2026 15:27
Comment thread datafusion/common/src/nested_struct.rs Outdated
Comment thread datafusion/common/src/nested_struct.rs Outdated
let source_values = source_list.values();
let target_type = target_inner_field.data_type();

validate_data_type_compatibility(

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 other cast_*_column methods don't do this validate call, is this significant?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This fixed-size-list path differs from the list/list-view paths because it has a retry that masks child values hidden behind null parent list slots. The schema compatibility check ensures that retry is only used for value-level failures in hidden child positions, not to accidentally relax planner/runtime type compatibility. The parity tests for incompatible nested type and missing non-nullable fields cover this. I’ll add a comment or restructure the code so this guard is explicit.

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.

I still don't think we need to compatibility check again here on this path, as we're just meant to be casting at this point 🤔

@kosiew kosiew Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Amended:

  • Removed fixed-size-list compatibility check from the normal cast path.
  • Moved compatibility guard into the null-parent retry path only.
  • Adjusted cast_struct_column all-null fast path to follow struct compatibility validation, ensuring all-null structs reject missing non-nullable fields.

Comment thread datafusion/common/src/nested_struct.rs Outdated
Comment on lines +293 to +298
let cast_values = match cast_column(source_values, target_type, cast_options) {
Ok(cast_values) => cast_values,
Err(error) => match source_list.nulls() {
Some(parent_nulls) if parent_nulls.null_count() > 0 => {
let hidden_child_nulls = parent_nulls.expand(target_list_size as usize);
let masked_values =

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.

I dont understand whats happening here; why do we have this fallback if we fail to cast?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The fallback is intentional. FixedSizeListArray stores len * list_size child values even for null parent list entries, so recursive casting can fail on child values that are semantically hidden by a null parent. The fallback expands the parent null bitmap to child positions, masks those hidden child values to null, and retries; visible child values are still cast normally. test_cast_fixed_size_list_struct_ignores_hidden_child_values_for_null_parent covers this case. I’ll add a targeted comment and keep the fallback limited to this null-parent masking case.

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.

This is interesting; it technically applies for the other list methods too but those would be rare cases where the null slots are non-empty (and have values that would error on cast), not to mention this would only occur when cast options has safe=false

Also I think this can also occur in the arrow-rs upstream cast code, so might need to raise an issue there too 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

kosiew added 3 commits June 18, 2026 14:08
…_array!

- Removed local `downcast_array!` implementation
- Used Arrow methods:
- `source_col.as_list::<O>()`
- `source_col.as_list_view::<O>()`
- `source_col.as_fixed_size_list()`
- Added `AsArray` import
- Added comment before `validate_data_type_compatibility` to explain planner/runtime guard.
- Restructured fallback into an explicit helper:
- Only attempts masked retry when parent nulls are present.
- Returns original cast error in all other cases.
- Clarified the split between guard and fallback logic.
… behavior

- Explain hidden child slots handling for null parents
- Clarify that retries occur only for parent-null masking
- State that behavior remains unchanged for scenarios without parent nulls, retaining original cast error
…ing logic

- Removed fixed-size-list compatibility check from the normal cast path.
- Moved compatibility guard into the null-parent retry path only.
- Adjusted cast_struct_column all-null fast path to follow struct compatibility validation, ensuring all-null structs reject missing non-nullable fields.
@TheBuilderJR

Copy link
Copy Markdown
Contributor

Hey @alamb @Jefffrey, could we get this merged? This is still affecting my users in a pretty pathological way. Right now I have to rewrite the data just to make the query work.

@alamb

alamb commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Hi @TheBuilderJR -- I am confused about what this PR is fixing / adding -- it clearly has a bunch of unit tests, but I am missing the bigger picture -- what type of plan / SQL / DataFrame are you running on what type of input that is not working?

I think it would help to have a fuller end to end test of what you are doing (e.g. is it reading some nested structure from parquet files and they are getting confused)

Otherwise I worry that we won't be able ot maintain this code as it is refactored over time as if we change how it is structured we won't know if we broke the original usecase

@codecov-commenter

codecov-commenter commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.35979% with 619 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.72%. Comparing base (c118002) to head (39ce782).
⚠️ Report is 31 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/proto/src/physical_plan/mod.rs 9.54% 218 Missing ⚠️
...ysical-plan/src/aggregates/ordered_final_stream.rs 75.93% 82 Missing and 8 partials ⚠️
...ion/physical-plan/src/joins/symmetric_hash_join.rs 82.84% 18 Missing and 23 partials ⚠️
datafusion/physical-plan/src/joins/cross_join.rs 0.00% 34 Missing ⚠️
datafusion/physical-plan/src/repartition/mod.rs 77.03% 14 Missing and 17 partials ⚠️
datafusion/session/src/catalog.rs 56.52% 20 Missing ⚠️
datafusion/physical-plan/src/sorts/sort.rs 77.92% 6 Missing and 11 partials ⚠️
datafusion/session/src/table.rs 85.47% 17 Missing ⚠️
datafusion/physical-plan/src/joins/proto.rs 85.32% 7 Missing and 9 partials ⚠️
...ical-plan/src/aggregates/ordered_partial_stream.rs 73.68% 10 Missing and 5 partials ⚠️
... and 27 more
Additional details and impacted files
@@            Coverage Diff            @@
##             main   #22980     +/-   ##
=========================================
  Coverage   80.72%   80.72%             
=========================================
  Files        1089     1090      +1     
  Lines      368911   370385   +1474     
  Branches   368911   370385   +1474     
=========================================
+ Hits       297785   298982   +1197     
- Misses      53374    53598    +224     
- Partials    17752    17805     +53     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TheBuilderJR

Copy link
Copy Markdown
Contributor

@alamb my high level goal is to get #20835 fixed. @kosiew can you advise here?

Comment thread datafusion/common/src/nested_struct.rs Outdated
// Before masking and retrying, guard schema compatibility so the retry only
// handles value-level failures in hidden child slots and cannot make runtime
// accept schemas that planning rejects.
if let Err(error) = validate_data_type_compatibility(

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.

i still dont agree with having this validate call on the cast path

we dont do anything similar for other list types even though they could technically contain invalid data (that cant be casted) in a non-empty null slot

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@kosiew
kosiew marked this pull request as draft July 24, 2026 08:20
kosiew added 6 commits July 24, 2026 16:21
…bility validation and field_name plumbing

- Removed retry-path compatibility validation.
- Eliminated field_name plumbing.
- Retained recursive cast enforcement.
- Implemented old/new schema evolution via ListingTable
- Added support for SELECT *
- Enabled nested-field projection
- Implemented rejection for missing non-nullable fields
- Added rejection for incompatible fields
- Corrected existing negative fixture to truly omit chain
- Added constructor-error regression test.
- Extended hidden-child regression to utilize sliced FixedSizeList input.
test: enhance size-mismatch test to verify compatibility validation rejection, runtime cast_column rejection, and width-specific Arrow diagnostic
@github-actions github-actions Bot added the core Core DataFusion crate label Jul 24, 2026
@kosiew

kosiew commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

@alamb

I think it would help to have a fuller end to end test of what you are doing (e.g. is it reading some nested structure from parquet files and they are getting confused)

Added e2e tests.

@kosiew
kosiew marked this pull request as ready for review July 24, 2026 09:11
@kosiew

kosiew commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

thanks @Jefffrey, @alamb for your reviews and feedback

@kosiew
kosiew added this pull request to the merge queue Jul 25, 2026
Merged via the queue into apache:main with commit f1ab86d Jul 25, 2026
37 checks passed
@kosiew
kosiew deleted the schema-evolution-02-20835 branch July 25, 2026 10:52
kosiew added a commit to kosiew/datafusion that referenced this pull request Aug 12, 2026
…che#22980)

## Which issue does this PR close?

* Part of apache#20835

## Rationale for this change

`FixedSizeList` containing `Struct` values was not handled by the
existing recursive nested adaptation logic used for schema evolution. As
a result, planner-time compatibility checks, nested cast detection, and
runtime casting did not support additive struct evolution within
`FixedSizeList` containers.

This change adds `FixedSizeList` support and verifies planner/runtime
parity so that planning allows exactly the cases runtime can adapt while
continuing to reject incompatible schema changes.

## What changes are included in this PR?

* Extend `cast_column` to support recursive casting of `FixedSizeList`
values when source and target list sizes match.
* Add `FixedSizeList` handling to:

  * `requires_nested_struct_cast`
  * `validate_data_type_compatibility`
* Implement recursive casting of nested `Struct` values contained in
`FixedSizeList`.
* Preserve planner/runtime parity by validating child type compatibility
before runtime fallback logic is applied.
* Add handling for null-parent `FixedSizeList` entries by masking hidden
child values before retrying casts, avoiding failures caused by
semantically inaccessible child data.
* Refactor list and list-view casting helpers to use Arrow `AsArray`
accessors.

## Are these changes tested?

Yes.

The following tests were added:

* `test_cast_fixed_size_list_struct`
* `test_validate_fixed_size_list_struct_compatibility`
*
`test_validate_fixed_size_list_struct_missing_non_nullable_field_rejected`
* `test_validate_fixed_size_list_struct_size_mismatch_rejected`
* `test_cast_fixed_size_list_struct_all_null`
*
`test_fixed_size_list_struct_planner_runtime_parity_on_incompatible_type`
*
`test_cast_fixed_size_list_struct_missing_non_nullable_field_runtime_rejected`
*
`test_cast_fixed_size_list_struct_ignores_hidden_child_values_for_null_parent`

Existing coverage in `test_requires_nested_struct_cast` was also
extended to include `FixedSizeList` cases.

These tests cover:

* Additive nullable nested-field evolution
* All-null and partially null list cases
* Incompatible nested type changes
* Non-nullable field addition rejection
* Planner/runtime parity validation

## Are there any user-facing changes?

No user-facing changes. This is an internal enhancement to nested schema
adaptation and casting behavior for `FixedSizeList<Struct>` types.

## LLM-generated code disclosure

This PR includes LLM-generated code and comments. All LLM-generated
content has been manually reviewed.

---------

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants