[feat](Nereids): rewrite sum literal to sum and count by keanji-x · Pull Request #32244 · apache/doris

keanji-x · 2024-03-14T08:45:36Z

Proposed changes

sum(v + 2) => sum(v) + 2*count(v)
sum(v - 2) => sum(v) - 2*count(v)

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...

doris-robot · 2024-03-14T08:45:40Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

keanji-x · 2024-03-14T08:46:37Z

run buildall

doris-robot · 2024-03-14T09:09:20Z

TPC-H: Total hot run time: 38352 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 2bca92b9a725b024843da6f278fa4e77247fc391, data reload: false
------ Round 1 ----------------------------------
q1	17701	4349	4127	4127
q2	2023	152	143	143
q3	10709	1043	904	904
q4	7095	730	738	730
q5	7470	2705	2711	2705
q6	188	122	121	121
q7	1189	821	814	814
q8	9333	2019	2041	2019
q9	7170	6482	6403	6403
q10	8460	3499	3620	3499
q11	417	228	214	214
q12	613	295	289	289
q13	17782	2860	2841	2841
q14	282	252	241	241
q15	510	456	433	433
q16	485	396	397	396
q17	952	643	557	557
q18	7320	6503	6401	6401
q19	1746	1409	1482	1409
q20	553	286	286	286
q21	6458	3529	3529	3529
q22	356	304	291	291
Total cold run time: 108812 ms
Total hot run time: 38352 ms
----- Round 2, with runtime_filter_mode=off -----
q1	4142	4100	4078	4078
q2	329	231	231	231
q3	3006	2829	2841	2829
q4	1856	1594	1631	1594
q5	5212	5242	5260	5242
q6	195	119	116	116
q7	2242	1871	1878	1871
q8	3184	3300	3308	3300
q9	8551	8578	8559	8559
q10	3702	3693	3579	3579
q11	534	437	441	437
q12	746	565	541	541
q13	16900	2881	2860	2860
q14	279	249	245	245
q15	482	441	440	440
q16	456	418	406	406
q17	1736	1493	1497	1493
q18	7635	7118	7128	7118
q19	1588	1523	1548	1523
q20	1888	1706	1757	1706
q21	4911	4565	4744	4565
q22	530	442	434	434
Total cold run time: 70104 ms
Total hot run time: 53167 ms

keanji-x · 2024-03-15T05:51:15Z

run buildall

keanji-x · 2024-03-15T07:27:00Z

run buildall

doris-robot · 2024-03-15T08:02:17Z

TPC-H: Total hot run time: 38452 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit f5e2cba61e87b105eac508518ca18e1782ab4f1a, data reload: false
------ Round 1 ----------------------------------
q1	17669	4265	4165	4165
q2	2031	154	158	154
q3	10576	1101	888	888
q4	8123	765	723	723
q5	7472	2736	2750	2736
q6	187	130	126	126
q7	1169	830	801	801
q8	9326	1972	1974	1972
q9	7099	6401	6379	6379
q10	8483	3531	3594	3531
q11	431	222	216	216
q12	597	301	291	291
q13	17782	2847	2848	2847
q14	276	248	255	248
q15	488	460	443	443
q16	515	395	386	386
q17	940	550	603	550
q18	7168	6445	6455	6445
q19	1566	1439	1413	1413
q20	531	282	276	276
q21	6434	3565	3573	3565
q22	347	297	307	297
Total cold run time: 109210 ms
Total hot run time: 38452 ms
----- Round 2, with runtime_filter_mode=off -----
q1	4112	4099	4068	4068
q2	324	229	219	219
q3	2921	2820	2796	2796
q4	1841	1596	1516	1516
q5	5198	5220	5212	5212
q6	195	116	116	116
q7	2219	1863	1850	1850
q8	3141	3305	3287	3287
q9	8605	8584	8599	8584
q10	3747	3731	3720	3720
q11	543	440	436	436
q12	714	558	575	558
q13	18518	2893	2916	2893
q14	286	242	251	242
q15	497	445	454	445
q16	458	431	413	413
q17	1747	1480	1479	1479
q18	7618	7198	7217	7198
q19	3536	1514	1531	1514
q20	1918	1739	1704	1704
q21	4836	4986	4845	4845
q22	569	511	487	487
Total cold run time: 73543 ms
Total hot run time: 53582 ms

keanji-x · 2024-03-15T08:13:04Z

run buildall

doris-robot · 2024-03-15T08:31:14Z

TPC-H: Total hot run time: 38240 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit f375f08e6ce92874f1c89162d69539b2e4bbd478, data reload: false
------ Round 1 ----------------------------------
q1	17703	5070	4208	4208
q2	2029	152	140	140
q3	10670	1079	878	878
q4	7097	728	707	707
q5	7479	2547	2597	2547
q6	188	123	123	123
q7	1186	806	808	806
q8	9326	1949	1991	1949
q9	7040	6478	6426	6426
q10	8454	3529	3676	3529
q11	433	225	222	222
q12	668	304	300	300
q13	17804	2887	2847	2847
q14	273	248	249	248
q15	506	453	446	446
q16	477	405	395	395
q17	951	512	509	509
q18	7314	6663	6458	6458
q19	3791	1484	1413	1413
q20	559	286	285	285
q21	6377	3552	3497	3497
q22	372	307	312	307
Total cold run time: 110697 ms
Total hot run time: 38240 ms
----- Round 2, with runtime_filter_mode=off -----
q1	4227	4196	4152	4152
q2	324	221	239	221
q3	2957	2790	2861	2790
q4	1831	1546	1538	1538
q5	5212	5230	5259	5230
q6	195	116	120	116
q7	2246	1862	1849	1849
q8	3173	3305	3263	3263
q9	8551	8554	8542	8542
q10	3669	3675	3636	3636
q11	540	430	432	430
q12	709	552	539	539
q13	17209	2876	2842	2842
q14	267	251	249	249
q15	477	451	449	449
q16	452	406	425	406
q17	1745	1508	1486	1486
q18	7489	7227	7067	7067
q19	1632	1534	1524	1524
q20	1907	1726	1707	1707
q21	4699	4790	4531	4531
q22	539	445	473	445
Total cold run time: 70050 ms
Total hot run time: 53012 ms

morrySnow · 2024-03-15T11:50:04Z

+ newAggOutput.add(namedCount);
+ }
+
+ LogicalAggregate<?> newAgg = agg.withAggOutput(ImmutableList.copyOf(newAggOutput));


we should keep group by key at head of output list

The order of origin output is not changed. It's ok

morrySnow · 2024-03-15T11:50:58Z

+ for (Expression e : countSumExpr) {
+ NamedExpression namedSum = constructSum(e, existedAggFunc);
+ NamedExpression namedCount = constructCount(e, existedAggFunc);
+ exprToSum.put(e, namedSum.toSlot());


could we try to reduce unnecessary expression compare as far as we can?

github-actions · 2024-03-19T07:13:03Z

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

github-actions · 2024-03-19T07:13:05Z

PR approved by anyone and no changes requested.

keanji-x · 2024-03-19T07:20:37Z

run p0

keanji-x · 2024-03-19T09:37:42Z

run buildall

wm1581066

LGTM

keanji-x · 2024-03-20T06:25:50Z

run buildall

github-actions · 2024-03-20T07:47:38Z

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

keanji-x · 2024-03-20T08:29:49Z

run buildall

keanji-x · 2024-03-21T02:37:43Z

run buildall

keanji-x · 2024-03-21T05:05:23Z

run p0

github-actions · 2024-03-22T02:26:43Z

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

sum(v + 2) => sum(v) + 2*count(v) sum(v - 2) => sum(v) - 2*count(v)

…55543) rule SumLiteralRewrite will rewrite `sum(expr +/- literal) ` ==> `sum(expr) +/- literal * count(expr)` but require `expr` is a slot, this PR remove this restriction, allow expr to be any expression. relate PR: - introduce rule SumLiteralRewrite: #32244 - add cast for `+,-,*, /`, then `sum(a + 10)` will become `sum(cast(a as bigint) + 10)`: #52837

…pache#55543) rule SumLiteralRewrite will rewrite `sum(expr +/- literal) ` ==> `sum(expr) +/- literal * count(expr)` but require `expr` is a slot, this PR remove this restriction, allow expr to be any expression. relate PR: - introduce rule SumLiteralRewrite: apache#32244 - add cast for `+,-,*, /`, then `sum(a + 10)` will become `sum(cast(a as bigint) + 10)`: apache#52837

sum(v + 2) => sum(v) + 2*count(v) sum(v - 2) => sum(v) - 2*count(v)

morrySnow reviewed Mar 14, 2024
View reviewed changes

Comment threadfe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewrite.java Outdated

morrySnow added the not-merge/2.0 do not merge into 2.0 branch label Mar 14, 2024

924060929 reviewed Mar 14, 2024
View reviewed changes

Comment threadfe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewrite.java Outdated

morrySnow reviewed Mar 15, 2024
View reviewed changes

Comment threadfe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewrite.java

keanji-x force-pushed the sum_literal branch from 186c58c to f5e2cbaCompare March 15, 2024 07:26

keanji-x force-pushed the sum_literal branch from f5e2cba to f375f08Compare March 15, 2024 08:12

morrySnow reviewed Mar 15, 2024
View reviewed changes

morrySnow previously approved these changes Mar 19, 2024
View reviewed changes

github-actionsBot added approved Indicates a PR has been approved by one committer. reviewed labels Mar 19, 2024

924060929 previously approved these changes Mar 19, 2024
View reviewed changes

wm1581066 approved these changes Mar 19, 2024
View reviewed changes

keanji-x dismissed stale reviews from morrySnow and 924060929 via 5336b48March 20, 2024 03:51

keanji-x force-pushed the sum_literal branch from 5db3d24 to 5336b48Compare March 20, 2024 03:51

github-actionsBot removed the approved Indicates a PR has been approved by one committer. label Mar 20, 2024

morrySnow previously approved these changes Mar 20, 2024
View reviewed changes

github-actionsBot added the approved Indicates a PR has been approved by one committer. label Mar 20, 2024

keanji-x dismissed morrySnow’s stale review via 7c24fddMarch 20, 2024 08:29

keanji-x force-pushed the sum_literal branch from 5336b48 to 7c24fddCompare March 20, 2024 08:29

github-actionsBot removed the approved Indicates a PR has been approved by one committer. label Mar 20, 2024

keanji-x added 4 commits March 21, 2024 10:05

rewrite sum literal
1eee608

add regress test
7e9e17e

change stream to for loop
b51149c

fix shape of clickbench
1875628

keanji-x force-pushed the sum_literal branch from 7c24fdd to 1875628Compare March 21, 2024 02:37

morrySnow approved these changes Mar 22, 2024
View reviewed changes

github-actionsBot added the approved Indicates a PR has been approved by one committer. label Mar 22, 2024

morrySnow merged commit 4d7be86 into apache:masterMar 22, 2024

yiguolei pushed a commit that referenced this pull request Mar 22, 2024
[feat](Nereids): rewrite sum literal to sum and count (#32244)
4de8775
sum(v + 2) => sum(v) + 2*count(v) sum(v - 2) => sum(v) - 2*count(v)

yujun777 mentioned this pull request Sep 1, 2025
[opt](nereids) sum literal rewrite rule support non slot expression #55543
Merged
16 tasks

HappenLee pushed a commit to HappenLee/incubator-doris that referenced this pull request Apr 24, 2026
[feat](Nereids): rewrite sum literal to sum and count (apache#32244)
89bbe6d
sum(v + 2) => sum(v) + 2*count(v) sum(v - 2) => sum(v) - 2*count(v)

Uh oh!

Conversation

keanji-x commented Mar 14, 2024

Proposed changes

Further comments

Uh oh!

doris-robot commented Mar 14, 2024

Uh oh!

keanji-x commented Mar 14, 2024

Uh oh!

doris-robot commented Mar 14, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

keanji-x commented Mar 15, 2024

Uh oh!

keanji-x commented Mar 15, 2024

Uh oh!

doris-robot commented Mar 15, 2024

Uh oh!

keanji-x commented Mar 15, 2024

Uh oh!

doris-robot commented Mar 15, 2024

Uh oh!

morrySnowMar 15, 2024

Choose a reason for hiding this comment

Uh oh!

keanji-xMar 19, 2024

Choose a reason for hiding this comment

Uh oh!

morrySnowMar 15, 2024

Choose a reason for hiding this comment

Uh oh!

github-actionsBot commented Mar 19, 2024

Uh oh!

github-actionsBot commented Mar 19, 2024

Uh oh!

keanji-x commented Mar 19, 2024

Uh oh!

keanji-x commented Mar 19, 2024

Uh oh!

wm1581066 left a comment

Choose a reason for hiding this comment

Uh oh!

keanji-x commented Mar 20, 2024

Uh oh!

github-actionsBot commented Mar 20, 2024

Uh oh!

keanji-x commented Mar 20, 2024

Uh oh!

keanji-x commented Mar 21, 2024

Uh oh!

keanji-x commented Mar 21, 2024

Uh oh!

github-actionsBot commented Mar 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants