Repository navigation
fix: preserve union output schema metadata - #26971
Phoenix500526 wants to merge 2 commits into
Conversation
ef0df4f to
1e11145
Compare
|
cc @kosiew Could you take a look at this follow-up to #24098? #24094 already moved UNION/INTERLEAVE schema alignment into physical planning, but the projection still inherits each input’s schema-level metadata instead of the merged metadata. This PR fixes that remaining output metadata mismatch and strengthens the existing test to compare complete batch schemas. |
1e11145 to
53869b1
Compare
There was a problem hiding this comment.
Thanks for the fix. The metadata-aware projection constructor is the right approach, and the strengthened regression test confirms that output batches expose the merged schema metadata. I found no correctness regression or public API break.
The important boundary is schema coercion, where the UNION's declared metadata should become authoritative for emitted batches. Enforcing this at construction time also covers INTERLEAVE without introducing runtime batch rewriting or additional execution branches. Existing projection machinery preserves metadata overrides through rebuilding and prevents unsafe projection elimination.
I have an optional integration-coverage suggestions, non blocking.
e647044 to
5f4490c
Compare
5f4490c to
7cd77ca
Compare
There was a problem hiding this comment.
Thanks for addressing the follow-up coverage request. The metadata-aware projection correctly preserves the merged UNION schema on emitted batches, and the expanded regression tests verify complete schema equality while retaining the existing nullability and value checks.
The construction-time coercion boundary remains the right place for this fix. It keeps schema ownership with the shared UNION and INTERLEAVE machinery, avoids runtime metadata adapters, and preserves existing execution semantics without introducing public API changes.
I verified that the new identical-field case exercises plain Column coercion when only schema-level metadata differs. My previous comment is addressed, and I found no remaining issues requiring changes.
7cd77ca to
d3cb94d
Compare
Output batches must retain the merged schema metadata. Refs apache#24098
Exercise schema coercion when all input fields are identical. Refs apache#24098
d3cb94d to
527351b
Compare
Which issue does this PR close?
Related to #24098. This PR addresses the output schema metadata mismatch.
Rationale for this change
When UNION inputs have different schema-level metadata, output batches can retain the metadata of their original inputs even though the UNION declares a merged schema. Consumers inspecting the returned batches therefore see incomplete or conflicting metadata.
For example, the existing regression test produces a batch with
shared: input0, while the declared output schema containsshared: input2and metadata keys from all three inputs.What changes are included in this PR?
ProjectionExec::try_new_with_schema_metadataincoerce_schemato apply the merged schema metadata to the projection output.test_union_schema_metadata_and_nullabilityto compare complete output batch schemas instead of only their fields.What is the testing strategy for this PR?
The strengthened test fails before the fix and passes afterward across all five nullability combinations. It also verifies that output values are preserved.
Passed locally:
cargo fmt --allcargo clippy --all-targets --all-features -- -D warningsAGENTS.md, including all 545 SQLLogicTest files.Are there any user-facing changes?
UNION output batches now carry the merged schema-level metadata declared by the plan. Values and row counts are unchanged. There are no public API changes.