fangchenli opened a new pull request, #27081:
URL: https://github.com/apache/datafusion/pull/27081
## Which issue does this PR close?
- Closes #23434.
This picks up #23442, which the stale bot closed. It has been redone on top
of #23952.
## Rationale for this change
With two inputs whose schemas carry the same metadata key with different
values, this query fails with an internal error, while the equivalent hash join
works:
```sql
-- Internal error: Physical input schema should be the same as the one
converted
-- from logical input schema. schema metadata differs:
-- (physical) {"metadata_key": "left"} vs (logical) {"metadata_key": "right"}
select count(distinct b) from smaller_table, larger_table;
-- Works
select count(distinct b) from smaller_table join larger_table on b = a;
```
The logical plan and `build_join_schema` let the left input win for inner
joins. `CrossJoinExec::new` merges metadata itself and lets the right input
win, so its physical schema disagrees with its logical one.
**A question for @2010YOUY01 and @mkleen**, given the conclusion in #23461
and #23952 that conflicting metadata should stay an error rather than be
silently resolved: does this change fit with that?
I think it does, because it does not introduce a new override. Cross joins
on `main` already resolve a conflict silently when they are not swapped, just
in the opposite direction from everything else. This is how the query shapes
behave:
| Shape, with conflicting metadata | Hash join on `main` | Cross join on
`main` | Cross join with this PR |
| --- | --- | --- | --- |
| Not swapped, no aggregate | succeeds, left wins | succeeds, right wins |
succeeds, left wins |
| Not swapped, under an aggregate | succeeds | **internal error** | succeeds
|
| Swapped | `join_selection` error | `join_selection` error |
`join_selection` error |
The swap error from #23952 stays. If you would rather have every conflict be
an error, this PR is not the way to get there. That would be a wider change
across all join types, and I'm happy to close this one in favor of it.
## What changes are included in this PR?
- `CrossJoinExec::new` builds its schema with the shared
`build_join_schema`, as the other joins do, rather than merging metadata
itself. Fields are unchanged for an inner join; only the metadata precedence
changes.
- The expected error for `select * from larger_table cross join
smaller_table` in `metadata.slt` changes direction (`Expected "left", got
"right"`). It is still an error, and the direction now matches what a swapped
hash join reports.
## What is the testing strategy for this PR?
- `metadata.slt` covers both input orders under an aggregate, using the
conflicting tables #23952 added, with reordering disabled for the order that
would otherwise be swapped. Both fail with the internal error above without the
fix.
- A unit test, `cross_join_schema_metadata_is_left_biased`, pins the
precedence, matching the per-join-type tests from #16221.
## Are there any user-facing changes?
- A cross join under an aggregate with conflicting schema metadata now
succeeds instead of failing with an internal error.
- When a cross join's inputs have conflicting schema metadata, the output
now carries the left input's value rather than the right's.
There are no public API changes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
