Skip to content

[refine](core) add compile-time safety checks to assert_cast#63133

Merged
Mryange merged 1 commit into
apache:masterfrom
Mryange:assert_cast-dev-11
May 12, 2026
Merged

[refine](core) add compile-time safety checks to assert_cast#63133
Mryange merged 1 commit into
apache:masterfrom
Mryange:assert_cast-dev-11

Conversation

@Mryange
Copy link
Copy Markdown
Contributor

@Mryange Mryange commented May 11, 2026

What problem does this PR solve?

Issue Number: N/A

Problem Summary:

assert_cast previously only performed runtime type checks (via typeid comparison). Misuse — such as casting between unrelated types or upcasting — would only be caught at runtime, making it easy for incorrect usage to slip into release builds where TypeCheckOnRelease::DISABLE turns checks into static_cast without any verification.

This catches invalid casts at build time instead of producing undefined behavior or hard-to-diagnose runtime errors.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented May 11, 2026

/review

@hello-stephen
Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

Copy link
Copy Markdown
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.

Review result: no blocking issues found in the actual PR diff. The change adds compile-time constraints to assert_cast and is consistent with the helper's existing downcast-only runtime contract; representative usages are class pointer/reference downcasts from common base types.

Critical checkpoints:

  • Goal/test: The PR aims to reject unrelated/upcast assert_cast misuse at compile time. The code implements that for class pointer/reference downcasts. No dedicated test was added; given this is a compile-time constraint in a heavily used template, existing BE compilation should cover compatibility.
  • Scope/focus: The change is small and focused in be/src/core/assert_cast.h.
  • Concurrency/lifecycle/config/compatibility: Not applicable; no runtime state, persistence, protocol, or config changes.
  • Parallel code paths: Not applicable; this is the single assert_cast implementation.
  • Error handling: Runtime error behavior is unchanged for valid template instantiations that keep checks enabled.
  • Memory/data correctness: Not applicable; no allocation, storage, or data visibility changes.
  • Performance: No runtime overhead added beyond compile-time static_asserts.
  • Observability: Not applicable.

User focus: No additional user-provided review focus was present.

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented May 11, 2026

run buildall

@hello-stephen
Copy link
Copy Markdown
Contributor

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

------ Round 1 ----------------------------------
orders	Doris	NULL	NULL	0	0	0	NULL	0	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL	
============================================
q1	17619	4059	3985	3985
q2	q3	10723	894	636	636
q4	4665	461	344	344
q5	7440	1336	1146	1146
q6	215	180	142	142
q7	943	948	766	766
q8	9925	1456	1328	1328
q9	6542	5606	5308	5308
q10	6324	2054	1794	1794
q11	481	271	264	264
q12	689	419	295	295
q13	18171	3300	2765	2765
q14	300	289	261	261
q15	q16	896	878	796	796
q17	961	1114	695	695
q18	6566	5693	5613	5613
q19	1184	1322	1103	1103
q20	530	405	264	264
q21	4736	2270	1875	1875
q22	434	374	308	308
Total cold run time: 99344 ms
Total hot run time: 29688 ms

----- Round 2, with runtime_filter_mode=off -----
orders	Doris	NULL	NULL	150000000	42	6422171781	NULL	22778155	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL	
============================================
q1	4270	4326	4119	4119
q2	q3	4663	4781	4176	4176
q4	2149	2165	1398	1398
q5	4982	4975	5296	4975
q6	192	169	133	133
q7	2262	1938	1640	1640
q8	3520	3189	3186	3186
q9	8509	8488	8348	8348
q10	4503	4511	4289	4289
q11	614	467	423	423
q12	717	755	521	521
q13	3274	3634	2933	2933
q14	307	300	285	285
q15	q16	775	800	754	754
q17	1446	1367	1314	1314
q18	8369	7267	7207	7207
q19	1190	1187	1174	1174
q20	2233	2244	1972	1972
q21	6379	5656	5094	5094
q22	592	565	442	442
Total cold run time: 60946 ms
Total hot run time: 54383 ms

@hello-stephen
Copy link
Copy Markdown
Contributor

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

query5	4386	673	516	516
query6	350	225	208	208
query7	4232	608	320	320
query8	342	236	229	229
query9	8849	4093	4091	4091
query10	454	347	309	309
query11	5770	2431	2206	2206
query12	183	130	129	129
query13	1256	616	431	431
query14	6725	5390	5024	5024
query14_1	4348	4363	4362	4362
query15	209	209	188	188
query16	1007	490	448	448
query17	1129	759	620	620
query18	2736	493	362	362
query19	230	221	172	172
query20	149	145	140	140
query21	216	150	125	125
query22	13609	13528	13450	13450
query23	17233	16368	16516	16368
query23_1	16350	16310	16383	16310
query24	8147	1900	1450	1450
query24_1	1426	1427	1458	1427
query25	643	555	484	484
query26	1709	344	182	182
query27	2832	631	367	367
query28	4435	2048	1983	1983
query29	1082	700	542	542
query30	304	246	203	203
query31	1113	1079	947	947
query32	87	78	71	71
query33	554	371	312	312
query34	1170	1139	651	651
query35	790	811	669	669
query36	1319	1335	1188	1188
query37	153	108	92	92
query38	3207	3186	3016	3016
query39	937	922	886	886
query39_1	878	871	897	871
query40	232	169	148	148
query41	72	71	113	71
query42	109	107	106	106
query43	320	334	281	281
query44	
query45	212	203	192	192
query46	1096	1231	750	750
query47	2277	2310	2178	2178
query48	402	443	273	273
query49	630	543	432	432
query50	728	302	229	229
query51	4337	4280	4193	4193
query52	108	108	93	93
query53	250	285	209	209
query54	310	272	253	253
query55	92	91	83	83
query56	306	315	295	295
query57	1408	1381	1309	1309
query58	291	281	268	268
query59	1564	1692	1432	1432
query60	353	340	327	327
query61	160	156	158	156
query62	668	616	550	550
query63	237	210	204	204
query64	2339	834	696	696
query65	
query66	1678	526	392	392
query67	30073	30039	29827	29827
query68	
query69	461	345	307	307
query70	1040	999	955	955
query71	319	281	272	272
query72	2877	2806	2412	2412
query73	873	749	420	420
query74	5066	4961	4721	4721
query75	2846	2650	2317	2317
query76	2300	1199	781	781
query77	425	445	355	355
query78	12891	12918	12318	12318
query79	1606	940	738	738
query80	1361	616	497	497
query81	533	288	245	245
query82	988	163	124	124
query83	363	280	256	256
query84	261	141	107	107
query85	906	526	433	433
query86	450	337	323	323
query87	3421	3351	3265	3265
query88	3588	2670	2606	2606
query89	435	380	339	339
query90	1978	190	176	176
query91	180	165	143	143
query92	82	79	72	72
query93	1164	986	560	560
query94	723	337	300	300
query95	675	378	440	378
query96	1004	815	335	335
query97	2711	2707	2580	2580
query98	255	237	231	231
query99	1116	1117	987	987
Total cold run time: 256646 ms
Total hot run time: 170749 ms

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label May 12, 2026
@github-actions
Copy link
Copy Markdown
Contributor

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

@github-actions
Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@hello-stephen
Copy link
Copy Markdown
Contributor

skip check_coverage

@Mryange Mryange merged commit df8bf16 into apache:master May 12, 2026
35 of 36 checks passed
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. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants