-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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] (planner) support auto aggregation for random distributed table #33630
[fix] (planner) support auto aggregation for random distributed table #33630
Conversation
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
run buildall |
TPC-H: Total hot run time: 38165 ms
|
TPC-DS: Total hot run time: 184601 ms
|
ClickBench: Total hot run time: 30.2 s
|
Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
|
run buildall |
TPC-H: Total hot run time: 38061 ms
|
TPC-DS: Total hot run time: 183526 ms
|
ClickBench: Total hot run time: 30.35 s
|
Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
|
fe/fe-core/src/main/java/org/apache/doris/analysis/StmtRewriter.java
Outdated
Show resolved
Hide resolved
26c6eeb
to
e005f3e
Compare
c932047
to
d2150a8
Compare
run p0 |
run buildall |
TPC-H: Total hot run time: 38329 ms
|
TPC-DS: Total hot run time: 186263 ms
|
ClickBench: Total hot run time: 30.35 s
|
run p0 |
Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
|
TPC-H: Total hot run time: 40795 ms
|
TPC-DS: Total hot run time: 187523 ms
|
pls hold on for a while, I need to think this through and discuss with my coworker. I'll give you feedback next week, thanks. |
public List<Rule> buildRules() { | ||
return ImmutableList.of( | ||
// Project(Scan) | ||
logicalProject(logicalOlapScan()).when(project -> isRandomDistributedTbl(project.child())) |
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.
better to simply this rule by only match logicalOlapScan with randomDist, then always gererate a agg node above it. The MergeAggregate rule can merge redundant agg later.
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.
check agg(olapScan) there will avoid to rewrite olapScan to agg(olapScan) twice.
if (col.isAggregated()) { | ||
AggregateType aggType = col.getAggregationType(); | ||
// agg type not mach | ||
return (aggType == AggregateType.SUM || aggType == AggregateType.MAX |
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.
we have to deal with other agg types, including HLL_UNION, BITMAP_UNION, QUANTILE_UNION and GENERIC(agg_state related)
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.
done
Generally, we can only fix the bug for nereids planner, unless there is a strong needs for branch 2.0.(the old planner will be discarded very soon). And we should consider not only MAX, MIN, SUM and also other agg types like HLL_UNION, BITMAP_UNION, QUANTILE_UNION and GENERIC(agg_state related) |
da53339
to
3d41b98
Compare
3d41b98
to
d4a168d
Compare
run buildall |
run p0 |
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
PR approved by at least one committer and no changes requested. |
…ed table (#33630) support auto aggregation for querying detail data of random distributed table: the same key column will return only one row.
…ed table (apache#33630) support auto aggregation for querying detail data of random distributed table: the same key column will return only one row.
…ed table (#33630) support auto aggregation for querying detail data of random distributed table: the same key column will return only one row.
Proposed changes
Issue Number: close #xxx
support auto aggregation for querying detail data of random distributed table: the same key column will return only one row.
step 1: fix legacy planner (√)
step 2: fix nereids planner (√)
Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...