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

Prioritize high confidence stats during broadcast joins #23016

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

abhinavmuk04
Copy link
Contributor

@abhinavmuk04 abhinavmuk04 commented Jun 14, 2024

Description

Prioritize high confidence stats during broadcast joins if enabled

Motivation and Context

When there are two PlanNodes in which they are both small enough for broadcast join we will prioritize the side which has higher confidence stats. If they both have high confidence stats then we keep the original behavior. The user has the ability to turn this on and off.

Impact

This change will create a feature which the user can utilize to improve optimization and help improve the execution time of broadcast join queries

Test Plan

Implemented various tests in both DetermineJoinDistributionType and ReorderJoinsType, which will check if, with the session property enable, nodes with the higher confidence stats will be broadcasted

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

General Changes
* Add confidence based broadcasting, side of join with highest confidence will be on build side. 
  This can be enabled with the ``confidence_based_broadcast`` session property :pr:`23016`

@abhinavmuk04 abhinavmuk04 force-pushed the milestone2p1 branch 18 times, most recently from 4ec8d3c to 3678abb Compare June 26, 2024 23:00
@abhinavmuk04 abhinavmuk04 force-pushed the milestone2p1 branch 2 times, most recently from 130f853 to ea33056 Compare June 27, 2024 19:46
@abhinavmuk04 abhinavmuk04 marked this pull request as ready for review June 27, 2024 19:55
@abhinavmuk04 abhinavmuk04 force-pushed the milestone2p1 branch 3 times, most recently from 14a700a to 23992de Compare June 29, 2024 22:13
@abhinavmuk04 abhinavmuk04 force-pushed the milestone2p1 branch 3 times, most recently from 0d702bb to 6395dd5 Compare June 29, 2024 22:59
@feilong-liu
Copy link
Contributor

Code lgtm. However, as a code owner, I do not have ownership for the SystemSessionProperty file, will need an committer approval for help to merge the change here.

@abhinavmuk04 abhinavmuk04 merged commit 461a31a into prestodb:master Jul 1, 2024
56 checks passed
@abhinavmuk04 abhinavmuk04 deleted the milestone2p1 branch July 1, 2024 22:38
@tdcmeehan tdcmeehan mentioned this pull request Aug 23, 2024
34 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants