Skip to content

[fix](be) Remove pure attribute from assert_cast#63417

Merged
BiteTheDDDDt merged 1 commit into
apache:masterfrom
BiteTheDDDDt:fix-assert-cast-pure
May 20, 2026
Merged

[fix](be) Remove pure attribute from assert_cast#63417
BiteTheDDDDt merged 1 commit into
apache:masterfrom
BiteTheDDDDt:fix-assert-cast-pure

Conversation

@BiteTheDDDDt
Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: assert_cast can throw doris::Exception when release type checking detects an invalid cast. The function was annotated with PURE, but throwing an exception is an observable side effect and changes control flow, so the annotation gives the compiler an invalid optimization contract for the failure path. This removes the incorrect annotation and leaves the existing cast logic unchanged.

Release note

None

Check List (For Author)

  • Test: Manual test
    • git diff --cached --check
  • Behavior changed: No
  • Does this need documentation: No

### What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: assert_cast may throw doris::Exception when release type checking detects an invalid cast. The function was annotated with PURE even though throwing an exception is an observable side effect and changes control flow. Remove the incorrect PURE annotation so compilers generate normal exception handling for assert_cast failure paths.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - git diff --cached --check
- Behavior changed: No
- Does this need documentation: No

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 20, 2026 02:14
@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

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an incorrect compiler optimization contract by removing the PURE attribute from assert_cast, since assert_cast can throw doris::Exception on invalid casts (observable side effect / control-flow change), making __attribute__((pure)) semantically invalid for this function.

Changes:

  • Removed the PURE attribute annotation from assert_cast to avoid incorrect compiler assumptions/optimizations on exception paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@BiteTheDDDDt
Copy link
Copy Markdown
Contributor Author

/review

@BiteTheDDDDt
Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen
Copy link
Copy Markdown
Contributor

TPC-H: Total hot run time: 31460 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit ba7c6eb49818615f222ee42792ee111f7737f478, 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	17611	3991	3937	3937
q2	q3	10783	1435	862	862
q4	4679	488	341	341
q5	7569	2304	2139	2139
q6	246	177	138	138
q7	986	759	638	638
q8	9387	1778	1612	1612
q9	5216	4906	4933	4906
q10	6401	2239	1835	1835
q11	459	286	240	240
q12	624	427	293	293
q13	18124	3367	2760	2760
q14	267	261	238	238
q15	q16	824	787	705	705
q17	911	840	959	840
q18	7141	5721	5553	5553
q19	1195	1256	1066	1066
q20	509	571	321	321
q21	6162	2946	2662	2662
q22	463	374	431	374
Total cold run time: 99557 ms
Total hot run time: 31460 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	4983	4672	4678	4672
q2	q3	4957	5228	4607	4607
q4	2122	2295	1456	1456
q5	4737	4886	4636	4636
q6	234	174	128	128
q7	1884	1810	1571	1571
q8	2444	2082	2092	2082
q9	7865	7531	7263	7263
q10	4452	4376	3982	3982
q11	534	381	349	349
q12	725	722	510	510
q13	3023	3431	2814	2814
q14	283	275	246	246
q15	q16	677	700	619	619
q17	1279	1254	1251	1251
q18	7295	6877	6768	6768
q19	1134	1108	1114	1108
q20	2227	2212	1932	1932
q21	5341	4620	4543	4543
q22	534	469	405	405
Total cold run time: 56730 ms
Total hot run time: 50942 ms

@hello-stephen
Copy link
Copy Markdown
Contributor

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

query5	4326	645	524	524
query6	328	239	202	202
query7	4233	594	300	300
query8	338	238	219	219
query9	8820	4036	3994	3994
query10	454	341	301	301
query11	5787	2404	2153	2153
query12	182	134	126	126
query13	1297	630	447	447
query14	5976	5370	5076	5076
query14_1	4358	4375	4315	4315
query15	208	203	180	180
query16	1017	447	438	438
query17	1138	718	592	592
query18	2576	487	346	346
query19	214	208	160	160
query20	135	130	129	129
query21	217	134	116	116
query22	13642	13579	13350	13350
query23	17166	16409	16019	16019
query23_1	16166	16247	16180	16180
query24	7452	1773	1281	1281
query24_1	1290	1313	1304	1304
query25	570	476	411	411
query26	1335	329	175	175
query27	2691	571	341	341
query28	4445	1954	1939	1939
query29	1013	627	503	503
query30	309	244	199	199
query31	1139	1057	926	926
query32	83	78	73	73
query33	557	368	301	301
query34	1161	1129	637	637
query35	754	777	675	675
query36	1327	1377	1164	1164
query37	150	108	97	97
query38	3214	3137	3022	3022
query39	937	915	905	905
query39_1	864	883	881	881
query40	235	151	127	127
query41	68	62	77	62
query42	122	118	111	111
query43	319	322	288	288
query44	
query45	214	200	196	196
query46	1047	1155	731	731
query47	2334	2351	2160	2160
query48	406	418	299	299
query49	629	503	388	388
query50	981	365	248	248
query51	4349	4351	4251	4251
query52	105	105	97	97
query53	247	288	204	204
query54	314	268	252	252
query55	94	93	85	85
query56	294	310	295	295
query57	1402	1409	1307	1307
query58	293	269	267	267
query59	1549	1690	1475	1475
query60	321	326	307	307
query61	167	154	159	154
query62	670	632	549	549
query63	251	204	206	204
query64	2420	853	702	702
query65	
query66	1748	487	368	368
query67	30021	29954	29884	29884
query68	
query69	491	356	321	321
query70	1045	1049	940	940
query71	312	279	277	277
query72	3170	2932	2538	2538
query73	825	759	429	429
query74	5035	4899	4736	4736
query75	2675	2605	2246	2246
query76	2304	1129	775	775
query77	397	394	330	330
query78	12301	12305	11567	11567
query79	1461	1039	718	718
query80	633	541	440	440
query81	462	281	246	246
query82	1364	159	123	123
query83	360	281	246	246
query84	257	144	118	118
query85	887	540	449	449
query86	386	330	315	315
query87	3421	3654	3210	3210
query88	3504	2637	2632	2632
query89	542	382	334	334
query90	1934	189	186	186
query91	182	166	142	142
query92	83	79	75	75
query93	1512	1484	869	869
query94	556	351	325	325
query95	687	516	353	353
query96	1064	833	351	351
query97	2724	2727	2565	2565
query98	239	228	226	226
query99	1127	1089	995	995
Total cold run time: 253518 ms
Total hot run time: 169170 ms

@hello-stephen
Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (1/1) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 73.64% (27921/37914)
Line Coverage 57.59% (302766/525745)
Region Coverage 54.68% (252988/462687)
Branch Coverage 56.25% (109351/194403)

@BiteTheDDDDt BiteTheDDDDt merged commit f4ce70e into apache:master May 20, 2026
31 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants