Skip to content

[SPARK-34729][SQL][FOLLOWUP] Broadcast nested loop join to use executeTake instead of execute#31845

Closed
c21 wants to merge 1 commit intoapache:masterfrom
c21:join-followup
Closed

[SPARK-34729][SQL][FOLLOWUP] Broadcast nested loop join to use executeTake instead of execute#31845
c21 wants to merge 1 commit intoapache:masterfrom
c21:join-followup

Conversation

@c21
Copy link
Contributor

@c21 c21 commented Mar 15, 2021

What changes were proposed in this pull request?

This is a followup minor change from #31821 (comment) , where we change from using execute() to executeTake(). Performance-wise there's no difference. We are just using a different API to be aligned with code path of Dataset.

Why are the changes needed?

To align with other code paths in SQL/Dataset.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing unit tests same as #31821 .

@c21
Copy link
Contributor Author

c21 commented Mar 15, 2021

cc @cloud-fan and @maropu could you help take a look when you have time, thanks.

@dongjoon-hyun dongjoon-hyun changed the title [MINOR][SQL][FOLLOWUP] Broadcast nested loop join to use executeTake instead of execute [SPARK-34729][SQL][FOLLOWUP] Broadcast nested loop join to use executeTake instead of execute Mar 15, 2021
@dongjoon-hyun
Copy link
Member

Hi, @c21 . [MINOR] is used when we have no corresponding JIRA id.

@c21
Copy link
Contributor Author

c21 commented Mar 15, 2021

@dongjoon-hyun - thanks for correction. I was wrongly assuming followup PRs should not use original JIRA id. Will follow the convention next time, thanks.

@dongjoon-hyun
Copy link
Member

BTW, GitHub Action is currently broken.

@c21
Copy link
Contributor Author

c21 commented Mar 15, 2021

retest this please

@c21 c21 closed this Mar 15, 2021
@c21 c21 reopened this Mar 15, 2021
@github-actions github-actions bot added the SQL label Mar 15, 2021
@c21
Copy link
Contributor Author

c21 commented Mar 15, 2021

Closed & reopened PR to trigger unit test in github action.

@dongjoon-hyun
Copy link
Member

Merged to master.

@c21
Copy link
Contributor Author

c21 commented Mar 16, 2021

Thank you @dongjoon-hyun and @maropu for review!

@c21 c21 deleted the join-followup branch March 16, 2021 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants