Add nested struct schema evolution support for Map types - #23914
Conversation
- Adapt recursive Map key/value structures. - Implement shared planner/runtime validation. - Preserve offsets and nulls; require matching sorted flags. - Enforce Arrow Map entry/key invariants. - Handle nullable additions, extras, null/all-null maps, and incompatible/non-nullable rejections. - Add Parquet end-to-end regression tests.
…etadata internally - Updated cast_map_column to accept &MapArray and reduce redundant arguments - Extracted a shared error assertion helper for planner/runtime components
- Map key/value children now matched positionally while preserving target technical names. - Implemented safe key evolution with no source key-field removal, injective primitive widening, nullable additions, and unchanged key types for sorted Maps. - Compacted hidden null-parents and sliced unreachable entries before casting. - Updated public rustdoc to document Map evolution semantics. - Expanded Parquet test coverage for differing physical/logical entry names. - Added planner/runtime regressions tests for uniqueness, sortedness, nulls, slices, and unsafe casts.
- Allow sorted targets to be set to false. - Reject transitioning from sorted=false to sorted=true. - Maintain requirement for unchanged key types in sorted targets. - Allow downgraded unsorted targets to utilize existing safe key evolution. - Update public Rust documentation. - Add planner/runtime parity and value/key assertions.
- Reused validated target Map children and eliminated duplicate validation and map_entry_fields. - Implemented common planner/runtime error assertion helper. - Removed redundant sortedness assertions that are already covered by exact type equality. - Public APIs remain unchanged.
…structs at planner boundary - Added regression test for empty key struct to ensure nullable-only target key field is rejected in both planner and runtime.
…chema helpers - Added `test_map_value_struct_incompatible_schema_evolution_rejected` for end-to-end negative testing of Parquet. - Refactored shared map fixture and schema helper functions for improved clarity and reusability.
- Removed unused `let _ =` in nested_struct.rs - Added local name lookup/set for map key struct validation in nested_struct.rs - Extracted shared map E2E setup helper in expr_adapter.rs - Added nullable/non-nullable schema wrapper helpers, avoiding bool at call sites in expr_adapter.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #23914 +/- ##
==========================================
+ Coverage 80.65% 80.69% +0.03%
==========================================
Files 1091 1094 +3
Lines 371031 372360 +1329
Branches 371031 372360 +1329
==========================================
+ Hits 299256 300467 +1211
- Misses 53935 53985 +50
- Partials 17840 17908 +68 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Updated the sorted to unsorted test to enforce rejection. - Clarified documentation for requires_nested_struct_cast Map behavior. - Extracted compact_map_entries() with explanation for identity take. - Extracted is_injective_map_key_cast() and added documentation for conservative policy.
… cast_map_column and rewording to “specialized Map casting path”
|
@TheBuilderJR |
|
yep lgtm. thanks @kosiew |
adriangb
left a comment
There was a problem hiding this comment.
Overall looks great!
I think this requires an upgrade guide entry since it's a change in behavior.
Could we add / move some tests to SLTs in https://github.com/apache/datafusion/blob/main/datafusion/sqllogictest/test_files/map.slt
| (DataType::Map(s, source_sorted), DataType::Map(t, target_sorted)) => { | ||
| validate_map_compatibility(s, *source_sorted, t, *target_sorted)?; | ||
| } |
There was a problem hiding this comment.
This is dropping field_name so the error will be of the form Cannot change Map sorted flag during schema adaptation which does not include a column name. Other errors (e.g. Dictionary below) do include it.
| #[test] | ||
| fn test_unsorted_map_non_injective_key_cast_rejected() { |
There was a problem hiding this comment.
Can we add positive allowlist tests? E.g. Map(Int32,V)→Map(Int64,V)
| let cast_entries = | ||
| StructArray::new(target_fields.clone(), vec![cast_keys, cast_values], None); |
There was a problem hiding this comment.
This can panic w/ Map(Utf8, non-null Utf8) → Map(Utf8, non-null Int32) and CastOptions { safe: true }. Can we use try_new(...)? instead?
The same issue already exists in cast_struct_column (pre existing), might be nice to fix at the same time:
datafusion/datafusion/common/src/nested_struct.rs
Lines 112 to 113 in c429919
| /// Returns an equivalent MapArray whose entries contain only values reachable | ||
| /// from visible Map rows. | ||
| /// | ||
| /// Arrow Map arrays can contain unreachable entries after slicing, or entries | ||
| /// hidden behind null parent rows. An identity `take` rebuilds the Map through | ||
| /// Arrow's selection kernel, normalizing offsets and dropping those unreachable | ||
| /// child entries before recursive key/value casts are applied. | ||
| fn compact_map_entries(map: &MapArray) -> Result<MapArray> { | ||
| let indices = UInt64Array::from_iter_values(0..map.len() as u64); | ||
| Ok(take(map, &indices, None)?.as_map().clone()) | ||
| } |
There was a problem hiding this comment.
Worth noting that this makes Map the only container here that compacts. cast_list_column reuses the source offsets and casts the full backing child, and cast_fixed_size_list_column masks-and-retries only after a failure. The three containers now handle slice-hidden child data three different ways.
The approach here for Map seems like the right approach. MapArray::slice keeps entries whole and keys()/values() return the full backing children, so without this the cast would process every backing entry regardless of the slice. Measured rows=2 entries=20 for SELECT CAST(m AS MAP(BIGINT, VARCHAR)) FROM t LIMIT 2, so compaction is a saving, not an overhead.
The reason to raise it is that the list path has the same exposure and no protection. On current main:
CREATE TABLE lt AS SELECT * FROM (VALUES
(1, [struct('1')]), (2, [struct('2')]), (3, [struct('bad')])
) AS t(i, l);
SELECT arrow_cast(l, 'List(Struct("c0": Int32))') FROM lt LIMIT 2;
-- Cast error: Cannot cast string 'bad' to value of Int32 typeRow 3 is excluded by the LIMIT, but the cast still sees it. I suggest we file a tracking issue and defer, but a comment here saying why Map compacts and the list types don't would stop it reading as an oversight.
|
This may be a bug: |
Which issue does this PR close?
Closes #20835
This is the last of a series of PR to close #20835
Rationale for this change
Maptypes were not handled by DataFusion's recursive nested schema adaptation logic, leaving map key/value struct evolution undefined and unsupported. This change adds centralized validation and casting semantics forMapcolumns so planner-time validation and runtime casting behave consistently for supported schema evolution scenarios while rejecting unsafe key evolution.What changes are included in this PR?
Add recursive
Mapsupport to the nested schema adaptation path used by planning and execution.Introduce
Mapcompatibility validation covering:Structfields,Implement
Mapcasting that:Extend nested-cast detection so compatible
Maptypes are routed through the specialized nested adaptation path.Document supported map evolution semantics alongside the validation logic.
Add comprehensive unit tests covering:
Add end-to-end Parquet tests covering successful map value struct evolution and rejection of incompatible evolution.
Are these changes tested?
Yes.
New test coverage includes unit tests in
nested_struct.rsfor:It also adds end-to-end Parquet tests:
test_map_value_struct_schema_evolution_end_to_endtest_map_value_struct_incompatible_schema_evolution_rejectedAre there any user-facing changes?
Yes.
DataFusion now supports recursive schema adaptation for compatible
Maptypes whose keys and/or values recursively containStructs. Compatible schema evolution is applied during planning and execution using consistent validation rules, while unsupported or unsafe map key evolution, sorted-flag changes, and incompatible non-nullable schema changes are rejected. This extends schema evolution behavior but does not introduce a public API change.LLM-generated code disclosure
This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed.