Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-45925][SQL] Making SubqueryBroadcastExec equivalent to SubqueryAdaptiveBroadcastExec #43807

Closed
wants to merge 5 commits into from

Conversation

ahshahid
Copy link

What changes were proposed in this pull request?

Implementing equals and hashCode in SubqueryBroadcastExec so that it is made equivalent to SubqueryAdaptiveBroadcastExec . During the bug testing and fix for re-use of exchange in AQE, it turned out that the first exchange got created with SubqueryAdaptiveBroadcastExec, while another incoming exchange which is as such identical to the existing one, but differed as it was having SubqueryBroadcastExec, which contributed to re-use of exchange not happening.

Why are the changes needed?

Needed so that reuse of exchange happens correctly in AQE

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added test for checking the equivalence with SubqueryAdaptiveBroadcastExec.

Was this patch authored or co-authored using generative AI tooling?

No

ashahid added 5 commits November 13, 2023 21:36
… by also canonicalizing the build plan. Also making SubqueryBroadcastExec equivalent to SubqueryAdaptiveBroadcastExec. Similar equivalence will also be done for SubquderyBroadcastExec under separate PR
@github-actions github-actions bot added the SQL label Nov 14, 2023
@ahshahid ahshahid changed the title Spark 45925: Making SubqueryBroadcastExec equivalent to SubqueryAdaptiveBroadcastExec SPARK-45925: Making SubqueryBroadcastExec equivalent to SubqueryAdaptiveBroadcastExec Nov 14, 2023
@HyukjinKwon HyukjinKwon changed the title SPARK-45925: Making SubqueryBroadcastExec equivalent to SubqueryAdaptiveBroadcastExec [SPARK-45925][SQL] Making SubqueryBroadcastExec equivalent to SubqueryAdaptiveBroadcastExec Nov 15, 2023
@HyukjinKwon
Copy link
Member

cc @ulysses-you and @peter-toth FYI

@ahshahid
Copy link
Author

ahshahid commented Nov 15, 2023

The other option is that we make the canonicalized form of both SubqueryAdaptiveBroadcastExec and SubqueryBroadcastExec to be of type SubqueryBroadcastExec. that way equals and hashCode will go away and it will serve our purpose.

@ahshahid
Copy link
Author

I think if I get clean run on PR SPARK-45924, then this PR can be closed without merging

@ahshahid
Copy link
Author

@beliefer I think you may be right. In my another PR for broadcast-var-pushdown, I am seeing unmodified SubqueryAdaptiveBroadcastExec in the stage cache 's keys. May be it is an issue in my code or something else. Will check my code again for this. So as of now, I think it makes sense to close this PR and also the other PR in SubqueryBroadcastHashExec

@ahshahid ahshahid closed this Nov 15, 2023
@ahshahid ahshahid deleted the SPARK-45925 branch November 15, 2023 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants