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](load) fix memtable agg functions #38017

Merged
merged 3 commits into from
Jul 18, 2024

Conversation

kaijchen
Copy link
Contributor

Proposed changes

fix init agg functions in memtable.

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

@liaoxin01
Copy link
Contributor

run buildall

Copy link
Contributor

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

@@ -187,7 +187,7 @@ Status MemTable::insert(const vectorized::Block* input_block,
_vec_row_comparator->set_block(&_input_mutable_block);
_output_mutable_block = vectorized::MutableBlock::build_mutable_block(&cloneBlock);
if (_keys_type != KeysType::DUP_KEYS) {
_init_agg_functions(input_block);
_init_agg_functions(&cloneBlock);
Copy link
Contributor

Choose a reason for hiding this comment

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

add comment here, to explain why cloned block not input block

@@ -187,6 +187,8 @@ Status MemTable::insert(const vectorized::Block* input_block,
_vec_row_comparator->set_block(&_input_mutable_block);
_output_mutable_block = vectorized::MutableBlock::build_mutable_block(&cloneBlock);
if (_keys_type != KeysType::DUP_KEYS) {
// there may be additional intermediate columns in input_block
// we only need columns indicated by column offset in the output
_init_agg_functions(&cloneBlock);
Copy link
Contributor

Choose a reason for hiding this comment

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

cloneBlock -> clone_block

Copy link
Contributor

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17628	7133	4400	4400
q2	2029	194	199	194
q3	10578	1212	1118	1118
q4	10311	743	804	743
q5	7506	2761	2679	2679
q6	225	142	139	139
q7	966	605	604	604
q8	9224	2122	2088	2088
q9	8760	6591	6615	6591
q10	8667	3809	3795	3795
q11	453	239	245	239
q12	413	228	229	228
q13	17769	2981	3015	2981
q14	281	242	244	242
q15	530	484	490	484
q16	509	403	384	384
q17	982	712	714	712
q18	8205	7459	7479	7459
q19	1616	1436	1478	1436
q20	669	330	321	321
q21	4963	3186	3150	3150
q22	356	286	288	286
Total cold run time: 112640 ms
Total hot run time: 40273 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4320	4243	4291	4243
q2	368	280	259	259
q3	3019	2811	2738	2738
q4	1864	1609	1618	1609
q5	5339	5342	5302	5302
q6	221	130	131	130
q7	2110	1761	1744	1744
q8	3220	3346	3343	3343
q9	8574	8429	8421	8421
q10	3927	3704	3754	3704
q11	577	495	483	483
q12	772	591	594	591
q13	17504	2990	2992	2990
q14	298	275	281	275
q15	513	470	482	470
q16	468	426	420	420
q17	1812	1495	1465	1465
q18	7670	7512	7218	7218
q19	1651	1517	1562	1517
q20	1957	1790	1786	1786
q21	4798	4704	4627	4627
q22	596	494	506	494
Total cold run time: 71578 ms
Total hot run time: 53829 ms

@kaijchen
Copy link
Contributor Author

run buildall

Copy link
Contributor

@liaoxin01 liaoxin01 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 Jul 17, 2024
Copy link
Contributor

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

Copy link
Contributor

PR approved by anyone and no changes requested.

Copy link
Contributor

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17612	4417	4322	4322
q2	2031	195	188	188
q3	10465	1238	1159	1159
q4	10191	812	843	812
q5	7612	2834	2716	2716
q6	225	137	135	135
q7	967	602	607	602
q8	9218	2123	2071	2071
q9	9964	6605	6641	6605
q10	8844	3860	3792	3792
q11	531	238	245	238
q12	415	233	229	229
q13	17762	3005	2997	2997
q14	275	232	247	232
q15	523	483	482	482
q16	501	385	384	384
q17	978	611	664	611
q18	8112	7526	7436	7436
q19	1625	1407	1342	1342
q20	674	330	327	327
q21	4924	3209	3283	3209
q22	360	287	287	287
Total cold run time: 113809 ms
Total hot run time: 40176 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4289	4246	4231	4231
q2	369	266	272	266
q3	3039	2757	2805	2757
q4	1893	1591	1628	1591
q5	5308	5330	5314	5314
q6	219	134	131	131
q7	2120	1690	1753	1690
q8	3217	3390	3338	3338
q9	8461	8439	8439	8439
q10	3894	3716	3716	3716
q11	580	479	486	479
q12	765	626	599	599
q13	15054	2999	2986	2986
q14	297	278	280	278
q15	519	475	471	471
q16	479	419	428	419
q17	1761	1513	1464	1464
q18	7815	7680	7470	7470
q19	1658	1444	1513	1444
q20	1966	1796	1783	1783
q21	4819	4669	4753	4669
q22	601	519	493	493
Total cold run time: 69123 ms
Total hot run time: 54028 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 172571 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 22f7becb128ee697da3910bb1308d886e3c60015, data reload: false

query1	913	376	367	367
query2	6447	1946	1826	1826
query3	6649	214	225	214
query4	28432	17533	17349	17349
query5	4215	478	479	478
query6	264	182	160	160
query7	4583	303	299	299
query8	240	198	194	194
query9	8583	2365	2369	2365
query10	449	286	267	267
query11	11368	10059	10168	10059
query12	157	85	79	79
query13	1628	355	362	355
query14	9571	8022	7758	7758
query15	213	166	165	165
query16	7148	306	311	306
query17	1814	549	540	540
query18	1721	281	274	274
query19	195	149	148	148
query20	89	81	83	81
query21	209	122	127	122
query22	4281	4144	3963	3963
query23	33697	33167	33064	33064
query24	11142	2818	2929	2818
query25	635	368	389	368
query26	1144	148	152	148
query27	2777	272	278	272
query28	7256	2001	1982	1982
query29	875	628	603	603
query30	284	152	149	149
query31	974	719	759	719
query32	89	52	59	52
query33	767	303	296	296
query34	934	507	494	494
query35	673	551	576	551
query36	1116	902	917	902
query37	147	82	80	80
query38	2883	2735	2759	2735
query39	859	832	842	832
query40	270	121	121	121
query41	46	44	44	44
query42	123	98	103	98
query43	525	462	464	462
query44	1251	741	719	719
query45	190	160	163	160
query46	1092	753	757	753
query47	1847	1767	1761	1761
query48	370	295	295	295
query49	1063	424	408	408
query50	790	397	405	397
query51	6886	6834	6815	6815
query52	103	94	92	92
query53	357	284	306	284
query54	1023	445	448	445
query55	76	78	76	76
query56	290	267	284	267
query57	1165	1057	1048	1048
query58	287	252	242	242
query59	2824	2522	2703	2522
query60	306	279	272	272
query61	116	91	98	91
query62	821	655	646	646
query63	331	291	294	291
query64	10650	2253	1734	1734
query65	3223	3122	3089	3089
query66	1425	346	327	327
query67	15469	14967	14855	14855
query68	4534	544	556	544
query69	475	332	323	323
query70	1179	1074	1128	1074
query71	401	277	282	277
query72	6810	5555	5849	5555
query73	747	334	325	325
query74	6138	5630	5667	5630
query75	3395	2728	2695	2695
query76	2559	999	977	977
query77	470	324	339	324
query78	10204	8969	9007	8969
query79	2275	537	529	529
query80	2266	546	470	470
query81	567	218	218	218
query82	745	136	137	136
query83	283	172	173	172
query84	256	92	86	86
query85	2192	303	297	297
query86	464	312	306	306
query87	3273	3060	3113	3060
query88	3920	2353	2374	2353
query89	480	394	403	394
query90	1796	201	196	196
query91	129	101	99	99
query92	65	51	48	48
query93	2307	505	516	505
query94	1078	214	213	213
query95	411	313	312	312
query96	614	273	270	270
query97	3179	3047	3080	3047
query98	211	196	198	196
query99	1564	1292	1223	1223
Total cold run time: 283327 ms
Total hot run time: 172571 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.03
query2	0.07	0.04	0.04
query3	0.22	0.05	0.05
query4	1.68	0.07	0.07
query5	0.52	0.48	0.49
query6	1.14	0.72	0.72
query7	0.02	0.01	0.02
query8	0.04	0.04	0.04
query9	0.55	0.49	0.49
query10	0.54	0.55	0.53
query11	0.16	0.11	0.12
query12	0.15	0.12	0.13
query13	0.58	0.59	0.59
query14	0.76	0.77	0.77
query15	0.86	0.81	0.82
query16	0.37	0.36	0.38
query17	0.98	0.98	1.04
query18	0.23	0.21	0.21
query19	1.82	1.73	1.79
query20	0.01	0.01	0.01
query21	15.44	0.77	0.66
query22	5.80	6.48	2.22
query23	18.31	1.36	1.31
query24	2.07	0.23	0.22
query25	0.15	0.09	0.09
query26	0.30	0.22	0.21
query27	0.45	0.23	0.22
query28	13.30	1.00	1.00
query29	12.62	3.32	3.30
query30	0.26	0.06	0.06
query31	2.87	0.40	0.39
query32	3.26	0.47	0.46
query33	2.86	2.89	2.99
query34	16.95	4.34	4.34
query35	4.43	4.42	4.44
query36	0.64	0.46	0.48
query37	0.18	0.15	0.16
query38	0.17	0.15	0.14
query39	0.05	0.03	0.04
query40	0.15	0.12	0.11
query41	0.09	0.05	0.05
query42	0.06	0.05	0.05
query43	0.04	0.04	0.04
Total cold run time: 111.19 s
Total hot run time: 30.97 s

yiguolei pushed a commit that referenced this pull request Jul 17, 2024
@dataroaring dataroaring merged commit 40c178f into apache:master Jul 18, 2024
26 of 29 checks passed
kaijchen added a commit to kaijchen/doris that referenced this pull request Jul 18, 2024
## Proposed changes
fix init agg functions in memtable.
dataroaring pushed a commit that referenced this pull request Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.5-merged dev/3.0.1-merged reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants