You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "xavlee (via GitHub)" <gi...@apache.org> on 2025/12/09 20:06:29 UTC
[PR] fix: use SortMergeJoinExec consistently in phyical plan outputs [datafusion]
xavlee opened a new pull request, #19246:
URL: https://github.com/apache/datafusion/pull/19246
Which issue does this PR close?
Closes #19238
Rationale for this change
SortMergeJoinExec is currently displayed inconsistently across physical plan formats.
The tree-format plan uses SortMergeJoinExec, while the indent-format plan uses SortMergeJoin.
This inconsistency leads to confusing output and mismatched expectations in SQL logic tests.
Standardizing the operator name improves clarity and aligns with the naming conventions of other execution operators.
What changes are included in this PR?
Updates the DisplayAs implementation for SortMergeJoinExec so both the indent and tree physical plan formats display the same operator name.
Updates SQL Logic Test expected outputs in joins.slt to reflect the unified naming.
No functional behavior changes; this is a display/consistency fix.
Are these changes tested?
Yes. Existing SQL Logic Tests reference the operator name in physical plan output.
I updated those expected outputs to match the new standardized naming.
All tests pass with the updated format.
Are there any user-facing changes?
Yes—users inspecting physical plans will now consistently see:
SortMergeJoinExec
instead of an inconsistent mix of SortMergeJoin and SortMergeJoinExec.
This is a non-breaking, display-only change.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec name consistently in physical plan outputs [datafusion]
Posted by "xavlee (via GitHub)" <gi...@apache.org>.
xavlee commented on PR #19246:
URL: https://github.com/apache/datafusion/pull/19246#issuecomment-3671394566
I will complete this today, apologies for the delay.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec name consistently in physical plan outputs [datafusion]
Posted by "xavlee (via GitHub)" <gi...@apache.org>.
xavlee commented on code in PR #19246:
URL: https://github.com/apache/datafusion/pull/19246#discussion_r2633273936
##########
datafusion/physical-plan/src/joins/sort_merge_join/exec.rs:
##########
@@ -360,7 +360,7 @@ impl DisplayAs for SortMergeJoinExec {
};
write!(
f,
- "SortMergeJoin: join_type={:?}, on=[{}]{}{}",
+ "SortMergeJoinExec: join_type={:?}, on=[{}]{}{}",
Review Comment:
Thx, added.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec name consistently in physical plan outputs [datafusion]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #19246:
URL: https://github.com/apache/datafusion/pull/19246#issuecomment-3673113001
> @alamb I've rebased my changes and resolved conflicts 🙏 . You mentioned there were other tests that may also have to be updated–should I open a separate PR to address those?
I kicked CI so let's see if there are any tests that expect specific node names. Those would be tests that need updating.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec consistently in physical plan outputs [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #19246:
URL: https://github.com/apache/datafusion/pull/19246#issuecomment-3637361083
Looks like there is a bunch more test updates that are needed here
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec name consistently in physical plan outputs [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #19246:
URL: https://github.com/apache/datafusion/pull/19246#issuecomment-3671293664
@xavlee will you be able to finish this PR (resolve the conflicts and update the tests?)
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec consistently in physical plan outputs [datafusion]
Posted by "NGA-TRAN (via GitHub)" <gi...@apache.org>.
NGA-TRAN commented on PR #19246:
URL: https://github.com/apache/datafusion/pull/19246#issuecomment-3634831398
cc @alamb @gabotechs : tiny but meaningful
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec consistently in physical plan outputs [datafusion]
Posted by "geoffreyclaude (via GitHub)" <gi...@apache.org>.
geoffreyclaude commented on code in PR #19246:
URL: https://github.com/apache/datafusion/pull/19246#discussion_r2608344337
##########
datafusion/physical-plan/src/joins/sort_merge_join/exec.rs:
##########
@@ -360,7 +360,7 @@ impl DisplayAs for SortMergeJoinExec {
};
write!(
f,
- "SortMergeJoin: join_type={:?}, on=[{}]{}{}",
+ "SortMergeJoinExec: join_type={:?}, on=[{}]{}{}",
Review Comment:
This could even be `"{}: ...", Self::static_name()`
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec name consistently in physical plan outputs [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #19246:
URL: https://github.com/apache/datafusion/pull/19246#issuecomment-3685016630
Thanks @xavlee and everyone involved, good job
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec name consistently in physical plan outputs [datafusion]
Posted by "xavlee (via GitHub)" <gi...@apache.org>.
xavlee commented on PR #19246:
URL: https://github.com/apache/datafusion/pull/19246#issuecomment-3673090000
@alamb I've rebased my changes and resolved conflicts 🙏 . You mentioned there were other tests that may also have to be updated–should I open a separate PR to address those?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec name consistently in physical plan outputs [datafusion]
Posted by "xavlee (via GitHub)" <gi...@apache.org>.
xavlee commented on PR #19246:
URL: https://github.com/apache/datafusion/pull/19246#issuecomment-3673209245
Got it, I'll update those tests as well
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Use SortMergeJoinExec name consistently in physical plan outputs [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead merged PR #19246:
URL: https://github.com/apache/datafusion/pull/19246
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org