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

[Fix](nereids) fix NormalizeAgg, change the upper project projections rewrite logic #36623

Merged
merged 2 commits into from
Jun 27, 2024

Conversation

feiniaofeiafei
Copy link
Contributor

cherry-pick #36161 to branch-2.0
NormalizeAggregate rewrite logic has a bug, for sql like this:

SELECT
	CASE
		1 WHEN CAST( NULL AS SIGNED ) THEN NULL
		WHEN COUNT( DISTINCT CAST( NULL AS SIGNED ) ) THEN NULL
		ELSE null
	END ;

This is the plan after NormalizeAggregate, the LogicalAggregate only output count(DISTINCT cast(NULL as SIGNED))#3, do not output cast(NULL as SIGNED)#2, but the upper project use cast(NULL as SIGNED)#2, so Doris report error "cast(NULL as SIGNED) not in aggregate's output".

LogicalResultSink[29] ( outputExprs=[__case_when_0#1] ) +--LogicalProject[26] ( distinct=false, projects=[CASE WHEN (1 = cast(NULL as SIGNED)#2) THEN NULL WHEN (1 = count(DISTINCT cast(NULL as SIGNED))#3) THEN NULL ELSE NULL END AS `CASE WHEN (1 = cast(NULL as SIGNED)) THEN NULL WHEN (1 = count(DISTINCT cast(NULL as SIGNED))) THEN NULL ELSE NULL END`#1], excepts=[] )
   +--LogicalAggregate[25] ( groupByExpr=[], outputExpr=[count(DISTINCT cast(NULL as SIGNED)#2) AS `count(DISTINCT cast(NULL as SIGNED))`#3], hasRepeat=false )
      +--LogicalProject[24] ( distinct=false, projects=[cast(NULL as SIGNED) AS `cast(NULL as SIGNED)`#2], excepts=[] )
         +--LogicalOneRowRelation ( projects=[0 AS `0`#0] )

The problem is that the cast(NULL as SIGNED)#2 should not outputted by LogicalAggregate, cast(NULL as SIGNED) should be computed in LogicalProject.
This pr change the upper project projections rewrite logic: aggregateOutputs is rewritten and become the upper-level LogicalProject projections. During the rewriting process, the expressions inside the agg function can be rewritten with expressions in aggregate function arguments and group by expressions, but the ones outside the agg function can only be rewritten with group by expressions.

Proposed changes

Issue Number: close #xxx

… rewrite logic (apache#36161)

NormalizeAggregate rewrite logic has a bug, for sql like this:

SELECT
	CASE
		1 WHEN CAST( NULL AS SIGNED ) THEN NULL
		WHEN COUNT( DISTINCT CAST( NULL AS SIGNED ) ) THEN NULL
		ELSE null
	END ;

This is the plan after NormalizeAggregate, the LogicalAggregate only
output `count(DISTINCT cast(NULL as SIGNED))`apache#3, do not output cast(NULL
as SIGNED)apache#2, but the upper project use cast(NULL as SIGNED)apache#2, so Doris
report error "cast(NULL as SIGNED) not in aggregate's output".

LogicalResultSink[29] ( outputExprs=[__case_when_0#1] )
+--LogicalProject[26] ( distinct=false, projects=[CASE WHEN (1 = cast(NULL as SIGNED)apache#2) THEN NULL WHEN (1 = count(DISTINCT cast(NULL as SIGNED))apache#3) THEN NULL ELSE NULL END AS `CASE WHEN (1 = cast(NULL as SIGNED)) THEN NULL WHEN (1 = count(DISTINCT cast(NULL as SIGNED))) THEN NULL ELSE NULL END`apache#1], excepts=[] )
   +--LogicalAggregate[25] ( groupByExpr=[], outputExpr=[count(DISTINCT cast(NULL as SIGNED)apache#2) AS `count(DISTINCT cast(NULL as SIGNED))`apache#3], hasRepeat=false )
      +--LogicalProject[24] ( distinct=false, projects=[cast(NULL as SIGNED) AS `cast(NULL as SIGNED)`apache#2], excepts=[] )
         +--LogicalOneRowRelation ( projects=[0 AS `0`#0] )

The problem is that the cast(NULL as SIGNED)apache#2 should not outputted by
LogicalAggregate, cast(NULL as SIGNED) should be computed in
LogicalProject.
This pr change the upper project projections rewrite logic:
aggregateOutputs is rewritten and become the upper-level LogicalProject
projections. During the rewriting process, the expressions inside the
agg function can be rewritten with expressions in aggregate function
arguments and group by expressions, but the ones outside the agg
function can only be rewritten with group by expressions.
@doris-robot
Copy link

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

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TPC-H: Total hot run time: 49790 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 40774dcad6d43255171933574726a85db6d751e5, data reload: false

------ Round 1 ----------------------------------
q1	18128	4348	4324	4324
q2	2067	154	148	148
q3	10405	1885	1932	1885
q4	10348	1264	1325	1264
q5	8491	3847	3938	3847
q6	233	152	125	125
q7	2071	1592	1601	1592
q8	9519	2738	2697	2697
q9	11295	10306	10224	10224
q10	8623	3539	3531	3531
q11	416	243	243	243
q12	474	315	315	315
q13	18346	3962	4023	3962
q14	355	327	338	327
q15	507	470	456	456
q16	670	578	583	578
q17	1141	959	963	959
q18	7386	6811	6954	6811
q19	1807	1628	1627	1627
q20	551	324	305	305
q21	4431	4122	4139	4122
q22	545	454	448	448
Total cold run time: 117809 ms
Total hot run time: 49790 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4353	4317	4308	4308
q2	314	227	227	227
q3	4146	4155	4150	4150
q4	2773	2746	2714	2714
q5	7099	7030	7024	7024
q6	240	120	117	117
q7	3207	2825	2843	2825
q8	4401	4463	4459	4459
q9	16755	16796	16676	16676
q10	4259	4289	4295	4289
q11	741	704	655	655
q12	1042	826	873	826
q13	6933	3752	3754	3752
q14	451	449	431	431
q15	509	466	462	462
q16	747	711	681	681
q17	3796	3833	3851	3833
q18	8871	8850	8859	8850
q19	1723	1718	1690	1690
q20	2429	2153	2135	2135
q21	8454	8510	8436	8436
q22	1029	981	955	955
Total cold run time: 84272 ms
Total hot run time: 79495 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 204909 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 40774dcad6d43255171933574726a85db6d751e5, data reload: false

query1	916	423	375	375
query2	6537	2774	2799	2774
query3	6915	213	201	201
query4	20101	18112	17902	17902
query5	19727	6526	6485	6485
query6	298	218	235	218
query7	4158	300	295	295
query8	418	439	436	436
query9	3085	2675	2602	2602
query10	430	306	290	290
query11	11270	10767	10592	10592
query12	125	73	77	73
query13	5598	677	714	677
query14	17383	13719	13589	13589
query15	361	238	243	238
query16	6457	300	268	268
query17	1707	1447	861	861
query18	2311	422	415	415
query19	205	150	151	150
query20	79	78	77	77
query21	201	102	95	95
query22	5155	4920	4963	4920
query23	32501	31845	32069	31845
query24	7041	6463	6436	6436
query25	532	434	423	423
query26	525	161	161	161
query27	1901	295	296	295
query28	6159	2340	2296	2296
query29	2952	2678	2827	2678
query30	240	168	171	168
query31	904	742	746	742
query32	68	64	59	59
query33	406	250	262	250
query34	847	483	472	472
query35	1106	962	885	885
query36	1275	1199	1183	1183
query37	86	60	59	59
query38	3081	2894	2966	2894
query39	1378	1330	1342	1330
query40	206	93	98	93
query41	46	43	45	43
query42	84	84	75	75
query43	763	705	746	705
query44	1137	730	738	730
query45	253	236	239	236
query46	1238	981	987	981
query47	1823	1659	1597	1597
query48	1005	726	725	725
query49	622	376	375	375
query50	854	620	602	602
query51	4696	4711	4646	4646
query52	91	82	86	82
query53	450	319	328	319
query54	2652	2465	2470	2465
query55	89	76	84	76
query56	257	214	212	212
query57	1104	1087	1108	1087
query58	218	210	211	210
query59	4420	4082	4017	4017
query60	212	209	198	198
query61	96	93	97	93
query62	809	483	502	483
query63	490	342	359	342
query64	2590	1586	1500	1500
query65	3678	3651	3626	3626
query66	800	382	378	378
query67	16002	17040	16246	16246
query68	8931	675	651	651
query69	559	344	348	344
query70	1652	1624	1633	1624
query71	411	302	327	302
query72	6596	3506	3465	3465
query73	735	316	315	315
query74	6274	5829	5960	5829
query75	5429	3701	3673	3673
query76	5381	1149	1153	1149
query77	942	250	256	250
query78	12467	11720	11729	11720
query79	7816	620	626	620
query80	1338	411	399	399
query81	485	236	232	232
query82	1660	104	101	101
query83	161	134	129	129
query84	265	71	70	70
query85	880	314	317	314
query86	327	284	287	284
query87	3315	2995	2980	2980
query88	4672	2281	2281	2281
query89	389	281	276	276
query90	1962	205	211	205
query91	173	140	138	138
query92	58	56	54	54
query93	6140	558	566	558
query94	714	211	206	206
query95	1095	1053	1064	1053
query96	626	321	318	318
query97	6419	6423	6452	6423
query98	192	177	178	177
query99	3063	1016	918	918
Total cold run time: 312251 ms
Total hot run time: 204909 ms

@doris-robot
Copy link

ClickBench: Total hot run time: 31.38 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 40774dcad6d43255171933574726a85db6d751e5, data reload: false

query1	0.03	0.03	0.02
query2	0.07	0.03	0.02
query3	0.25	0.05	0.05
query4	1.79	0.11	0.10
query5	0.55	0.52	0.52
query6	1.23	0.60	0.59
query7	0.01	0.01	0.01
query8	0.03	0.03	0.02
query9	0.54	0.49	0.47
query10	0.54	0.53	0.53
query11	0.12	0.09	0.10
query12	0.11	0.09	0.10
query13	0.63	0.62	0.62
query14	0.80	0.78	0.78
query15	0.78	0.76	0.76
query16	0.37	0.37	0.36
query17	1.02	1.01	0.98
query18	0.23	0.26	0.26
query19	1.92	1.90	1.82
query20	0.02	0.01	0.01
query21	15.49	0.54	0.55
query22	1.98	1.95	1.48
query23	17.13	1.13	0.96
query24	4.65	1.88	1.59
query25	0.37	0.13	0.06
query26	0.63	0.16	0.17
query27	0.04	0.04	0.04
query28	6.99	0.80	0.77
query29	12.64	2.32	2.34
query30	0.62	0.61	0.53
query31	2.81	0.39	0.38
query32	3.38	0.51	0.50
query33	3.09	3.05	3.10
query34	15.24	4.82	4.79
query35	4.86	4.84	4.85
query36	1.05	1.02	1.03
query37	0.06	0.05	0.04
query38	0.03	0.02	0.02
query39	0.02	0.02	0.01
query40	0.17	0.14	0.14
query41	0.06	0.02	0.01
query42	0.02	0.01	0.01
query43	0.03	0.02	0.02
Total cold run time: 102.4 s
Total hot run time: 31.38 s

@doris-robot
Copy link

Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'

Load test result on commit 40774dcad6d43255171933574726a85db6d751e5 with default session variables
Stream load json:         19 seconds loaded 2358488459 Bytes, about 118 MB/s
Stream load orc:          59 seconds loaded 1101869774 Bytes, about 17 MB/s
Stream load parquet:      32 seconds loaded 861443392 Bytes, about 25 MB/s
Insert into select:       24.1 seconds inserted 10000000 Rows, about 414K ops/s

@morrySnow morrySnow changed the title [Fix](nereids) fix NormalizeAgg, change the upper project projections rewrite logic (#36161) [Fix](nereids) fix NormalizeAgg, change the upper project projections rewrite logic Jun 24, 2024
@morrySnow morrySnow merged commit 65255cd into apache:branch-2.0 Jun 27, 2024
24 of 25 checks passed
mongo360 pushed a commit to mongo360/doris that referenced this pull request Aug 16, 2024
… rewrite logic (apache#36623)

cherry-pick apache#36161 to branch-2.0

NormalizeAggregate rewrite logic has a bug, for sql like this:

SELECT
	CASE
		1 WHEN CAST( NULL AS SIGNED ) THEN NULL
		WHEN COUNT( DISTINCT CAST( NULL AS SIGNED ) ) THEN NULL
		ELSE null
	END ;

This is the plan after NormalizeAggregate, the LogicalAggregate only
output `count(DISTINCT cast(NULL as SIGNED))`apache#3, do not output cast(NULL
as SIGNED)apache#2, but the upper project use cast(NULL as SIGNED)apache#2, so Doris
report error "cast(NULL as SIGNED) not in aggregate's output".

LogicalResultSink[29] ( outputExprs=[__case_when_0#1] ) +--LogicalProject[26] ( distinct=false, projects=[CASE WHEN (1 = cast(NULL as SIGNED)apache#2) THEN NULL WHEN (1 = count(DISTINCT cast(NULL as SIGNED))apache#3) THEN NULL ELSE NULL END AS `CASE WHEN (1 = cast(NULL as SIGNED)) THEN NULL WHEN (1 = count(DISTINCT cast(NULL as SIGNED))) THEN NULL ELSE NULL END`apache#1], excepts=[] )
   +--LogicalAggregate[25] ( groupByExpr=[], outputExpr=[count(DISTINCT cast(NULL as SIGNED)apache#2) AS `count(DISTINCT cast(NULL as SIGNED))`apache#3], hasRepeat=false )
      +--LogicalProject[24] ( distinct=false, projects=[cast(NULL as SIGNED) AS `cast(NULL as SIGNED)`apache#2], excepts=[] )
         +--LogicalOneRowRelation ( projects=[0 AS `0`#0] )

The problem is that the cast(NULL as SIGNED)apache#2 should not outputted by
LogicalAggregate, cast(NULL as SIGNED) should be computed in
LogicalProject.
This pr change the upper project projections rewrite logic:
aggregateOutputs is rewritten and become the upper-level LogicalProject
projections. During the rewriting process, the expressions inside the
agg function can be rewritten with expressions in aggregate function
arguments and group by expressions, but the ones outside the agg
function can only be rewritten with group by expressions.

---------

Co-authored-by: moailing <moailing@selectdb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants