Skip to content

Conversation

@kaijchen
Copy link
Member

What problem does this PR solve?

Followup #52042
Modifying hdr.load_id() is unsafe and may cause undefined behavior.
Also passing load_id by reference in constructors.

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

@Thearas
Copy link
Contributor

Thearas commented Jun 26, 2025

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?

@kaijchen
Copy link
Member Author

run buildall

@hello-stephen
Copy link
Contributor

BE UT Coverage Report

Increment line coverage 100.00% (4/4) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 57.05% (15365/26932)
Line Coverage 46.14% (139415/302164)
Region Coverage 45.47% (70637/155364)
Branch Coverage 40.23% (37318/92760)

dataroaring
dataroaring previously approved these changes Jun 26, 2025
Copy link
Contributor

@dataroaring dataroaring 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
Copy link
Contributor

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

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Jun 26, 2025
@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

hello-stephen
hello-stephen previously approved these changes Jun 27, 2025
@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17625	5199	5058	5058
q2	1963	299	194	194
q3	10427	1335	769	769
q4	10239	1053	577	577
q5	7753	2383	2353	2353
q6	184	156	131	131
q7	892	765	618	618
q8	9338	1322	1123	1123
q9	6840	5090	5153	5090
q10	6920	2399	1981	1981
q11	483	313	298	298
q12	366	364	229	229
q13	17774	3718	3192	3192
q14	228	233	228	228
q15	573	483	481	481
q16	439	435	408	408
q17	665	873	402	402
q18	7759	7414	7018	7018
q19	1707	974	580	580
q20	378	367	242	242
q21	4325	3726	3207	3207
q22	1051	1018	970	970
Total cold run time: 107929 ms
Total hot run time: 35149 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5248	5111	5092	5092
q2	256	326	224	224
q3	2190	2714	2345	2345
q4	1397	1790	1423	1423
q5	4468	4351	4424	4351
q6	219	166	127	127
q7	1970	2000	1788	1788
q8	2625	2565	2513	2513
q9	7135	7232	7211	7211
q10	3132	3294	2894	2894
q11	587	533	494	494
q12	718	807	682	682
q13	3597	3966	3391	3391
q14	281	302	279	279
q15	533	495	477	477
q16	459	504	442	442
q17	1136	1498	1354	1354
q18	7463	7122	7032	7032
q19	821	891	1124	891
q20	1942	2003	1874	1874
q21	4953	4476	4508	4476
q22	1086	1053	1019	1019
Total cold run time: 52216 ms
Total hot run time: 50379 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 188660 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 229945264ab118aedcf5581aca229e9429b3efc7, data reload: false

query1	1026	393	393	393
query2	6560	1872	1893	1872
query3	6747	235	224	224
query4	26436	23782	22886	22886
query5	4349	645	462	462
query6	319	231	206	206
query7	4626	545	314	314
query8	292	241	239	239
query9	8644	2930	2925	2925
query10	503	340	292	292
query11	15338	15448	14884	14884
query12	163	114	106	106
query13	1666	580	454	454
query14	9518	6169	5989	5989
query15	222	208	177	177
query16	7683	678	497	497
query17	1233	783	627	627
query18	2053	441	328	328
query19	208	213	183	183
query20	133	124	118	118
query21	212	131	111	111
query22	4055	4201	4203	4201
query23	34217	33169	33275	33169
query24	8554	2476	2528	2476
query25	564	484	422	422
query26	1223	298	160	160
query27	2672	562	366	366
query28	4335	2341	2296	2296
query29	766	603	449	449
query30	298	229	195	195
query31	972	884	760	760
query32	73	65	64	64
query33	584	379	322	322
query34	843	940	559	559
query35	849	830	781	781
query36	1060	1074	917	917
query37	121	100	85	85
query38	4336	4309	4212	4212
query39	1511	1425	1420	1420
query40	219	124	110	110
query41	58	57	54	54
query42	135	116	116	116
query43	545	561	518	518
query44	1439	904	899	899
query45	183	194	164	164
query46	937	1118	683	683
query47	1787	1841	1741	1741
query48	424	444	336	336
query49	759	492	420	420
query50	700	760	454	454
query51	4255	4257	4147	4147
query52	127	116	122	116
query53	253	277	210	210
query54	624	617	540	540
query55	88	84	87	84
query56	330	335	316	316
query57	1195	1249	1123	1123
query58	280	277	284	277
query59	2817	2941	2762	2762
query60	355	353	340	340
query61	126	127	124	124
query62	841	782	686	686
query63	238	202	203	202
query64	4233	1038	688	688
query65	4394	4202	4288	4202
query66	1103	470	321	321
query67	15801	15894	15534	15534
query68	8941	970	582	582
query69	483	325	293	293
query70	1359	1236	1183	1183
query71	469	358	333	333
query72	5716	4880	5169	4880
query73	765	753	390	390
query74	9302	9278	8704	8704
query75	4164	3425	2784	2784
query76	3611	1280	791	791
query77	811	404	316	316
query78	10130	10344	9415	9415
query79	2189	915	636	636
query80	663	550	472	472
query81	476	270	236	236
query82	430	142	110	110
query83	300	272	265	265
query84	302	112	89	89
query85	780	381	318	318
query86	347	330	306	306
query87	4527	4562	4376	4376
query88	3244	2477	2444	2444
query89	434	346	306	306
query90	1953	293	221	221
query91	141	145	116	116
query92	78	61	59	59
query93	1185	1026	655	655
query94	685	436	304	304
query95	388	304	298	298
query96	554	636	309	309
query97	2835	2815	2666	2666
query98	251	217	213	213
query99	1530	1457	1282	1282
Total cold run time: 278253 ms
Total hot run time: 188660 ms

@doris-robot
Copy link

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

query1	0.05	0.03	0.04
query2	0.08	0.04	0.04
query3	0.24	0.08	0.07
query4	1.61	0.11	0.11
query5	0.44	0.45	0.42
query6	1.15	0.67	0.67
query7	0.03	0.02	0.02
query8	0.05	0.04	0.04
query9	0.62	0.54	0.51
query10	0.58	0.57	0.58
query11	0.17	0.12	0.11
query12	0.16	0.12	0.12
query13	0.65	0.62	0.63
query14	0.83	0.84	0.83
query15	0.92	0.88	0.89
query16	0.39	0.40	0.40
query17	1.07	1.07	1.11
query18	0.25	0.23	0.22
query19	2.03	1.85	1.92
query20	0.02	0.02	0.02
query21	15.37	0.94	0.58
query22	0.77	1.31	0.69
query23	14.78	1.50	0.67
query24	9.39	0.79	0.41
query25	0.44	0.15	0.08
query26	0.54	0.18	0.15
query27	0.07	0.06	0.06
query28	8.95	0.85	0.47
query29	12.53	4.12	3.46
query30	0.26	0.10	0.06
query31	2.83	0.63	0.43
query32	3.26	0.57	0.48
query33	3.07	3.16	3.22
query34	15.97	5.36	4.79
query35	4.86	4.80	4.85
query36	0.69	0.52	0.50
query37	0.10	0.06	0.06
query38	0.05	0.05	0.04
query39	0.04	0.03	0.03
query40	0.18	0.15	0.14
query41	0.08	0.02	0.03
query42	0.04	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 105.65 s
Total hot run time: 29.74 s

@kaijchen kaijchen dismissed stale reviews from hello-stephen and dataroaring via cc91de7 June 27, 2025 06:22
@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Jun 27, 2025
@hello-stephen
Copy link
Contributor

skip buildall

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jun 27, 2025
@github-actions
Copy link
Contributor

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

@hello-stephen hello-stephen merged commit 956ba9c into apache:master Jun 27, 2025
29 of 31 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 27, 2025
### What problem does this PR solve?

Followup #52042 
Modifying `hdr.load_id()` is unsafe and may cause undefined behavior.
Also passing load_id by reference in constructors.
github-actions bot pushed a commit that referenced this pull request Jun 27, 2025
### What problem does this PR solve?

Followup #52042 
Modifying `hdr.load_id()` is unsafe and may cause undefined behavior.
Also passing load_id by reference in constructors.
dataroaring pushed a commit that referenced this pull request Jun 27, 2025
Cherry-picked from #52339

Co-authored-by: Kaijie Chen <chenkaijie@selectdb.com>
morrySnow pushed a commit that referenced this pull request Jun 30, 2025
Cherry-picked from #52339

Co-authored-by: Kaijie Chen <chenkaijie@selectdb.com>
wyxxxcat pushed a commit to wyxxxcat/doris that referenced this pull request Jun 30, 2025
Followup apache#52042
Modifying `hdr.load_id()` is unsafe and may cause undefined behavior.
Also passing load_id by reference in constructors.
koarz pushed a commit to koarz/doris that referenced this pull request Jul 3, 2025
…ache#52426)

Cherry-picked from apache#52339

Co-authored-by: Kaijie Chen <chenkaijie@selectdb.com>
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.7-merged dev/3.1.0-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants