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

[opt](assert_cast) Make assert cast do type check in release build by default #39030

Merged
merged 22 commits into from
Aug 13, 2024

Conversation

zhiqiang-hhhh
Copy link
Contributor

@zhiqiang-hhhh zhiqiang-hhhh commented Aug 7, 2024

  • Problem to solve

We encountered many issues that ultimately proved to be caused by memory insecurity. These issues are hard to solve, and the final crash log maybe not related to the root problem.

  • Fix

We make assert_cast do type check in release build by default. And we can use a template arg TypeCheckOnRelease::DISABLE to disable type check in release build.
TypeCheckOnRelease::DISABLE should be used when user agrees that this function will be called many many times (eg. add method of AggregatedData, which will be called by rows) or you think type safe has already been guaranteed (eg. assert_cast<const Derived*, TypeCheckOnRelease::DISABLE>(this).

@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

github-actions bot commented Aug 7, 2024

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

2 similar comments
Copy link
Contributor

github-actions bot commented Aug 7, 2024

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

Copy link
Contributor

github-actions bot commented Aug 8, 2024

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

@zhiqiang-hhhh
Copy link
Contributor Author

run buildall

Copy link
Contributor

github-actions bot commented Aug 8, 2024

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

4 similar comments
Copy link
Contributor

github-actions bot commented Aug 8, 2024

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

Copy link
Contributor

github-actions bot commented Aug 8, 2024

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

Copy link
Contributor

github-actions bot commented Aug 8, 2024

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

Copy link
Contributor

github-actions bot commented Aug 8, 2024

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

@zhiqiang-hhhh zhiqiang-hhhh marked this pull request as ready for review August 8, 2024 09:11
@zhiqiang-hhhh zhiqiang-hhhh changed the title [WIP](assert_cast) Opt assert cast [opt](assert_cast) Make assert cast do type check in release build by default Aug 8, 2024
@@ -360,7 +360,8 @@ Status BlockChanger::change_block(vectorized::Block* ref_block,
// not nullable to nullable
if (new_col_nullable) {
auto* new_nullable_col =
assert_cast<vectorized::ColumnNullable*>(new_col->assume_mutable().get());
assert_cast<vectorized::ColumnNullable*, TypeCheckOnRelease::DISABLE>(
new_col->assume_mutable().get());
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed disable

@@ -69,13 +70,15 @@ class BloomFilterColumnPredicate : public ColumnPredicate {

uint16_t new_size = 0;
if (column.is_column_dictionary()) {
const auto* dict_col = assert_cast<const vectorized::ColumnDictI32*>(&column);
const auto* dict_col =
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed disable

/** Perform static_cast in release build.
* Checks type by comparing typeid and throw an exception in debug build.
* The exact match of the type is checked. That is, cast to the ancestor will be unsuccessful.
*/
template <typename To, typename From>
template <typename To, TypeCheckOnRelease check = TypeCheckOnRelease::ENABLE, typename From>
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 to explain why need this parameter

@@ -408,7 +408,10 @@ struct MethodKeysFixed : public MethodBase<TData> {
CHECK_EQ(sizeof(Fixed), key_sizes[j]);
if (!nullmap_columns.empty() && nullmap_columns[j]) {
const auto& nullmap =
assert_cast<const ColumnUInt8&>(*nullmap_columns[j]).get_data().data();
Copy link
Contributor

Choose a reason for hiding this comment

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

Not need disable

@@ -518,7 +518,8 @@ std::string Block::dump_data(size_t begin, size_t row_limit, bool allow_null_mis
if (data[i].column) {
if (data[i].type->is_nullable() && !data[i].column->is_nullable()) {
assert(allow_null_mismatch);
s = assert_cast<const DataTypeNullable*>(data[i].type.get())
Copy link
Contributor

Choose a reason for hiding this comment

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

not need disable,this is a debug function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -239,7 +239,8 @@ DataTypes make_nullable(const DataTypes& types) {

DataTypePtr remove_nullable(const DataTypePtr& type) {
if (type->is_nullable()) {
return assert_cast<const DataTypeNullable*>(type.get())->get_nested_type();
Copy link
Contributor

Choose a reason for hiding this comment

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

not need

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -155,12 +155,15 @@ Status TransactionalHiveReader::init_row_filters(const TFileRangeDesc& range) {
static int ORIGINAL_TRANSACTION_INDEX = 0;
static int BUCKET_ID_INDEX = 1;
static int ROW_ID_INDEX = 2;
const ColumnInt64& original_transaction_column = assert_cast<const ColumnInt64&>(
Copy link
Contributor

Choose a reason for hiding this comment

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

not need

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -382,8 +382,10 @@ Status VOrcTransformer::_resize_row_batch(const DataTypePtr& type, const IColumn
for (auto* child : struct_batch->fields) {
const IColumn& child_column = struct_col.get_column(idx);
child->resize(child_column.size());
auto child_type = assert_cast<const vectorized::DataTypeStruct*>(real_type.get())
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe not need

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -576,8 +577,8 @@ struct Trim2Impl {
const auto& rcol =
assert_cast<const ColumnConst*>(block.get_by_position(arguments[1]).column.get())
->get_data_column_ptr();
if (const auto* col = assert_cast<const ColumnString*>(column.get())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

not need

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -429,7 +429,8 @@ ColumnPtr convert_to_ipv6(const StringColumnType& string_column,
(*vec_null_map_to)[i] = true;
}
if constexpr (std::is_same_v<ToColumn, ColumnString>) {
auto* column_string = assert_cast<ColumnString*>(col_res.get());
Copy link
Contributor

Choose a reason for hiding this comment

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

not need

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -55,7 +55,7 @@ template <typename Type>
const ColumnConst* check_and_get_column_const(const IColumn* column) {
if (!column || !is_column_const(*column)) return {};

const ColumnConst* res = assert_cast<const ColumnConst*>(column);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this not need
because it already has may if logic

@@ -148,7 +148,7 @@ void validate_argument_type(const IFunction& func, const DataTypes& arguments,
const ColumnConst* check_and_get_column_const_string_or_fixedstring(const IColumn* column) {
if (!is_column_const(*column)) return {};

const ColumnConst* res = assert_cast<const ColumnConst*>(column);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe not need

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Const property is guaranteed by is_column_const

@@ -327,14 +328,15 @@ class FunctionArrayElement : public IFunction {
const UInt8* src_null_map, UInt8* dst_null_map) const {
// check array nested column type and get data
auto left_column = arguments[0].column->convert_to_full_column_if_const();
const auto& array_column = reinterpret_cast<const ColumnArray&>(*left_column);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe not need

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	18342	5030	4459	4459
q2	2535	182	181	181
q3	10986	1180	1118	1118
q4	10529	746	793	746
q5	7747	2641	2611	2611
q6	230	146	143	143
q7	984	631	614	614
q8	9563	1971	1920	1920
q9	8732	6551	6607	6551
q10	7064	2195	2200	2195
q11	460	248	254	248
q12	402	224	226	224
q13	18808	2974	2978	2974
q14	281	239	227	227
q15	521	487	495	487
q16	501	391	401	391
q17	980	648	798	648
q18	8112	7553	7445	7445
q19	5116	1040	953	953
q20	766	326	336	326
q21	5245	4130	4670	4130
q22	1114	1031	996	996
Total cold run time: 119018 ms
Total hot run time: 39587 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4525	4279	4322	4279
q2	380	265	265	265
q3	2875	2567	2593	2567
q4	1929	1612	1626	1612
q5	5271	5327	5253	5253
q6	221	131	133	131
q7	2117	1678	1664	1664
q8	3140	3360	3346	3346
q9	8409	8421	8460	8421
q10	3424	3192	3181	3181
q11	604	498	480	480
q12	785	627	605	605
q13	16324	3007	2953	2953
q14	312	288	290	288
q15	518	469	475	469
q16	464	416	401	401
q17	1792	1505	1479	1479
q18	7723	7652	7448	7448
q19	1657	1636	1599	1599
q20	1987	1839	1782	1782
q21	5201	5159	5054	5054
q22	1097	1016	988	988
Total cold run time: 70755 ms
Total hot run time: 54265 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 202453 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 59433b2cac784e8008853291f625d657e341ba4f, data reload: false

query1	959	407	393	393
query2	6450	1905	1765	1765
query3	6640	217	227	217
query4	33160	23158	22957	22957
query5	4205	497	512	497
query6	286	181	182	181
query7	4588	301	308	301
query8	266	199	192	192
query9	8491	2503	2484	2484
query10	929	880	865	865
query11	16610	14974	15017	14974
query12	148	95	97	95
query13	1642	389	368	368
query14	9627	7736	7833	7736
query15	409	336	333	333
query16	7856	450	453	450
query17	1711	557	537	537
query18	2122	378	431	378
query19	235	212	210	210
query20	112	107	107	107
query21	205	102	94	94
query22	4279	4158	4179	4158
query23	33896	33059	33272	33059
query24	11229	2979	2909	2909
query25	630	364	362	362
query26	1380	151	154	151
query27	2870	289	281	281
query28	7177	2107	2059	2059
query29	922	401	404	401
query30	297	149	145	145
query31	989	745	770	745
query32	95	53	51	51
query33	738	287	283	283
query34	992	473	489	473
query35	986	866	834	834
query36	1073	936	908	908
query37	144	77	80	77
query38	4230	4076	4148	4076
query39	1451	1382	1389	1382
query40	273	112	115	112
query41	48	45	46	45
query42	120	95	97	95
query43	507	472	452	452
query44	1274	725	746	725
query45	398	377	362	362
query46	1105	753	774	753
query47	1872	1771	1785	1771
query48	376	305	297	297
query49	1087	423	411	411
query50	819	411	421	411
query51	6807	6664	6705	6664
query52	101	91	90	90
query53	253	179	182	179
query54	937	450	450	450
query55	77	77	77	77
query56	265	243	257	243
query57	1141	1109	1061	1061
query58	239	223	239	223
query59	3182	2612	2735	2612
query60	297	261	249	249
query61	97	97	96	96
query62	850	657	666	657
query63	217	192	184	184
query64	9818	2413	1935	1935
query65	3196	3129	3139	3129
query66	977	335	334	334
query67	15266	15025	15030	15025
query68	4544	560	564	560
query69	442	382	370	370
query70	1199	1088	1129	1088
query71	400	279	276	276
query72	18022	16864	16462	16462
query73	753	333	332	332
query74	9070	8787	8691	8691
query75	3399	2664	2673	2664
query76	2790	1011	994	994
query77	487	320	311	311
query78	9595	8903	8919	8903
query79	2871	528	519	519
query80	2127	484	477	477
query81	613	232	225	225
query82	797	132	140	132
query83	300	166	158	158
query84	266	84	82	82
query85	1800	323	289	289
query86	458	305	295	295
query87	4753	4696	4492	4492
query88	4486	2497	2505	2497
query89	386	291	284	284
query90	1801	197	193	193
query91	143	119	119	119
query92	70	48	51	48
query93	2627	538	532	532
query94	937	295	293	293
query95	354	259	262	259
query96	615	279	278	278
query97	3295	3072	3058	3058
query98	220	198	199	198
query99	1733	1244	1270	1244
Total cold run time: 312981 ms
Total hot run time: 202453 ms

yiguolei
yiguolei previously approved these changes Aug 12, 2024
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

Copy link
Contributor

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Aug 12, 2024
assert_cast<const Derived*>(this)->deserialize_and_merge(place, deserialized_place,
buffer_reader, arena);
(assert_cast<const ColumnString&, TypeCheckOnRelease::DISABLE>(column))
.get_data_at(i));
Copy link
Contributor

@jacktengg jacktengg Aug 12, 2024

Choose a reason for hiding this comment

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

I think this one should enable check.
Also an optimization can be done: perform assert cast outside of the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

make sense

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Aug 12, 2024
Copy link
Contributor

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

@zhiqiang-hhhh
Copy link
Contributor Author

run buildall

@zhiqiang-hhhh
Copy link
Contributor Author

run buildall

@zhiqiang-hhhh
Copy link
Contributor 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! 👍"

Copy link
Contributor

@jacktengg jacktengg 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 Aug 12, 2024
Copy link
Contributor

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	18038	4643	4440	4440
q2	2257	173	173	173
q3	10511	1165	1074	1074
q4	10141	701	791	701
q5	7495	2492	2490	2490
q6	228	140	135	135
q7	975	602	586	586
q8	9215	1929	1924	1924
q9	8719	6574	6543	6543
q10	7019	2202	2214	2202
q11	455	250	252	250
q12	395	216	224	216
q13	17755	3006	2984	2984
q14	292	238	229	229
q15	533	475	489	475
q16	486	385	388	385
q17	975	634	763	634
q18	8136	7614	7397	7397
q19	6880	1047	1003	1003
q20	671	339	338	338
q21	5403	4321	4501	4321
q22	1098	1031	1016	1016
Total cold run time: 117677 ms
Total hot run time: 39516 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4527	4233	4259	4233
q2	369	267	261	261
q3	3007	2618	2602	2602
q4	1908	1619	1656	1619
q5	5227	5267	5282	5267
q6	218	128	128	128
q7	2070	1694	1643	1643
q8	3154	3363	3306	3306
q9	8439	8397	8338	8338
q10	3365	3135	3143	3135
q11	589	484	476	476
q12	772	604	576	576
q13	16435	2942	2955	2942
q14	298	279	270	270
q15	525	489	465	465
q16	481	411	409	409
q17	1755	1472	1437	1437
q18	7646	7609	7534	7534
q19	1692	1538	1592	1538
q20	2013	1780	1772	1772
q21	5335	5039	5145	5039
q22	1111	995	1013	995
Total cold run time: 70936 ms
Total hot run time: 53985 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 199574 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 15c0dcef9b9f0e5589dad6b2204984e034e96f9b, data reload: false

query1	914	368	355	355
query2	6441	1971	1859	1859
query3	6647	229	224	224
query4	34021	23293	22935	22935
query5	4237	487	471	471
query6	273	156	159	156
query7	4585	290	292	290
query8	237	202	178	178
query9	8619	2427	2377	2377
query10	542	463	466	463
query11	17775	14800	15006	14800
query12	143	98	93	93
query13	1641	378	365	365
query14	10166	7693	6376	6376
query15	269	230	222	222
query16	7621	487	503	487
query17	1740	571	553	553
query18	1956	285	289	285
query19	200	144	145	144
query20	111	105	105	105
query21	210	101	106	101
query22	4645	4236	4097	4097
query23	34030	33277	33218	33218
query24	11039	2658	2594	2594
query25	612	382	382	382
query26	1357	159	155	155
query27	2827	281	289	281
query28	7613	2001	2006	2001
query29	910	425	419	419
query30	303	149	148	148
query31	984	822	759	759
query32	93	54	54	54
query33	743	285	279	279
query34	948	468	490	468
query35	945	799	808	799
query36	1075	948	900	900
query37	143	79	77	77
query38	4182	4172	4099	4099
query39	1410	1367	1378	1367
query40	287	114	112	112
query41	47	46	43	43
query42	111	94	96	94
query43	480	460	462	460
query44	1253	733	752	733
query45	216	205	211	205
query46	1088	743	747	743
query47	1874	1806	1774	1774
query48	363	300	290	290
query49	1084	411	415	411
query50	811	404	407	404
query51	6858	6608	6714	6608
query52	104	88	93	88
query53	255	192	179	179
query54	887	451	450	450
query55	78	74	74	74
query56	261	273	258	258
query57	1210	1065	1119	1065
query58	235	220	222	220
query59	2895	2809	2761	2761
query60	287	260	258	258
query61	100	92	98	92
query62	827	622	663	622
query63	214	178	185	178
query64	9789	2232	1748	1748
query65	3185	3135	3152	3135
query66	954	355	320	320
query67	15376	14901	14914	14901
query68	4633	553	547	547
query69	445	346	411	346
query70	1208	1066	1129	1066
query71	443	275	271	271
query72	19610	16529	16562	16529
query73	768	338	339	338
query74	9180	8694	8755	8694
query75	4167	2710	2635	2635
query76	3299	1002	1054	1002
query77	674	310	306	306
query78	9880	9141	9236	9141
query79	2494	520	520	520
query80	1110	531	498	498
query81	602	220	222	220
query82	850	140	132	132
query83	323	145	143	143
query84	259	79	77	77
query85	2005	271	265	265
query86	483	314	288	288
query87	4746	4462	4455	4455
query88	3996	2492	2481	2481
query89	400	277	292	277
query90	1986	194	193	193
query91	121	94	98	94
query92	67	47	49	47
query93	3180	535	531	531
query94	1013	287	304	287
query95	358	263	259	259
query96	601	277	271	271
query97	3244	3043	3034	3034
query98	218	200	195	195
query99	1465	1295	1270	1270
Total cold run time: 317164 ms
Total hot run time: 199574 ms

@doris-robot
Copy link

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

query1	0.05	0.04	0.04
query2	0.08	0.04	0.04
query3	0.23	0.05	0.05
query4	1.68	0.08	0.08
query5	0.50	0.49	0.50
query6	1.13	0.72	0.73
query7	0.01	0.01	0.02
query8	0.05	0.04	0.04
query9	0.55	0.49	0.48
query10	0.54	0.56	0.54
query11	0.16	0.12	0.12
query12	0.15	0.12	0.12
query13	0.59	0.59	0.57
query14	0.78	0.76	0.80
query15	0.88	0.81	0.83
query16	0.35	0.38	0.37
query17	0.98	0.96	1.02
query18	0.23	0.22	0.21
query19	1.85	1.77	1.85
query20	0.01	0.02	0.01
query21	15.39	0.77	0.68
query22	3.77	7.70	1.97
query23	18.32	1.37	1.24
query24	2.15	0.21	0.21
query25	0.14	0.08	0.09
query26	0.31	0.20	0.21
query27	0.45	0.23	0.23
query28	13.30	1.01	1.00
query29	12.61	3.31	3.26
query30	0.23	0.04	0.04
query31	2.91	0.37	0.38
query32	3.26	0.47	0.48
query33	2.96	2.97	2.90
query34	17.09	4.36	4.33
query35	4.45	4.45	4.43
query36	0.65	0.47	0.49
query37	0.19	0.16	0.16
query38	0.15	0.15	0.14
query39	0.05	0.03	0.04
query40	0.15	0.13	0.13
query41	0.09	0.05	0.04
query42	0.06	0.05	0.05
query43	0.05	0.04	0.04
Total cold run time: 109.53 s
Total hot run time: 30.66 s

@yiguolei yiguolei merged commit aa2929e into apache:master Aug 13, 2024
29 of 32 checks passed
@zhiqiang-hhhh zhiqiang-hhhh deleted the opt-assert-cast branch August 13, 2024 04:14
wyxxxcat pushed a commit to wyxxxcat/doris that referenced this pull request Aug 14, 2024
… default (apache#39030)

* Problem to solve

We encountered many issues that ultimately proved to be caused by memory
insecurity. These issues are hard to solve, and the final crash log
maybe not related to the root problem.

* Fix

We make `assert_cast` do type check in release build by default. And we
can use a template arg `TypeCheckOnRelease::DISABLE` to disable type
check in release build.
`TypeCheckOnRelease::DISABLE` should be used when user agrees that this
function will be called many many times (eg. add method of
AggregatedData, which will be called by rows) or you think type safe has
already been guaranteed (eg. `assert_cast<const Derived*,
TypeCheckOnRelease::DISABLE>(this)`.
dataroaring pushed a commit that referenced this pull request Aug 17, 2024
… default (#39030)

* Problem to solve

We encountered many issues that ultimately proved to be caused by memory
insecurity. These issues are hard to solve, and the final crash log
maybe not related to the root problem.

* Fix

We make `assert_cast` do type check in release build by default. And we
can use a template arg `TypeCheckOnRelease::DISABLE` to disable type
check in release build.
`TypeCheckOnRelease::DISABLE` should be used when user agrees that this
function will be called many many times (eg. add method of
AggregatedData, which will be called by rows) or you think type safe has
already been guaranteed (eg. `assert_cast<const Derived*,
TypeCheckOnRelease::DISABLE>(this)`.
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/3.0.2-merged doing reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants