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](Variant) implement resize and set num_rows right when subcolumns is empty #38364

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

eldenmoon
Copy link
Member

@eldenmoon eldenmoon commented Jul 25, 2024

Proposed changes

Set rows correct when subcolumns is empty, and implement resize method with correct semantic
This fix the potential crash with stack bellow

6# doris::vectorized::ColumnObject::get(unsigned long, doris::vectorized::Field&) const at /root/doris/be/src/vec/columns/column_object.cpp:841
7# doris::vectorized::ColumnObject::operator[](unsigned long) const at /root/doris/be/src/vec/columns/column_object.cpp:838
8# doris::vectorized::ColumnObject::insert_from(doris::vectorized::IColumn const&, unsigned long) in /home/work/unlimit_teamcity/TeamCity/Agents/20240725170551agent_172.16.0.48_1/work/60183217f6ee2a9c/output/be/lib/doris_be
9# doris::vectorized::ColumnObject::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) at /root/doris/be/src/vec/columns/column_object.cpp:1581

Issue Number: close #xxx

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

Copy link
Contributor

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

1 similar comment
Copy link
Contributor

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

@eldenmoon eldenmoon changed the title [Fix](Variant) implement resize and set num_rows right in `apply_for_… [Fix](Variant) implement resize and set num_rows right when subcolumns is empty Jul 25, 2024
@eldenmoon
Copy link
Member Author

run buildall

Copy link
Contributor

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

1 similar comment
Copy link
Contributor

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17677	4537	4328	4328
q2	2015	198	193	193
q3	10503	1202	1078	1078
q4	10152	723	720	720
q5	7620	2725	2607	2607
q6	222	136	137	136
q7	956	598	609	598
q8	9231	1928	1932	1928
q9	8705	6596	6509	6509
q10	8839	3844	3791	3791
q11	482	244	250	244
q12	497	223	220	220
q13	18054	2979	2958	2958
q14	274	234	234	234
q15	518	481	491	481
q16	515	377	375	375
q17	971	677	643	643
q18	8158	7684	7380	7380
q19	6581	1069	971	971
q20	649	338	326	326
q21	4993	3188	3255	3188
q22	345	288	285	285
Total cold run time: 117957 ms
Total hot run time: 39193 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4476	4247	4216	4216
q2	389	282	269	269
q3	3069	2873	2916	2873
q4	1977	1659	1681	1659
q5	5713	5514	5530	5514
q6	221	131	159	131
q7	2179	1867	1836	1836
q8	3271	3414	3473	3414
q9	8783	8747	8889	8747
q10	4061	3882	3762	3762
q11	600	490	504	490
q12	809	682	641	641
q13	17281	3152	3159	3152
q14	325	294	285	285
q15	532	494	477	477
q16	501	450	440	440
q17	1815	1548	1533	1533
q18	8144	7921	7957	7921
q19	1754	1666	1605	1605
q20	2528	1909	1872	1872
q21	5183	4816	4729	4729
q22	582	531	520	520
Total cold run time: 74193 ms
Total hot run time: 56086 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 173593 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 0c3a817902eee520bcd899abc714c331758491df, data reload: false

query1	938	375	366	366
query2	6425	1948	1820	1820
query3	6633	207	221	207
query4	27679	17512	17447	17447
query5	3576	500	503	500
query6	272	171	176	171
query7	4579	299	289	289
query8	243	199	206	199
query9	8615	2439	2436	2436
query10	430	287	288	287
query11	12109	10172	9997	9997
query12	121	89	84	84
query13	1621	384	372	372
query14	9869	7801	7669	7669
query15	221	168	170	168
query16	7526	461	508	461
query17	1534	577	531	531
query18	1912	307	284	284
query19	186	140	143	140
query20	93	87	86	86
query21	212	97	100	97
query22	4373	4197	4076	4076
query23	34067	33683	33567	33567
query24	11104	2931	2868	2868
query25	627	387	426	387
query26	1140	167	157	157
query27	2930	271	285	271
query28	7643	2113	2088	2088
query29	861	430	437	430
query30	258	151	153	151
query31	987	788	758	758
query32	97	52	56	52
query33	763	338	330	330
query34	966	480	481	480
query35	885	775	752	752
query36	1127	1012	974	974
query37	151	89	80	80
query38	2923	2883	2822	2822
query39	892	872	845	845
query40	197	109	112	109
query41	45	43	44	43
query42	121	108	97	97
query43	503	448	464	448
query44	1241	748	722	722
query45	206	189	177	177
query46	1094	714	729	714
query47	1834	1754	1790	1754
query48	355	297	288	288
query49	841	410	419	410
query50	792	406	399	399
query51	6775	6734	6680	6680
query52	102	95	93	93
query53	266	188	184	184
query54	844	443	439	439
query55	77	75	74	74
query56	289	277	304	277
query57	1118	1063	1034	1034
query58	253	271	270	270
query59	2948	2676	2554	2554
query60	311	287	282	282
query61	97	110	94	94
query62	797	647	666	647
query63	216	179	179	179
query64	9537	2238	1778	1778
query65	3163	3455	3161	3161
query66	857	326	336	326
query67	15539	15099	14891	14891
query68	4690	568	555	555
query69	671	423	313	313
query70	1117	1178	1078	1078
query71	434	277	269	269
query72	7102	5608	5949	5608
query73	755	326	328	326
query74	6080	5657	5702	5657
query75	3524	2662	2728	2662
query76	3208	931	899	899
query77	625	304	319	304
query78	9794	9566	8966	8966
query79	3100	511	517	511
query80	1463	487	492	487
query81	574	215	224	215
query82	763	139	145	139
query83	207	174	179	174
query84	274	79	79	79
query85	1418	315	365	315
query86	457	286	298	286
query87	3231	3093	3117	3093
query88	4061	2418	2440	2418
query89	429	285	287	285
query90	1787	191	194	191
query91	123	99	102	99
query92	57	47	48	47
query93	4369	549	544	544
query94	780	285	301	285
query95	351	260	263	260
query96	615	282	277	277
query97	3212	2975	3019	2975
query98	226	195	199	195
query99	1587	1258	1292	1258
Total cold run time: 284090 ms
Total hot run time: 173593 ms

@doris-robot
Copy link

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

query1	0.04	0.04	0.03
query2	0.08	0.04	0.04
query3	0.22	0.05	0.06
query4	1.68	0.07	0.07
query5	0.50	0.48	0.47
query6	1.16	0.73	0.74
query7	0.02	0.01	0.01
query8	0.05	0.04	0.04
query9	0.56	0.51	0.49
query10	0.55	0.55	0.54
query11	0.15	0.11	0.11
query12	0.15	0.12	0.12
query13	0.59	0.58	0.58
query14	0.75	0.76	0.78
query15	0.86	0.81	0.82
query16	0.35	0.37	0.37
query17	1.03	0.95	0.99
query18	0.24	0.22	0.22
query19	1.85	1.72	1.66
query20	0.01	0.01	0.02
query21	15.38	0.82	0.67
query22	4.22	7.84	1.82
query23	18.27	1.37	1.29
query24	2.08	0.21	0.22
query25	0.17	0.09	0.08
query26	0.29	0.20	0.20
query27	0.45	0.23	0.24
query28	13.30	1.03	1.01
query29	12.59	3.29	3.31
query30	0.25	0.06	0.05
query31	2.87	0.40	0.38
query32	3.27	0.48	0.47
query33	2.88	2.96	2.89
query34	17.09	4.42	4.34
query35	4.46	4.45	4.36
query36	0.66	0.46	0.46
query37	0.18	0.16	0.15
query38	0.15	0.14	0.15
query39	0.05	0.03	0.04
query40	0.15	0.13	0.12
query41	0.09	0.05	0.05
query42	0.05	0.05	0.05
query43	0.04	0.04	0.04
Total cold run time: 109.78 s
Total hot run time: 30.39 s

Copy link
Contributor

@qidaye qidaye 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 26, 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.

@airborne12
Copy link
Member

Proposed changes need to be filled

@eldenmoon
Copy link
Member Author

Proposed changes need to be filled

done

Copy link
Member

@airborne12 airborne12 left a comment

Choose a reason for hiding this comment

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

LGTM

@eldenmoon
Copy link
Member Author

Block the regression-test pipeline, so i merged it

@eldenmoon eldenmoon merged commit d176998 into apache:master Jul 26, 2024
29 of 32 checks passed
@eldenmoon eldenmoon deleted the fix-var-rows branch July 26, 2024 04:19
eldenmoon added a commit to eldenmoon/incubator-doris that referenced this pull request Jul 30, 2024
…ns` is empty (apache#38364)

Set rows correct when subcolumns is empty, and implement resize method
with correct semantic
This fix the potential crash with stack bellow
```
6# doris::vectorized::ColumnObject::get(unsigned long, doris::vectorized::Field&) const at /root/doris/be/src/vec/columns/column_object.cpp:841
7# doris::vectorized::ColumnObject::operator[](unsigned long) const at /root/doris/be/src/vec/columns/column_object.cpp:838
8# doris::vectorized::ColumnObject::insert_from(doris::vectorized::IColumn const&, unsigned long) in /home/work/unlimit_teamcity/TeamCity/Agents/20240725170551agent_172.16.0.48_1/work/60183217f6ee2a9c/output/be/lib/doris_be
9# doris::vectorized::ColumnObject::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) at /root/doris/be/src/vec/columns/column_object.cpp:1581
```
eldenmoon added a commit to eldenmoon/incubator-doris that referenced this pull request Jul 30, 2024
…ns` is empty (apache#38364)

Set rows correct when subcolumns is empty, and implement resize method
with correct semantic
This fix the potential crash with stack bellow
```
6# doris::vectorized::ColumnObject::get(unsigned long, doris::vectorized::Field&) const at /root/doris/be/src/vec/columns/column_object.cpp:841
7# doris::vectorized::ColumnObject::operator[](unsigned long) const at /root/doris/be/src/vec/columns/column_object.cpp:838
8# doris::vectorized::ColumnObject::insert_from(doris::vectorized::IColumn const&, unsigned long) in /home/work/unlimit_teamcity/TeamCity/Agents/20240725170551agent_172.16.0.48_1/work/60183217f6ee2a9c/output/be/lib/doris_be
9# doris::vectorized::ColumnObject::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) at /root/doris/be/src/vec/columns/column_object.cpp:1581
```
xiaokang pushed a commit that referenced this pull request Jul 31, 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.6-merged dev/3.0.1-merged p0_c reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants