Add special case in validate_struct_compatibility for empty structs

[pepijnve]

Which issue does this PR close?

• Closes Avro decoder fails to decode Iceberg manifest when data file has empty partition info #25022.

Rationale for this change

nested_struct::validate_struct_compatibility checks for at least one common field between the source and target field sets. This does not take into account that the source field set might be empty. This can cause issue during query planning.

For instance, when querying an Iceberg table's manifest with no partitioning information the following error is produced:

Execution error: Cannot cast column 'data_file' from 'Struct("content": non-null Int32, "file_path": non-null Utf8, "file_format": non-null Utf8, "partition": non-null Struct(), metadata: {"avro.name": "r102"}, "record_count": non-null Int64, "file_size_in_bytes": non-null Int64, "column_sizes": List(non-null Struct("key": non-null Int32, "value": non-null Int64), metadata: {"avro.name": "k117_v118"}), "value_counts": List(non-null Struct("key": non-null Int32, "value": non-null Int64), metadata: {"avro.name": "k119_v120"}), "null_value_counts": List(non-null Struct("key": non-null Int32, "value": non-null Int64), metadata: {"avro.name": "k121_v122"}), "nan_value_counts": List(non-null Struct("key": non-null Int32, "value": non-null Int64), metadata: {"avro.name": "k138_v139"}), "lower_bounds": List(non-null Struct("key": non-null Int32, "value": non-null Binary), metadata: {"avro.name": "k126_v127"}), "upper_bounds": List(non-null Struct("key": non-null Int32, "value": non-null Binary), metadata: {"avro.name": "k129_v130"}), "key_metadata": Binary, "split_offsets": List(non-null Int64), metadata: {"element-id": "133"}, "equality_ids": List(non-null Int32), metadata: {"element-id": "136"}, "sort_order_id": Int32, "referenced_data_file": Utf8)' (physical data type) to 'Struct("content": non-null Int32, "file_path": non-null Utf8, "file_format": non-null Utf8, "partition": non-null Struct(), "record_count": non-null Int64, "file_size_in_bytes": non-null Int64, "column_sizes": List(non-null Struct("key": non-null Int32, "value": non-null Int64)), "value_counts": List(non-null Struct("key": non-null Int32, "value": non-null Int64)), "null_value_counts": List(non-null Struct("key": non-null Int32, "value": non-null Int64)), "nan_value_counts": List(non-null Struct("key": non-null Int32, "value": non-null Int64)), "lower_bounds": List(non-null Struct("key": non-null Int32, "value": non-null Binary)), "upper_bounds": List(non-null Struct("key": non-null Int32, "value": non-null Binary)), "key_metadata": Binary, "split_offsets": List(non-null Int64), "equality_ids": List(non-null Int32), "sort_order_id": Int32, "referenced_data_file": Utf8)' (logical data type): Error during planning: Cannot cast struct with 0 fields to 0 fields because there is no field name overlap

What changes are included in this PR?

• Skip the field overlap check when source_fields.is_empty()

What is the testing strategy for this PR?

• Added unit tests
• Verified SLTs still pass

Are there any user-facing changes?

No

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.72%. Comparing base (46bbf0d) to head (4a370e1).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #25023      +/-   ##
==========================================
+ Coverage   81.70%   81.72%   +0.02%     
==========================================
  Files        1127     1127              
  Lines      416001   416395     +394     
  Branches   416001   416395     +394     
==========================================
+ Hits       339902   340312     +410     
+ Misses      56125    56091      -34     
- Partials    19974    19992      +18     

☔ 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.

[kumarUjjawal]

Thank you @pepijnve for the fix. I left a comment.

Also The documentation at [nested_struct.rs line 45](

datafusion/datafusion/common/src/nested_struct.rs

Line 45 in 96e2e94

/// - **No Positional Mapping**: Structs with no overlapping field names are rejected

) is no longer accurate. It says structs without overlapping field names are rejected, but this change allows an empty source struct to cast to an empty or fully nullable target struct. Could we update this sentence to document the empty source exception?

[kumarUjjawal]

This test only verifies validation. After validation succeeds, cast_struct_column still calls StructArray::try_new with empty fields and arrays, which Arrow rejects because it cannot infer the row count. I reproduced this with a two row StructArray::new_empty_fields(2, None).

Please use try_new_with_length or new_empty_fields and add a cast_column regression that verifies the output length and null validity. Otherwise, the reported Iceberg query moves past planning but still fails during execution.

[pepijnve]

Thanks for catching this. I had fixed the validation, then hit the arrow-rs error in the Avro decoder and didn't think of checking the cast implementation itself 🤦‍♂️
I adjusted the cast implementation and added tests covering the change.

[pepijnve]

The documentation .. is no longer accurate. It says structs without overlapping field names are rejected, but this change allows an empty source struct to cast to an empty or fully nullable target struct. Could we update this sentence to document the empty source exception?

I reviewed the documentation and I'm now wondering if this function should actually be even more flexible. It states

structs should always be allowed to cast to other structs

but then requires at least one overlapping field by name. Should we just drop the overlap check and let the nullability check that comes after it deal with the case where mapping is not possible instead?

[kumarUjjawal]

The documentation .. is no longer accurate. It says structs without overlapping field names are rejected, but this change allows an empty source struct to cast to an empty or fully nullable target struct. Could we update this sentence to document the empty source exception?

I reviewed the documentation and I'm now wondering if this function should actually be even more flexible. It states

structs should always be allowed to cast to other structs

but then requires at least one overlapping field by name. Should we just drop the overlap check and let the nullability check that comes after it deal with the case where mapping is not possible instead?

I think it was introduced in PR #19955 and was DataFusion 53 breaking change because unrelated structs could otherwise silently lose data. For example, casting {left: 1} to {alpha: INT NULL} would discard left and produce {alpha: NULL}. The nullability check only protects nonnullable target fields, so it cannot replace the overlap check.

The empty source case is different because there are no source fields to discard. I suggest keeping the current exception and updating the documentation around nested_struct.rs lines 39 to 45 to say that a nonempty source requires at least one overlapping field name, while an empty source can cast to an empty or fully nullable target.

[pepijnve]

I think it was introduced in PR #19955 and was DataFusion 53 breaking change because unrelated structs could otherwise silently lose data. For example, casting {left: 1} to {alpha: INT NULL} would discard left and produce {alpha: NULL}. The nullability check only protects nonnullable target fields, so it cannot replace the overlap check.

Thanks for the pointer to the PR. The comment made by @adriangb that this matches DuckDB behaviour makes sense, and as you said, coming from an empty struct there's no information to lose anyway. I'll have a go at revising the doc comment.

[pepijnve]

I've updated the documentation a bit and removed the duplication between the validation and cast functions.

[kumarUjjawal]

Thank you @pepijnve

[pepijnve]

Clippy warning fixed.