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.

Sign up for GitHub

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

Jump to bottom

[Fix](inverted index) fix wrong opt for count_on_index #41127

Merged
merged 1 commit into from
Sep 23, 2024
Merged

[Fix](inverted index) fix wrong opt for count_on_index #41127

merged 1 commit into from
Sep 23, 2024

Conversation

airborne12
Copy link
Member

Proposed changes

The pushdownCountOnIndex optimization rule in AggregateStrategies was incorrectly applied to COUNT functions with complex child expressions, such as COUNT(CASE WHEN ...). This led to incorrect query results.

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

@airborne12
Copy link
Member Author

run buildall

@wm1581066 wm1581066 added the p0_w label Sep 23, 2024
@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17626	7440	7321	7321
q2	2029	154	153	153
q3	10595	1056	1167	1056
q4	10221	814	638	638
q5	7744	3084	3061	3061
q6	244	153	150	150
q7	1064	629	608	608
q8	9434	2038	2060	2038
q9	6813	6417	6446	6417
q10	7031	2279	2295	2279
q11	438	251	239	239
q12	404	209	216	209
q13	17776	2961	2968	2961
q14	244	203	209	203
q15	572	526	516	516
q16	694	629	618	618
q17	979	828	858	828
q18	7312	6715	6681	6681
q19	1421	1102	1077	1077
q20	587	293	281	281
q21	3981	3660	3326	3326
q22	1102	991	1004	991
Total cold run time: 108311 ms
Total hot run time: 41651 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7297	7274	7235	7235
q2	332	233	225	225
q3	2982	2943	2978	2943
q4	2050	1817	1804	1804
q5	5647	5638	5626	5626
q6	241	144	145	144
q7	2405	1793	1788	1788
q8	3347	3443	3731	3443
q9	8848	8983	8827	8827
q10	3510	3443	3509	3443
q11	578	492	501	492
q12	818	614	633	614
q13	10427	3115	3131	3115
q14	318	270	274	270
q15	573	531	520	520
q16	701	666	665	665
q17	1795	1592	1581	1581
q18	8250	7769	7837	7769
q19	1739	1538	1436	1436
q20	2177	1906	1880	1880
q21	5519	5329	5362	5329
q22	1160	1038	1037	1037
Total cold run time: 70714 ms
Total hot run time: 60186 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 195263 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 8aa259660cf0db6dadcfba552cfec038f7da4d29, data reload: false

query1	1263	866	895	866
query2	6414	1979	2011	1979
query3	10768	3892	3961	3892
query4	64524	29528	23548	23548
query5	5091	476	470	470
query6	396	181	157	157
query7	5502	315	300	300
query8	317	223	223	223
query9	8445	2614	2587	2587
query10	462	314	265	265
query11	17737	15279	15840	15279
query12	159	104	105	104
query13	1480	423	411	411
query14	10916	7648	7254	7254
query15	217	179	179	179
query16	6780	521	510	510
query17	1152	628	608	608
query18	1719	315	322	315
query19	223	163	151	151
query20	126	119	110	110
query21	214	110	107	107
query22	4671	4675	4610	4610
query23	34353	34752	34043	34043
query24	6014	2898	2871	2871
query25	526	410	409	409
query26	660	158	158	158
query27	1658	282	290	282
query28	4314	2458	2417	2417
query29	671	431	431	431
query30	228	163	156	156
query31	980	816	794	794
query32	75	55	56	55
query33	388	297	294	294
query34	893	515	487	487
query35	835	723	728	723
query36	1050	965	927	927
query37	152	92	83	83
query38	4009	3950	4005	3950
query39	1494	1411	1430	1411
query40	206	94	96	94
query41	50	48	49	48
query42	112	95	97	95
query43	522	479	477	477
query44	1167	823	804	804
query45	199	168	171	168
query46	1124	760	767	760
query47	1949	1836	1894	1836
query48	455	378	373	373
query49	708	420	408	408
query50	834	415	427	415
query51	6927	6839	6836	6836
query52	102	87	84	84
query53	247	176	178	176
query54	573	446	440	440
query55	79	77	82	77
query56	266	249	253	249
query57	1227	1108	1069	1069
query58	223	260	223	223
query59	3033	2913	2736	2736
query60	279	264	272	264
query61	103	107	107	107
query62	763	653	646	646
query63	214	183	179	179
query64	1595	675	636	636
query65	3270	3160	3210	3160
query66	679	309	292	292
query67	15808	15495	15269	15269
query68	3830	597	574	574
query69	521	299	314	299
query70	1120	1052	1125	1052
query71	395	270	275	270
query72	7469	4129	3985	3985
query73	776	326	332	326
query74	9946	8991	9010	8991
query75	3407	2659	2671	2659
query76	2290	928	982	928
query77	703	288	293	288
query78	9993	9115	9143	9115
query79	1724	532	539	532
query80	1032	442	443	442
query81	539	239	251	239
query82	395	146	151	146
query83	332	136	138	136
query84	289	80	78	78
query85	1029	281	276	276
query86	386	309	297	297
query87	4584	4358	4397	4358
query88	3713	2326	2295	2295
query89	398	284	293	284
query90	2130	189	190	189
query91	181	145	144	144
query92	63	50	47	47
query93	2521	542	536	536
query94	949	306	283	283
query95	348	248	250	248
query96	609	279	283	279
query97	3256	3202	3141	3141
query98	215	213	210	210
query99	1560	1291	1279	1279
Total cold run time: 316293 ms
Total hot run time: 195263 ms

@doris-robot
Copy link

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

query1	0.04	0.05	0.04
query2	0.06	0.03	0.03
query3	0.23	0.06	0.06
query4	1.65	0.10	0.10
query5	0.48	0.52	0.49
query6	1.15	0.73	0.72
query7	0.02	0.01	0.02
query8	0.04	0.04	0.03
query9	0.57	0.51	0.50
query10	0.55	0.54	0.55
query11	0.15	0.11	0.11
query12	0.13	0.11	0.11
query13	0.60	0.59	0.59
query14	3.04	2.97	2.98
query15	0.89	0.81	0.82
query16	0.37	0.37	0.39
query17	1.08	1.06	1.05
query18	0.19	0.19	0.19
query19	1.92	1.86	1.97
query20	0.02	0.01	0.01
query21	15.35	0.56	0.59
query22	2.68	2.97	2.02
query23	17.34	0.85	0.85
query24	2.94	1.26	0.86
query25	0.30	0.13	0.05
query26	0.46	0.15	0.13
query27	0.04	0.03	0.04
query28	10.82	1.09	1.06
query29	12.54	3.26	3.26
query30	0.25	0.06	0.05
query31	2.87	0.38	0.38
query32	3.30	0.48	0.46
query33	2.97	3.04	3.05
query34	17.09	4.40	4.40
query35	4.40	4.40	4.42
query36	0.69	0.51	0.48
query37	0.09	0.07	0.05
query38	0.05	0.03	0.03
query39	0.03	0.03	0.03
query40	0.15	0.14	0.13
query41	0.07	0.02	0.02
query42	0.04	0.02	0.02
query43	0.03	0.03	0.03
Total cold run time: 107.68 s
Total hot run time: 32.93 s

xiaokang
xiaokang approved these changes Sep 23, 2024
View reviewed changes
Copy link
Contributor

@xiaokang xiaokang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Sep 23, 2024
@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

csun5285
csun5285 approved these changes Sep 23, 2024
View reviewed changes
Copy link
Contributor

@csun5285 csun5285 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@airborne12 airborne12 merged commit c2e4d97 into apache:master Sep 23, 2024
28 of 33 checks passed
@airborne12 airborne12 deleted the fix-count-on-index branch September 23, 2024 10:08
This was referenced Sep 23, 2024
Merged
Merged
airborne12 added a commit to airborne12/apache-doris that referenced this pull request Sep 23, 2024
@airborne12
[Fix](inverted index) fix wrong opt for count_on_index (apache#41127)
4dca57e 
## Proposed changes

The pushdownCountOnIndex optimization rule in AggregateStrategies was
incorrectly applied to COUNT functions with complex child expressions,
such as COUNT(CASE WHEN ...). This led to incorrect query results.
airborne12 added a commit to airborne12/apache-doris that referenced this pull request Sep 23, 2024
@airborne12
[Fix](inverted index) fix wrong opt for count_on_index (apache#41127)
0b54920 
## Proposed changes

The pushdownCountOnIndex optimization rule in AggregateStrategies was
incorrectly applied to COUNT functions with complex child expressions,
such as COUNT(CASE WHEN ...). This led to incorrect query results.
@airborne12 airborne12 mentioned this pull request Sep 23, 2024
Merged
airborne12 added a commit to airborne12/apache-doris that referenced this pull request Sep 23, 2024
@airborne12
[Fix](inverted index) fix wrong opt for count_on_index (apache#41127)
4ccb606 
## Proposed changes

The pushdownCountOnIndex optimization rule in AggregateStrategies was
incorrectly applied to COUNT functions with complex child expressions,
such as COUNT(CASE WHEN ...). This led to incorrect query results.
airborne12 added a commit that referenced this pull request Sep 23, 2024
@airborne12
[Fix](inverted index) fix wrong opt for count_on_index #41127 (#41152)
c6a7436 
cherry pick from #41127
airborne12 added a commit that referenced this pull request Sep 23, 2024
@airborne12
[Fix](inverted index) fix wrong opt for count_on_index #41127 (#41153)
5418807 
cherry pick from #41127
airborne12 added a commit that referenced this pull request Sep 23, 2024
@airborne12
[Fix](inverted index) fix wrong opt for count_on_index #41127 (#41154)
48e60f3 
cherry pick from #41127
@yiguolei yiguolei mentioned this pull request Nov 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@englefly englefly englefly left review comments

@csun5285 csun5285 csun5285 approved these changes

@xiaokang xiaokang xiaokang approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/2.0.16-merged dev/2.1.7-merged dev/3.0.2-merged p0_w reviewed usercase Important user case type label
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@airborne12 @doris-robot @xiaokang @englefly @csun5285 @wm1581066