Skip to content

[refactor](be) Guard redundant column and typeid casts#64063

Open
Mryange wants to merge 1 commit into
apache:masterfrom
Mryange:typeid-column-cast-guard
Open

[refactor](be) Guard redundant column and typeid casts#64063
Mryange wants to merge 1 commit into
apache:masterfrom
Mryange:typeid-column-cast-guard

Conversation

@Mryange
Copy link
Copy Markdown
Contributor

@Mryange Mryange commented Jun 3, 2026

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: Add compile-time checks to reject redundant same-type typeid_cast and check_and_get_column usage. This keeps dynamic column probing available for IColumn-derived source types while making already-concrete same-type calls fail at compile time instead of silently adding unnecessary casts.

Release note

None

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

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

### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: Add compile-time checks to reject redundant same-type typeid_cast and check_and_get_column usage. This keeps dynamic column probing available for IColumn-derived source types while making already-concrete same-type calls fail at compile time instead of silently adding unnecessary casts.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - sh run-be-ut.sh --clean -j 48
- Behavior changed: No
- Does this need documentation: No
@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?

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented Jun 3, 2026

run buildall

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented Jun 3, 2026

/review

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.

Reviewed the actual GitHub PR diff for PR 64063, which contains only be/src/core/column/column.h and be/src/core/typeid_cast.h. I did not find a blocking issue.

Critical checkpoint conclusions:

  • Goal/test proof: the code implements compile-time rejection of redundant same-type typeid_cast / check_and_get_column calls. The stated testing is BE unit tests from the PR description; I did not rerun them in this review.
  • Scope/focus: the actual PR diff is focused on BE column/typeid cast helpers.
  • Concurrency: no concurrency behavior is involved.
  • Lifecycle/static initialization: no static/global lifecycle issue identified.
  • Configuration: no config changes.
  • Compatibility/storage format: no protocol or storage format change.
  • Parallel code paths: both typeid_cast and the check_and_get_column wrappers were updated consistently.
  • Conditional/error handling: no new runtime error path; the change is compile-time assertions.
  • Test coverage: no new test file is added, but the change is primarily compile-time enforcement. Residual risk is compile breakage in less-covered call sites that intentionally used same-type probing.
  • Observability: not applicable.
  • Transaction/persistence/data writes: not applicable.
  • FE/BE variable passing: not applicable.
  • Performance: no runtime performance regression identified; static assertions do not add runtime cost.
  • Other issues: no additional issue found.

User focus points: no additional focus points were provided.

@hello-stephen
Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 100.00% (38/38) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 54.23% (21153/39005)
Line Coverage 37.78% (200923/531881)
Region Coverage 33.90% (157911/465793)
Branch Coverage 34.85% (69000/198016)

@hello-stephen
Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (38/38) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 72.04% (27511/38191)
Line Coverage 55.52% (294545/530501)
Region Coverage 52.37% (246229/470208)
Branch Coverage 53.46% (106254/198764)

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented Jun 3, 2026

run external

@hello-stephen
Copy link
Copy Markdown
Contributor

TPC-H: Total hot run time: 29357 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit b650ecf0c08fb8a40133c7369267b5e2eeab29a4, 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	17667	4060	4055	4055
q2	q3	10764	1487	836	836
q4	4688	477	351	351
q5	7582	892	585	585
q6	186	172	136	136
q7	785	880	639	639
q8	9390	1541	1500	1500
q9	5810	4496	4516	4496
q10	6802	1833	1544	1544
q11	434	275	263	263
q12	636	431	290	290
q13	18111	3404	2755	2755
q14	260	257	246	246
q15	q16	825	782	712	712
q17	1004	898	1040	898
q18	6931	5751	5706	5706
q19	1365	1277	1139	1139
q20	524	416	260	260
q21	6339	2898	2618	2618
q22	475	388	328	328
Total cold run time: 100578 ms
Total hot run time: 29357 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	5064	4730	4824	4730
q2	q3	4862	5364	4753	4753
q4	2149	2194	1416	1416
q5	4792	4831	4771	4771
q6	230	178	130	130
q7	1832	1752	1619	1619
q8	2464	2123	2100	2100
q9	8256	7739	7486	7486
q10	4732	4696	4263	4263
q11	536	386	358	358
q12	734	745	522	522
q13	2999	3388	2851	2851
q14	286	282	249	249
q15	q16	684	736	617	617
q17	1280	1264	1250	1250
q18	7644	6839	6704	6704
q19	1133	1118	1111	1111
q20	2217	2217	1952	1952
q21	5312	4598	4538	4538
q22	520	460	412	412
Total cold run time: 57726 ms
Total hot run time: 51832 ms

@hello-stephen
Copy link
Copy Markdown
Contributor

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

query5	4333	631	492	492
query6	449	197	195	195
query7	4850	579	332	332
query8	373	221	200	200
query9	8737	3981	4011	3981
query10	455	314	263	263
query11	5937	2317	2198	2198
query12	153	107	100	100
query13	1267	609	450	450
query14	6423	5398	5061	5061
query14_1	4427	4395	4385	4385
query15	218	208	177	177
query16	1036	469	443	443
query17	948	712	613	613
query18	2420	466	335	335
query19	196	176	135	135
query20	107	106	105	105
query21	211	136	119	119
query22	13584	13519	13371	13371
query23	17300	16553	16175	16175
query23_1	16203	16319	16325	16319
query24	7591	1772	1290	1290
query24_1	1297	1281	1329	1281
query25	542	452	381	381
query26	1311	301	165	165
query27	2735	600	340	340
query28	4494	2028	2056	2028
query29	1069	623	478	478
query30	322	232	196	196
query31	1131	1074	955	955
query32	113	61	59	59
query33	510	305	244	244
query34	1158	1160	659	659
query35	763	781	690	690
query36	1379	1368	1258	1258
query37	156	108	90	90
query38	3199	3138	3041	3041
query39	943	930	914	914
query39_1	888	890	882	882
query40	226	128	98	98
query41	63	63	61	61
query42	94	101	99	99
query43	321	316	275	275
query44	
query45	199	188	178	178
query46	1069	1245	729	729
query47	2395	2349	2230	2230
query48	385	392	289	289
query49	647	483	362	362
query50	998	362	269	269
query51	4305	4291	4207	4207
query52	88	88	77	77
query53	239	270	193	193
query54	268	226	197	197
query55	78	73	70	70
query56	229	220	212	212
query57	1426	1384	1356	1356
query58	296	219	209	209
query59	1564	1635	1431	1431
query60	283	252	215	215
query61	161	152	158	152
query62	679	667	577	577
query63	232	186	179	179
query64	2590	799	626	626
query65	
query66	1803	508	344	344
query67	29664	29769	29537	29537
query68	
query69	434	302	262	262
query70	946	932	951	932
query71	299	221	208	208
query72	3058	2760	2399	2399
query73	861	788	439	439
query74	5100	4947	4773	4773
query75	2659	2570	2277	2277
query76	2292	1149	778	778
query77	356	373	289	289
query78	12301	12403	11805	11805
query79	1305	1089	770	770
query80	542	507	413	413
query81	442	290	249	249
query82	237	162	127	127
query83	284	281	255	255
query84	300	148	122	122
query85	926	600	527	527
query86	336	307	278	278
query87	3328	3355	3181	3181
query88	3632	2788	2770	2770
query89	432	384	343	343
query90	2151	187	183	183
query91	195	185	157	157
query92	67	65	57	57
query93	1539	1491	877	877
query94	557	367	361	361
query95	679	423	455	423
query96	1072	798	326	326
query97	2673	2690	2580	2580
query98	214	209	209	209
query99	1163	1179	1031	1031
Total cold run time: 250381 ms
Total hot run time: 169494 ms

@hello-stephen
Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (38/38) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 72.04% (27511/38191)
Line Coverage 55.52% (294546/530501)
Region Coverage 52.37% (246270/470208)
Branch Coverage 53.46% (106260/198764)

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented Jun 4, 2026

run external

@hello-stephen
Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (38/38) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 73.79% (28180/38191)
Line Coverage 57.86% (306944/530501)
Region Coverage 54.78% (257564/470208)
Branch Coverage 56.13% (111568/198764)

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented Jun 4, 2026

run external

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants