Disallow positional struct casting when field names don’t overlap#19955
Merged
adriangb merged 17 commits intoapache:mainfrom Feb 3, 2026
Merged
Disallow positional struct casting when field names don’t overlap#19955adriangb merged 17 commits intoapache:mainfrom
adriangb merged 17 commits intoapache:mainfrom
Conversation
Remove positional struct field mapping, enforcing name overlap for struct casts and document new behavior in casting logic and validation errors. Update struct-cast tests to assert new no-overlap error conditions, including equal/unequal field counts and cast-time rejection scenarios.
github-actions
bot
added
sqllogictest
Contributor
|
I think we'll need an entry in |
kosiew
force-pushed
the
positional-fallback-19841
branch
from
d031ce0 to
fc32f78
Compare
github-actions
bot
added
the
documentation
adriangb
approved these changes
Jan 26, 2026
Contributor
adriangb
left a comment
adriangb
left a comment
There was a problem hiding this comment.
Some relatively minor nits
| /// | ||
| /// This is useful for validating struct compatibility when casting between structs, | ||
| /// ensuring that source and target fields have overlapping names. | ||
| pub fn has_one_of_more_common_fields( |
Contributor
There was a problem hiding this comment.
Does this need to be pub?
Contributor
Author
There was a problem hiding this comment.
Yes, this function is re-used at
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs#L662
// Skip const-folding when there is no field name overlap
if !has_one_of_more_common_fields(&source_fields, target_fields) {
return false;
}
| // Use struct builders or row constructors that preserve your mapping logic | ||
| ``` | ||
|
|
||
| **Why this change:** |
Contributor
There was a problem hiding this comment.
I think it's worth mentioning that this matches DuckDBs behavior
|
|
||
| ```sql | ||
| -- Explicitly rename to match source field names | ||
| SELECT CAST(source_col AS STRUCT<x INT, y INT>) FROM table1; |
Contributor
There was a problem hiding this comment.
Is this something other engines support? I would have guessed this fails under the "no name overlap" rule.
adriangb
approved these changes
Jan 27, 2026
Contributor
|
I plan to merge this tomorrow morning EST if there is no further feedback |
Contributor
|
Thanks for merging up @alamb ! |
Contributor
|
Thanks -- yeah I am just trying to keep the code flowing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Rationale for this change
Struct-to-struct casting previously fell back to positional mapping when there was no field-name overlap and the number of fields matched. That behavior is ambiguous and can silently produce incorrect results when source/target schemas have different field naming conventions or ordering.
This PR makes struct casting strictly name-based: when there is no overlap in field names between the source and target structs, the cast is rejected with a clear planning error. This prevents accidental, hard-to-detect data corruption and forces callers to provide explicit field names (or align schemas) when casting.
What changes are included in this PR?
cast_struct_column; child fields are now resolved only by name.validate_struct_compatibilityto error out when there is no field name overlap, instead of allowing positional compatibility checks.{id: 1}/{a: 1, b: 'x'}orstruct(… AS field)), avoiding reliance on positional behavior.Are these changes tested?
Yes.
nested_struct.rsto assert the new failure mode and error message.struct.slt,joins.slt) to use named struct literals so tests continue to validate struct behavior without positional casting.Are there any user-facing changes?
Yes — behavioral change / potential breaking change.
STRUCTs with no overlapping field names now fails (previously it could succeed via positional mapping if field counts matched).{a: 1, b: 'x'}orstruct(expr AS a, expr AS b)) or ensure schemas share field names.LLM-generated code disclosure
This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed and tested.