-
Notifications
You must be signed in to change notification settings - Fork 574
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
fix(batch): schedule single task for singleton table scan #5907
Conversation
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Codecov Report
@@ Coverage Diff @@
## main #5907 +/- ##
==========================================
- Coverage 74.89% 74.86% -0.03%
==========================================
Files 924 924
Lines 147014 146913 -101
==========================================
- Hits 110104 109992 -112
- Misses 36910 36921 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hey @BugenZhao, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by clicking "Update branch" or pushing an empty commit with |
BatchExchange { order: [], dist: Single } | ||
└─BatchProject { exprs: [max(s.v)] } | ||
└─BatchHashAgg { group_key: [s.k], aggs: [max(s.v)] } | ||
└─BatchScan { table: s, columns: [s.k, s.v], distribution: Single } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take this for example, root fragment task, no matter before-and-after pr, is always executed on FE, while one is exchange singleton and another is hash agg.
What's the benefit of changing? 🤔 Unify with DML?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's for this bug: #4164
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.
What's changed and what's your intention?
For singleton table, we make the distribution of the table scan
Single
instead ofUpstreamHashShard(vec![])
.Per discussion with @liurenjie1024, as the root stage is always executed on the frontend, we insert an additional
Single
if necessary to ensure the table scan runs on the compute node just like what we do for DMLs.Checklist
./risedev check
(or alias,./risedev c
)Refer to a related PR or issue link (optional)