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

[Bug](array) fix array column core dump in get_shrinked_column as not check type #33295

Merged
merged 3 commits into from
Apr 7, 2024

Conversation

zhangstar333
Copy link
Contributor

get_shrinked_column() function should only call by string type.
so should check column type.

 6# google::LogMessage::SendToLog() in /mnt/disk2/zhangsida/doris/output/be/lib/doris_be
 7# google::LogMessage::Flush() in /mnt/disk2/zhangsida/doris/output/be/lib/doris_be
 8# google::LogMessageFatal::~LogMessageFatal() in /mnt/disk2/zhangsida/doris/output/be/lib/doris_be
 9# doris::vectorized::IColumn::get_shrinked_column() in /mnt/disk2/zhangsida/doris/output/be/lib/doris_be
10# doris::vectorized::ColumnNullable::get_shrinked_column() at /mnt/disk2/zhangsida/doris/be/src/vec/columns/column_nullable.cpp:52
11# doris::vectorized::ColumnArray::get_shrinked_column() at /mnt/disk2/zhangsida/doris/be/src/vec/columns/column_array.cpp:129
12# doris::vectorized::ColumnNullable::get_shrinked_column() at /mnt/disk2/zhangsida/doris/be/src/vec/columns/column_nullable.cpp:52
13# doris::vectorized::ColumnStruct::get_shrinked_column() at /mnt/disk2/zhangsida/doris/be/src/vec/columns/column_struct.cpp:305
14# doris::vectorized::ColumnNullable::get_shrinked_column() at /mnt/disk2/zhangsida/doris/be/src/vec/columns/column_nullable.cpp:52
15# doris::vectorized::Block::shrink_char_type_column_suffix_zero(std::vector<unsigned long, std::allocator<unsigned long> > const&)

Proposed changes

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

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

@zhangstar333
Copy link
Contributor Author

run buildall

Copy link
Contributor

github-actions bot commented Apr 7, 2024

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

@zhangstar333
Copy link
Contributor Author

run buildall

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

bool is_variable_length() const override { return nested_column->is_variable_length(); }

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'get_family_name' can be made static [readability-convert-member-functions-to-static]

Suggested change
static const char* get_family_name() override { return "Nullable"; }

@@ -120,6 +120,7 @@ class ColumnString final : public COWHelper<IColumn, ColumnString> {
MutableColumnPtr clone_resized(size_t to_size) const override;

MutableColumnPtr get_shrinked_column() override;
bool could_shrinked_column() override { return true; }
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'could_shrinked_column' can be made static [readability-convert-member-functions-to-static]

Suggested change
bool could_shrinked_column() override { return true; }
static bool could_shrinked_column() override { return true; }

@@ -295,13 +295,22 @@ ColumnPtr ColumnStruct::replicate(const Offsets& offsets) const {
return ColumnStruct::create(new_columns);
}

bool ColumnStruct::could_shrinked_column() {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'could_shrinked_column' can be made static [readability-convert-member-functions-to-static]

be/src/vec/columns/column_struct.h:158:

-     bool could_shrinked_column() override;
+     static bool could_shrinked_column() override;

Copy link
Contributor

@yiguolei yiguolei 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 Apr 7, 2024
Copy link
Contributor

github-actions bot commented Apr 7, 2024

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

Copy link
Contributor

github-actions bot commented Apr 7, 2024

PR approved by anyone and no changes requested.

Copy link
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

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

LGTM

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17640	4140	4112	4112
q2	2014	190	182	182
q3	10479	1209	1337	1209
q4	10193	834	959	834
q5	7487	3029	2962	2962
q6	222	132	133	132
q7	1113	629	615	615
q8	9408	2093	2068	2068
q9	6710	6253	6187	6187
q10	8483	3526	3537	3526
q11	415	235	249	235
q12	375	215	211	211
q13	17779	2914	2930	2914
q14	274	234	233	233
q15	525	485	484	484
q16	481	372	373	372
q17	975	930	930	930
q18	7421	6586	6423	6423
q19	1610	1541	1537	1537
q20	558	316	318	316
q21	3567	3133	3139	3133
q22	352	300	318	300
Total cold run time: 108081 ms
Total hot run time: 38915 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4121	4016	4040	4016
q2	322	217	218	217
q3	2965	2961	2983	2961
q4	1891	1862	1817	1817
q5	5268	5251	5252	5251
q6	210	128	126	126
q7	2232	1798	1770	1770
q8	3253	3328	3325	3325
q9	8547	8539	8500	8500
q10	3795	4060	4053	4053
q11	575	469	464	464
q12	790	595	665	595
q13	16549	3223	3176	3176
q14	323	275	269	269
q15	526	506	475	475
q16	504	440	435	435
q17	1766	1778	1770	1770
q18	8445	7704	7577	7577
q19	1704	1745	1716	1716
q20	2065	1752	1746	1746
q21	5320	5103	5110	5103
q22	499	454	437	437
Total cold run time: 71670 ms
Total hot run time: 55799 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.64% (8887/24938)
Line Coverage: 27.38% (72983/266520)
Region Coverage: 26.56% (37726/142042)
Branch Coverage: 23.36% (19227/82302)
Coverage Report: http://coverage.selectdb-in.cc/coverage/ba457a6e0b1de347a7199e6e45d1fb1f57be7aaf_ba457a6e0b1de347a7199e6e45d1fb1f57be7aaf/report/index.html

@doris-robot
Copy link

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

query1	1212	1120	357	357
query2	6195	2054	1956	1956
query3	6643	211	199	199
query4	24108	21511	21506	21506
query5	4186	410	419	410
query6	288	193	183	183
query7	4591	305	300	300
query8	232	177	179	177
query9	8452	2252	2241	2241
query10	459	253	257	253
query11	14978	14533	14451	14451
query12	150	92	88	88
query13	1639	386	374	374
query14	8612	6961	7205	6961
query15	214	173	173	173
query16	6444	255	253	253
query17	1016	565	554	554
query18	1749	273	262	262
query19	192	154	154	154
query20	91	88	84	84
query21	193	132	132	132
query22	4916	4771	4840	4771
query23	33283	32392	32233	32233
query24	10552	3144	3129	3129
query25	683	389	414	389
query26	1852	160	160	160
query27	3225	363	375	363
query28	7353	1890	1882	1882
query29	1155	629	612	612
query30	308	178	177	177
query31	1014	765	759	759
query32	99	60	53	53
query33	651	256	259	256
query34	1091	530	520	520
query35	856	734	731	731
query36	1036	872	861	861
query37	146	77	77	77
query38	3623	3582	3625	3582
query39	1660	1595	1560	1560
query40	237	136	137	136
query41	51	48	47	47
query42	115	113	109	109
query43	453	411	413	411
query44	1176	745	775	745
query45	281	266	272	266
query46	1104	851	817	817
query47	2036	1873	1858	1858
query48	388	317	304	304
query49	964	373	386	373
query50	874	406	416	406
query51	6881	6828	6813	6813
query52	109	104	97	97
query53	370	296	298	296
query54	289	242	248	242
query55	88	85	75	75
query56	256	240	252	240
query57	1292	1146	1182	1146
query58	274	217	223	217
query59	2774	2553	2406	2406
query60	263	239	251	239
query61	112	96	87	87
query62	660	453	457	453
query63	300	281	283	281
query64	5874	3158	3119	3119
query65	3048	3001	3004	3001
query66	1289	326	311	311
query67	15503	15049	14811	14811
query68	9074	565	582	565
query69	542	303	309	303
query70	1370	1141	1131	1131
query71	521	278	267	267
query72	6306	2576	2417	2417
query73	941	317	321	317
query74	6784	6229	6378	6229
query75	3865	2285	2310	2285
query76	6269	1195	1225	1195
query77	636	259	245	245
query78	10927	10114	10203	10114
query79	9886	527	531	527
query80	1530	422	421	421
query81	502	236	232	232
query82	744	95	93	93
query83	220	168	171	168
query84	270	85	86	85
query85	1313	288	282	282
query86	419	286	282	282
query87	3587	3478	3508	3478
query88	3743	2279	2275	2275
query89	557	377	366	366
query90	2026	183	182	182
query91	132	101	103	101
query92	59	48	47	47
query93	6482	521	525	521
query94	1016	184	184	184
query95	433	313	306	306
query96	615	268	270	268
query97	2661	2517	2478	2478
query98	228	218	217	217
query99	1356	813	850	813
Total cold run time: 297222 ms
Total hot run time: 180883 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.03
query2	0.08	0.04	0.04
query3	0.23	0.05	0.05
query4	1.67	0.06	0.06
query5	0.49	0.48	0.49
query6	1.14	0.65	0.65
query7	0.02	0.01	0.01
query8	0.05	0.04	0.04
query9	0.57	0.51	0.51
query10	0.57	0.56	0.58
query11	0.16	0.13	0.12
query12	0.16	0.12	0.11
query13	0.60	0.59	0.60
query14	0.76	0.81	0.79
query15	0.87	0.85	0.85
query16	0.36	0.35	0.36
query17	0.96	0.98	0.99
query18	0.26	0.25	0.27
query19	1.81	1.65	1.71
query20	0.02	0.01	0.01
query21	15.41	0.67	0.64
query22	3.85	6.96	2.13
query23	17.95	1.25	1.20
query24	1.53	0.21	0.20
query25	0.17	0.08	0.07
query26	0.26	0.16	0.16
query27	0.08	0.07	0.08
query28	13.79	0.97	0.95
query29	12.64	3.25	3.26
query30	0.26	0.06	0.05
query31	2.88	0.38	0.38
query32	3.30	0.47	0.48
query33	2.84	2.84	2.92
query34	15.50	4.34	4.33
query35	4.40	4.39	4.39
query36	0.68	0.48	0.47
query37	0.18	0.15	0.15
query38	0.15	0.14	0.13
query39	0.04	0.03	0.04
query40	0.17	0.14	0.15
query41	0.10	0.04	0.05
query42	0.05	0.05	0.05
query43	0.04	0.03	0.04
Total cold run time: 107.09 s
Total hot run time: 30.3 s

@doris-robot
Copy link

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

Load test result on commit ba457a6e0b1de347a7199e6e45d1fb1f57be7aaf with default session variables
Stream load json:         18 seconds loaded 2358488459 Bytes, about 124 MB/s
Stream load orc:          59 seconds loaded 1101869774 Bytes, about 17 MB/s
Stream load parquet:      33 seconds loaded 861443392 Bytes, about 24 MB/s
Insert into select:       17.6 seconds inserted 10000000 Rows, about 568K ops/s

@yiguolei yiguolei merged commit f9de308 into apache:master Apr 7, 2024
26 of 29 checks passed
yiguolei pushed a commit that referenced this pull request Apr 7, 2024
… check type (#33295)

* [Bug](array) fix array column core dump in get_shrinked_column as not check type

* add function could_shrinked_column
@yiguolei yiguolei mentioned this pull request Apr 8, 2024
seawinde pushed a commit to seawinde/doris that referenced this pull request Apr 10, 2024
… check type (apache#33295)

* [Bug](array) fix array column core dump in get_shrinked_column as not check type

* add function could_shrinked_column
@xiaokang xiaokang added the p0_c label Apr 10, 2024
zhangstar333 added a commit to zhangstar333/incubator-doris that referenced this pull request Apr 11, 2024
… check type (apache#33295)

* [Bug](array) fix array column core dump in get_shrinked_column as not check type

* add function could_shrinked_column
yiguolei pushed a commit that referenced this pull request Apr 11, 2024
… check type (#33295) (#33552)

* [Bug](array) fix array column core dump in get_shrinked_column as not check type

* add function could_shrinked_column
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.0.9-merged dev/2.1.2-merged p0_c reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants