Skip to content

Conversation

@shuke987
Copy link
Collaborator

Proposed changes

fcc reports error: comparing the result of pointer addition when compile
if (places[i] + offset)

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@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.

@shuke987
Copy link
Collaborator Author

run buildall

@shuke987
Copy link
Collaborator Author

run buildall

@github-actions
Copy link
Contributor

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

@shuke987
Copy link
Collaborator Author

run performance

@github-actions
Copy link
Contributor

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

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.27% (8728/24746)
Line Coverage: 27.08% (71435/263762)
Region Coverage: 26.32% (37067/140817)
Branch Coverage: 23.24% (18957/81588)
Coverage Report: http://coverage.selectdb-in.cc/coverage/ecfaf5f0cf74c1f40363c55b63e3b056ff0b9937_ecfaf5f0cf74c1f40363c55b63e3b056ff0b9937/report/index.html

if constexpr (ShowNull::value) {
for (size_t i = 0; i != num_rows; ++i) {
if (places[i] + offset) {
if (places[i] || offset) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not change the logic while fix compile error.

offset is added from pr #32387, please have a review @amorynan

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

place[i] is a pointer, and offset is an unsigned int, so if placces[i] != null or offset != 0, equals places[i] + offset != null

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe the correct answer is like: if(places[i] && offset < places[i].size()), check offset != 0 or == 0 is strange here.

Copy link
Contributor

Choose a reason for hiding this comment

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

here below we use places[i]+offset as an pointer. so maybe places[i] + offset != nullptr is correct and more readable

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17619	4327	4102	4102
q2	2025	153	152	152
q3	10582	1112	1122	1112
q4	10242	777	793	777
q5	7454	2963	2885	2885
q6	200	120	121	120
q7	1027	566	550	550
q8	9318	1973	1962	1962
q9	7154	6459	6397	6397
q10	8374	3322	3496	3322
q11	426	223	211	211
q12	407	197	193	193
q13	17782	2824	2809	2809
q14	236	197	207	197
q15	521	450	455	450
q16	453	360	348	348
q17	949	640	578	578
q18	7141	6460	6429	6429
q19	5222	1394	1455	1394
q20	535	271	249	249
q21	3515	2967	2736	2736
q22	344	294	286	286
Total cold run time: 111526 ms
Total hot run time: 37259 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4144	4112	4038	4038
q2	316	240	220	220
q3	2984	2831	2795	2795
q4	1826	1573	1560	1560
q5	5203	5216	5219	5216
q6	199	114	116	114
q7	2216	1843	1833	1833
q8	3131	3263	3256	3256
q9	8602	8606	8622	8606
q10	3758	3718	3660	3660
q11	536	448	432	432
q12	715	552	561	552
q13	16909	2869	2896	2869
q14	264	242	245	242
q15	481	441	442	441
q16	458	412	402	402
q17	1738	1492	1458	1458
q18	7469	7264	7284	7264
q19	1602	1539	1520	1520
q20	1908	1726	1751	1726
q21	4819	4633	4569	4569
q22	530	445	420	420
Total cold run time: 69808 ms
Total hot run time: 53193 ms

@doris-robot
Copy link

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

query1	912	354	333	333
query2	7497	2064	2137	2064
query3	6711	216	228	216
query4	31418	20495	20528	20495
query5	4306	408	407	407
query6	281	170	181	170
query7	4635	294	297	294
query8	228	165	176	165
query9	9984	2318	2297	2297
query10	451	240	248	240
query11	17356	14154	13958	13958
query12	137	84	87	84
query13	1635	418	430	418
query14	10990	10515	10594	10515
query15	260	202	196	196
query16	7914	268	251	251
query17	1942	556	554	554
query18	1967	269	263	263
query19	207	148	149	148
query20	87	82	88	82
query21	200	126	134	126
query22	4680	4403	4465	4403
query23	31665	31053	30887	30887
query24	10767	2822	2765	2765
query25	625	378	382	378
query26	1281	151	152	151
query27	2543	357	355	355
query28	7738	1891	1871	1871
query29	917	677	623	623
query30	303	145	146	145
query31	951	717	733	717
query32	96	56	55	55
query33	775	258	255	255
query34	1100	478	485	478
query35	834	610	604	604
query36	979	870	899	870
query37	130	78	73	73
query38	3591	3417	3423	3417
query39	1446	1372	1382	1372
query40	206	114	112	112
query41	52	50	45	45
query42	103	94	94	94
query43	463	446	466	446
query44	1350	705	712	705
query45	285	254	253	253
query46	1096	732	710	710
query47	1664	1598	1585	1585
query48	451	359	354	354
query49	1131	337	339	337
query50	752	375	367	367
query51	6603	6704	6644	6644
query52	107	86	91	86
query53	346	277	272	272
query54	311	247	238	238
query55	82	80	76	76
query56	248	237	234	234
query57	1090	996	1004	996
query58	231	204	206	204
query59	2767	2570	2478	2478
query60	273	258	267	258
query61	114	113	109	109
query62	594	423	406	406
query63	308	277	273	273
query64	5730	3863	3923	3863
query65	3044	2984	3022	2984
query66	868	353	346	346
query67	14983	14404	14481	14404
query68	5976	524	527	524
query69	588	363	372	363
query70	1211	1118	1182	1118
query71	487	285	292	285
query72	6492	2656	2461	2461
query73	731	321	323	321
query74	7143	6550	6580	6550
query75	3946	2822	2779	2779
query76	4274	924	955	924
query77	682	266	268	266
query78	10431	9585	9537	9537
query79	8744	518	510	510
query80	2017	386	384	384
query81	554	214	214	214
query82	1638	203	193	193
query83	298	144	144	144
query84	290	79	79	79
query85	1616	332	317	317
query86	482	284	298	284
query87	3687	3514	3522	3514
query88	5102	2434	2406	2406
query89	525	356	365	356
query90	1977	180	173	173
query91	165	136	136	136
query92	63	50	45	45
query93	6940	508	496	496
query94	1203	180	174	174
query95	425	321	336	321
query96	610	276	273	273
query97	3076	2869	2862	2862
query98	229	210	199	199
query99	1106	761	737	737
Total cold run time: 307499 ms
Total hot run time: 180519 ms

@shuke987 shuke987 closed this Mar 21, 2024
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.

4 participants