Skip to content

[fix](array)fix array_except/union for left const return only one row result #36776

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
Merged
eldenmoon merged 1 commit into from
Jun 25, 2024
Merged

[fix](array)fix array_except/union for left const return only one row result #36776

eldenmoon merged 1 commit into from
Jun 25, 2024

Conversation

@amorynan
Copy link
Contributor

@amorynan amorynan commented Jun 24, 2024

Proposed changes

if left param in array_except/union is const, it will make ret column only has one row column which will make result wrong

Issue Number: close #xxx

@doris-robot
Copy link

doris-robot commented Jun 24, 2024

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.

@amorynan
Copy link
Contributor Author

amorynan commented Jun 24, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Jun 24, 2024

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

doris-robot commented Jun 24, 2024

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

------ Round 1 ----------------------------------
q1	17615	4956	4407	4407
q2	2005	192	189	189
q3	10677	1092	1108	1092
q4	10601	736	831	736
q5	7503	2668	2700	2668
q6	225	137	139	137
q7	980	600	626	600
q8	9276	2074	2072	2072
q9	9366	6608	6568	6568
q10	9773	3696	3732	3696
q11	457	252	253	252
q12	518	223	222	222
q13	17773	2983	3013	2983
q14	264	218	224	218
q15	528	471	464	464
q16	501	399	387	387
q17	972	661	677	661
q18	8072	7486	7466	7466
q19	2006	1494	1464	1464
q20	652	313	334	313
q21	4992	3195	3317	3195
q22	398	342	348	342
Total cold run time: 115154 ms
Total hot run time: 40132 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4415	4273	4257	4257
q2	367	275	254	254
q3	3004	2759	2871	2759
q4	1939	1766	1684	1684
q5	5574	5523	5492	5492
q6	218	128	135	128
q7	2182	1897	1820	1820
q8	3336	3433	3495	3433
q9	8645	8500	8602	8500
q10	4034	3848	3752	3752
q11	604	496	504	496
q12	805	656	671	656
q13	17364	3121	3171	3121
q14	337	282	281	281
q15	522	492	483	483
q16	491	427	457	427
q17	1838	1549	1522	1522
q18	8053	7857	7748	7748
q19	1782	1685	1593	1593
q20	2139	1860	1872	1860
q21	4917	4928	4994	4928
q22	619	594	582	582
Total cold run time: 73185 ms
Total hot run time: 55776 ms

@doris-robot
Copy link

doris-robot commented Jun 24, 2024

TPC-DS: Total hot run time: 173962 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 f5789fdd1dbbb591be448b66c52dd27ed0750e22, data reload: false

query1	936	388	379	379
query2	6352	2559	2341	2341
query3	6635	199	201	199
query4	19274	17263	17367	17263
query5	3533	497	464	464
query6	235	155	156	155
query7	4576	297	285	285
query8	329	314	294	294
query9	8483	2470	2456	2456
query10	543	303	293	293
query11	10769	10018	9998	9998
query12	111	85	82	82
query13	1630	387	358	358
query14	9960	7649	7402	7402
query15	234	190	189	189
query16	7702	276	266	266
query17	1877	527	500	500
query18	1848	273	285	273
query19	197	151	147	147
query20	92	81	77	77
query21	204	128	126	126
query22	4233	4208	3969	3969
query23	33663	33647	33553	33553
query24	11228	2965	2857	2857
query25	663	390	372	372
query26	1635	154	156	154
query27	2924	326	333	326
query28	7741	2159	2157	2157
query29	1038	647	613	613
query30	256	159	155	155
query31	938	776	778	776
query32	103	54	57	54
query33	766	309	304	304
query34	1000	481	475	475
query35	773	629	637	629
query36	1156	995	960	960
query37	173	74	74	74
query38	2958	2805	2817	2805
query39	884	830	843	830
query40	256	126	123	123
query41	54	54	51	51
query42	119	95	96	95
query43	591	586	582	582
query44	1167	736	733	733
query45	189	166	158	158
query46	1066	724	744	724
query47	1883	1766	1731	1731
query48	369	304	305	304
query49	919	412	426	412
query50	780	398	389	389
query51	6870	6769	6685	6685
query52	114	92	92	92
query53	367	324	293	293
query54	931	447	440	440
query55	77	75	74	74
query56	280	260	267	260
query57	1140	1079	1043	1043
query58	247	251	241	241
query59	3521	3214	3168	3168
query60	316	264	272	264
query61	90	88	102	88
query62	632	441	443	441
query63	324	315	293	293
query64	9719	2272	1742	1742
query65	3172	3198	3111	3111
query66	1247	322	334	322
query67	15392	15057	14884	14884
query68	4542	552	544	544
query69	584	445	385	385
query70	1172	1106	1116	1106
query71	424	281	272	272
query72	7692	5491	5210	5210
query73	751	327	327	327
query74	5903	5431	5448	5431
query75	3419	2710	2676	2676
query76	3002	946	904	904
query77	593	301	312	301
query78	10339	9779	9834	9779
query79	2733	516	504	504
query80	1126	491	463	463
query81	565	215	221	215
query82	780	109	104	104
query83	262	171	170	170
query84	237	92	86	86
query85	1503	284	289	284
query86	463	305	322	305
query87	3268	3061	3076	3061
query88	3724	2449	2442	2442
query89	467	385	409	385
query90	1808	191	191	191
query91	126	100	102	100
query92	64	53	48	48
query93	1820	523	524	523
query94	1177	195	194	194
query95	401	329	326	326
query96	593	277	274	274
query97	3263	2996	3085	2996
query98	225	200	204	200
query99	1101	803	847	803
Total cold run time: 273611 ms
Total hot run time: 173962 ms

@doris-robot
Copy link

doris-robot commented Jun 24, 2024

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

query1	0.04	0.03	0.04
query2	0.07	0.04	0.04
query3	0.22	0.05	0.04
query4	1.68	0.08	0.08
query5	0.48	0.49	0.49
query6	1.13	0.74	0.72
query7	0.02	0.02	0.01
query8	0.05	0.04	0.04
query9	0.54	0.48	0.48
query10	0.55	0.55	0.54
query11	0.15	0.11	0.11
query12	0.14	0.13	0.12
query13	0.59	0.59	0.58
query14	0.76	0.78	0.79
query15	0.84	0.84	0.81
query16	0.37	0.35	0.37
query17	1.03	1.03	1.00
query18	0.21	0.23	0.23
query19	1.78	1.65	1.70
query20	0.01	0.01	0.01
query21	15.41	0.67	0.67
query22	4.51	6.89	2.53
query23	18.29	1.34	1.26
query24	2.06	0.23	0.22
query25	0.14	0.10	0.08
query26	0.25	0.18	0.18
query27	0.07	0.08	0.09
query28	13.26	1.01	1.00
query29	12.61	3.22	3.24
query30	0.26	0.06	0.05
query31	2.87	0.38	0.38
query32	3.28	0.47	0.47
query33	2.89	2.94	2.89
query34	17.11	4.42	4.45
query35	4.51	4.44	4.53
query36	0.65	0.47	0.50
query37	0.19	0.16	0.16
query38	0.16	0.14	0.14
query39	0.04	0.04	0.03
query40	0.18	0.14	0.14
query41	0.09	0.04	0.06
query42	0.06	0.05	0.04
query43	0.04	0.04	0.04
Total cold run time: 109.59 s
Total hot run time: 31.06 s

zhiqiang-hhhh
zhiqiang-hhhh reviewed Jun 24, 2024
View reviewed changes
be/src/vec/functions/array/function_array_set.h
size_t current = 0;
Impl impl;
for (size_t row = 0; row < left_data.offsets_ptr->size(); ++row) {
size_t row_size = left_data.offsets_ptr->size();
Copy link
Contributor

@zhiqiang-hhhh zhiqiang-hhhh Jun 24, 2024

Choose a reason for hiding this comment

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

what if right const

Copy link
Contributor Author

@amorynan amorynan Jun 25, 2024

Choose a reason for hiding this comment

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

right and left is all not const ==> left_data.offsets_ptr->size()
right const && left is not const ==> left_data.offsets_ptr->size()
right is not const && left is const ==> right.offsets_ptr->size()
right and left is all const ==> left_data.offsets_ptr->size()

@github-actions
Copy link
Contributor

github-actions bot commented Jun 25, 2024

PR approved by anyone and no changes requested.

eldenmoon
eldenmoon approved these changes Jun 25, 2024
View reviewed changes
Copy link
Member

@eldenmoon eldenmoon 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 Jun 25, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Jun 25, 2024

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

@eldenmoon eldenmoon merged commit c938845 into apache:master Jun 25, 2024
dataroaring pushed a commit that referenced this pull request Jun 26, 2024
@dataroaring
[fix](array)fix array_except/union for left const return only one row…
3d5d763 
… result (#36776)

if left param in array_except/union is const, it will make ret column
only has one row column which will make result wrong
This was referenced Jun 27, 2024
Merged
Merged
xiaokang pushed a commit that referenced this pull request Jun 30, 2024
[fix](array)fix array_except/union for left const return only one row…
d237a4d 
… result #36776 (#36986)
xiaokang pushed a commit that referenced this pull request Jul 2, 2024
[fix](array) fix array_except/union for left const return only one ro…
0c9d20c 
…w result #36776 (#36934)
mongo360 pushed a commit to mongo360/doris that referenced this pull request Aug 16, 2024
[fix](array) fix array_except/union for left const return only one ro…
88742fa 
…w result apache#36776 (apache#36934)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eldenmoon eldenmoon eldenmoon approved these changes

+1 more reviewer

@zhiqiang-hhhh zhiqiang-hhhh zhiqiang-hhhh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. dev/2.0.13-merged dev/2.1.5-merged dev/3.0.0-merged reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@amorynan @doris-robot @zhiqiang-hhhh @eldenmoon @xiaokang @dataroaring