[Refactor](Nerieds) Refactor aggregate function/plan/rules and support related cbo rules #14827

924060929 · 2022-12-05T10:29:47Z

Proposed changes

refactor

add AggregateExpression to shield the difference of AggregateFunction before disassemble and after
request GATHER physicalProperties for query, because query always collect result to the coordinator, use GATHER maybe select a better plan
refactor NormalizeAggregate
remove some physical fields for the LogicalAggregate, like AggPhase, isDisassemble
remove AggregateDisassemble and DistinctAggregateDisassemble, and use AggregateStrategies to generate various of PhysicalHashAggregate, like two phases aggregate, three phases aggregate, and cascades can auto select the lowest cost alternative.
move PushAggregateToOlapScan to AggregateStrategies
separate the traverse and visit method in FoldConstantRuleOnFE
- if some expression not implement the visit method, the traverse method can handle and rewrite the children by default
- if some expression implement the visit, the user defined traverse(invoke accept/visit method) will quickly return because the default visit method will not forward to the children, and the pre-process in traverse method will not be skipped.

new feature

support disable_nereids_rules to skip some rules.

example:

create 1 bucket table n

CREATE TABLE `n` (
  `id` bigint(20) NOT NULL
) ENGINE=OLAP
DUPLICATE KEY(`id`)
COMMENT 'OLAP'
DISTRIBUTED BY HASH(`id`) BUCKETS 1
PROPERTIES (
"replication_allocation" = "tag.location.default: 1",
"in_memory" = "false",
"storage_format" = "V2",
"disable_auto_compaction" = "false"
);

insert some rows into n

insert into n select * from numbers('number'='20000000')

query table n

SET enable_nereids_planner=true;
SET enable_vectorized_engine=true;
SET enable_fallback_to_original_planner=false;
explain plan select id from n group by id;

the result show that we use the one stage aggregate

| PhysicalHashAggregate ( aggPhase=LOCAL, aggMode=INPUT_TO_RESULT, groupByExpr=[id#0], outputExpr=[id#0], partitionExpr=Optional.empty, requestProperties=[GATHER], stats=(rows=1, width=1, penalty=2.0E7) ) |
| +--PhysicalProject ( projects=[id#0], stats=(rows=20000000, width=1, penalty=0.0) )                                                                                                                                                                                                |
|    +--PhysicalOlapScan ( qualified=default_cluster:test.n, output=[id#0, name#1], stats=(rows=20000000, width=1, penalty=0.0) )                                                                                                                                                    |

disable one stage aggregate

explain plan select
  /*+SET_VAR(disable_nereids_rules=DISASSEMBLE_ONE_PHASE_AGGREGATE_WITHOUT_DISTINCT)*/
  id
from n
group by id

the result is two stage aggregate

| PhysicalHashAggregate ( aggPhase=GLOBAL, aggMode=BUFFER_TO_RESULT, groupByExpr=[id#0], outputExpr=[id#0], partitionExpr=Optional[[id#0]], requestProperties=[GATHER], stats=(rows=1, width=1, penalty=2.0E7) ) |
| +--PhysicalHashAggregate ( aggPhase=LOCAL, aggMode=INPUT_TO_BUFFER, groupByExpr=[id#0], outputExpr=[id#0], partitionExpr=Optional[[id#0]], requestProperties=[ANY], stats=(rows=1, width=1, penalty=2.0E7) )     |
|    +--PhysicalProject ( projects=[id#0], stats=(rows=20000000, width=1, penalty=0.0) )                                                                                                                                                                                                   |
|       +--PhysicalOlapScan ( qualified=default_cluster:test.n, output=[id#0, name#1], stats=(rows=20000000, width=1, penalty=0.0) )                                                                                                                                                       |

Checklist(Required)

Does it affect the original behavior:
- Yes
- No
- I don't know
Has unit tests been added:
- Yes
- No
- No Need
Has document been added or modified:
- Yes
- No
- No Need
Does it need to update dependencies:
- Yes
- No
Are there any changes that cannot be rolled back:
- Yes (If Yes, please explain WHY)
- No

hello-stephen · 2022-12-05T14:39:28Z

TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 34.81 seconds
load time: 724 seconds
storage size: 17123497756 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221218045050_clickbench_pr_64696.html

fe/fe-core/src/main/java/org/apache/doris/analysis/AggregateInfo.java

morrySnow · 2022-12-12T09:28:10Z

fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java

@@ -314,6 +320,11 @@ public CascadesContext getCascadesContext() {
        return cascadesContext;
    }

+    public static PhysicalProperties buildInitRequireProperties(Plan initPlan) {
+        boolean isQuery = !(initPlan instanceof Command) || (initPlan instanceof ExplainCommand);


why add (initPlan instanceof ExplainCommand)

currently, explain command only for explain query, so we should require gather for it.
when the explain command support other sqls, like insert into, we should change it.

fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/DeriveStatsJob.java

morrySnow · 2022-12-13T03:57:25Z

fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java

@@ -667,7 +668,8 @@ public String toString() {
            builder.append(group).append("\n");
            builder.append("  stats=").append(group.getStatistics()).append("\n");
            StatsDeriveResult stats = group.getStatistics();
-            if (stats != null && group.getLogicalExpressions().get(0).getPlan() instanceof LogicalOlapScan) {
+            if (stats != null && !group.getLogicalExpressions().isEmpty()


this is because, some group has no logical expression?

morrySnow · 2022-12-13T04:10:00Z

fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java

+        if (!olapScan.getTable().isColocateTable() && olapScan.getScanTabletNum() == 1) {
+            return PhysicalProperties.GATHER;


add a todo, let's find a better way to handle both tablet num == 1 and colocate table together in future

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/DistinctToGroupBy.java

morrySnow · 2022-12-13T16:44:39Z

.../main/java/org/apache/doris/nereids/rules/expression/rewrite/rules/FoldConstantRuleOnFE.java

+            return expr;
+        } else if (expr instanceof AggregateExpression && ((AggregateExpression) expr).getFunction().isDistinct()) {
+            return expr;


why their child could not be fold?

maybe erase some distinct columns, that will be some bug

morrySnow · 2022-12-13T16:53:22Z

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/NormalizeToSlot.java

+        public final Expression originExpr;
+        public final Slot remainExpr;
+        public final NamedExpression pushedExpr;


nit: add a example to explain them

morrySnow · 2022-12-13T17:14:02Z

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/NormalizeAggregate.java

-
-            return root;
+            // push expression to bottom project
+            Set<Alias> existsAliases = ExpressionUtils.collect(


does it solve the problem of explosion of data? SQL like:

SELECT a + 1, a + 2, a + 3, SUM(b) FROM t GROUP BY a + 1, a + 2, a + 3; SELECT a, SUM(b + 1), SUM(b + 2), SUM(b + 3) FROM t GROUP BY a;

the result is

LogicalAggregate(groupBy=(slot#1, slot#2, slot#3), output=[slot#1, slot#2, slot#3, sum(slot#4)]) | LogicalProject(projects=[(a + 1)#1, (a + 2)#2, (a + 3)#3, b#4])

and

LogicalAggregate(groupBy=(slot#0), output=[slot#0, sum(slot#1), sum(slot#2), sum(slot#3)]) | LogicalProject(projects=[a#0, (b + 1)#1, (b + 2)#2, (b + 3)#3])

morrySnow · 2022-12-13T17:24:12Z

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateStrategies.java

+     */
+    private List<PhysicalHashAggregate<Plan>> onePhaseAggregateWithoutDistinct(
+            LogicalAggregate<? extends Plan> logicalAgg, ConnectContext connectContext) {
+        RequireProperties requireGather = RequireProperties.of(PhysicalProperties.GATHER);


gather require could be a static member of RequireProperties

RequireProperties has a List<PhysicalProperties> properties match to every require physical properties of children.
so RequireProperties.GATHER is only used for one child and not very general

github-actions · 2022-12-18T13:49:05Z

PR approved by at least one committer and no changes requested.

github-actions · 2022-12-18T13:49:08Z

PR approved by anyone and no changes requested.

github-actions bot added area/nereids area/planner Issues or PRs related to the query planner kind/test labels Dec 5, 2022

924060929 marked this pull request as draft December 5, 2022 10:31

924060929 force-pushed the refactor-agg branch from 449c99f to f3377b6 Compare December 5, 2022 13:55

924060929 force-pushed the refactor-agg branch 2 times, most recently from 4f1a2be to 0ed25f3 Compare December 9, 2022 10:33

924060929 marked this pull request as ready for review December 9, 2022 10:33

924060929 force-pushed the refactor-agg branch 5 times, most recently from 67dc73e to ea3e000 Compare December 12, 2022 15:33

924060929 changed the title ~~(Refactor) (Nerieds) Refactor aggregate function/plan/rules and support related cbo rules~~ [Refactor] (Nerieds) Refactor aggregate function/plan/rules and support related cbo rules Dec 13, 2022

924060929 changed the title ~~[Refactor] (Nerieds) Refactor aggregate function/plan/rules and support related cbo rules~~ [Refactor](Nerieds) Refactor aggregate function/plan/rules and support related cbo rules Dec 13, 2022

924060929 force-pushed the refactor-agg branch from ea3e000 to b071b96 Compare December 13, 2022 05:03

morrySnow reviewed Dec 13, 2022

View reviewed changes

924060929 force-pushed the refactor-agg branch 2 times, most recently from d2b5625 to 0989b92 Compare December 17, 2022 14:02

924060929 added 10 commits December 17, 2022 22:04

refector NormalizeAggregate

372e239

refector NormalizeAggregate

8e0bfc5

refector NormalizeAggregate

881203c

refector NormalizeAggregate

1ebd5a4

refector NormalizeAggregate

982e921

refector NormalizeAggregate

5b5f2be

refector NormalizeAggregate

37f581c

refector NormalizeAggregate

405cb31

refector NormalizeAggregate

5487606

refector NormalizeAggregate

eb02090

924060929 added 20 commits December 17, 2022 22:04

fix rebase conflict

20ff449

fix checkstyle

b0242b7

add multi distinct CheckAnalysis

1bdccf4

fix ut and refactor FoldConstantRuleOnFE

29ff5b8

fix regression test

742f0bc

skip a wrong result

8b52a32

add enableSingleDistinctColumnOpt

c65c030

fix bug

4bbf037

fix bug

36f1045

rebase

b8f8a70

fix ut

d555eed

fix the bug when in multi-backends environment

712613e

rebase

5216529

rebase & add more strategies

9c510ea

rebase 5

fcfe710

remove useless code

18820e8

fix by comment

11b4b52

rebase and add EliminateAggregate

ff938d1

fix regression test

f8f97a5

fix regression test

d596c1c

924060929 force-pushed the refactor-agg branch from 0989b92 to d596c1c Compare December 18, 2022 04:24

morrySnow approved these changes Dec 18, 2022

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Dec 18, 2022

github-actions bot added the reviewed label Dec 18, 2022

morrySnow merged commit af4d9b6 into apache:master Dec 18, 2022

ByteYue mentioned this pull request Dec 20, 2022

[Bug](FE) fix compile error due to code refactor #15192

Merged

13 tasks

hubgeter mentioned this pull request Jul 22, 2023

[opt](hive)opt select count(*) stmt push down agg on parquet in hive . #22115

Merged

924060929 deleted the refactor-agg branch April 16, 2024 11:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Refactor](Nerieds) Refactor aggregate function/plan/rules and support related cbo rules #14827

[Refactor](Nerieds) Refactor aggregate function/plan/rules and support related cbo rules #14827

924060929 commented Dec 5, 2022 •

edited

Loading

hello-stephen commented Dec 5, 2022 •

edited

Loading

morrySnow Dec 12, 2022

924060929 Dec 16, 2022

morrySnow Dec 13, 2022

924060929 Dec 16, 2022

morrySnow Dec 13, 2022

morrySnow Dec 13, 2022

924060929 Dec 16, 2022

morrySnow Dec 13, 2022

morrySnow Dec 13, 2022

924060929 Dec 16, 2022

morrySnow Dec 13, 2022

924060929 Dec 16, 2022 •

edited

Loading

github-actions bot commented Dec 18, 2022

github-actions bot commented Dec 18, 2022

		if (!olapScan.getTable().isColocateTable() && olapScan.getScanTabletNum() == 1) {
		return PhysicalProperties.GATHER;

[Refactor](Nerieds) Refactor aggregate function/plan/rules and support related cbo rules #14827

[Refactor](Nerieds) Refactor aggregate function/plan/rules and support related cbo rules #14827

Conversation

924060929 commented Dec 5, 2022 • edited Loading

Proposed changes

refactor

new feature

Checklist(Required)

hello-stephen commented Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

924060929 Dec 16, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Dec 18, 2022

github-actions bot commented Dec 18, 2022

924060929 commented Dec 5, 2022 •

edited

Loading

hello-stephen commented Dec 5, 2022 •

edited

Loading

924060929 Dec 16, 2022 •

edited

Loading