[SQL][minor] correct semanticEquals logic #6261
Conversation
|
Merged build triggered. |
|
Merged build started. |
|
Test build #33077 has started for PR 6261 at commit |
|
Test build #33077 has finished for PR 6261 at commit
|
|
Merged build finished. Test PASSed. |
|
Test PASSed. |
There was a problem hiding this comment.
Is this for expressions like Coalesce whose children order is sensitive?
There was a problem hiding this comment.
It's not about the order, it's just we don't need a map here as we search it with find, not get.
|
@cloud-fan Would you mind to update the PR description so that it briefly summaries changes made in this change, and add the current PR description as a comment? |
|
Merged build triggered. |
|
Merged build started. |
|
Test build #34604 has started for PR 6261 at commit |
|
Test build #34604 has finished for PR 6261 at commit
|
|
Merged build finished. Test FAILed. |
|
Merged build triggered. |
|
Merged build started. |
|
Test build #34747 has started for PR 6261 at commit |
|
Test build #34747 has finished for PR 6261 at commit
|
|
Merged build finished. Test PASSed. |
|
Thanks, merging to master. |
|
Forgot to mention before merging this... But please always file a JIRA ticket to track PRs, unless the PR is really minor updates (like documentation/comment typos). |
|
sorry... will keep it in mind :) |
It's a follow up of apache#6173, for expressions like `Coalesce` that have a `Seq[Expression]`, when we do semantic equal check for it, we need to do semantic equal check for all of its children. Also we can just use `Seq[(Expression, NamedExpression)]` instead of `Map[Expression, NamedExpression]` as we only search it with `find`. chenghao-intel, I agree that we probably never knows `semanticEquals` in a general way, but I think we have done that in `TreeNode`, so we can use similar logic. Then we can handle something like `Coalesce(children: Seq[Expression])` correctly. Author: Wenchen Fan <[email protected]> Closes apache#6261 from cloud-fan/tmp and squashes the following commits: 4daef88 [Wenchen Fan] address comments dd8fbd9 [Wenchen Fan] correct semanticEquals
4 participants
It's a follow up of #6173, for expressions like
Coalescethat have aSeq[Expression], when we do semantic equal check for it, we need to do semantic equal check for all of its children.Also we can just use
Seq[(Expression, NamedExpression)]instead ofMap[Expression, NamedExpression]as we only search it withfind.@chenghao-intel, I agree that we probably never knows
semanticEqualsin a general way, but I think we have done that inTreeNode, so we can use similar logic. Then we can handle something likeCoalesce(children: Seq[Expression])correctly.