Skip to content

Conversation

@Mryange
Copy link
Contributor

@Mryange Mryange commented Jul 24, 2025

…53660)
#53660
my_double_round did not handle NaN values correctly. For example, when dec is very large and value is 0, there will be a case of 0 * inf, resulting in a NaN value.
do_format_round assumes the input is always a valid double value, which causes a core dump when a NaN is passed in.

Additionally,
(value.size() - (is_positive ? (decimal_places + 2) : (decimal_places + 3))) / 3;
This code does not account for the situation where value.size() is 0.

Currently, a custom add_thousands_separator function is implemented. If the fmt library is upgraded in the future, we should use fmt to add thousands separators.

What problem does this PR solve?

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

@Mryange Mryange requested a review from dataroaring as a code owner July 24, 2025 10:40
@hello-stephen
Copy link
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
Contributor Author

Mryange commented Jul 24, 2025

run buildall

1 similar comment
@Mryange
Copy link
Contributor Author

Mryange commented Jul 24, 2025

run buildall

@Mryange Mryange force-pushed the branch-3.0-pick-53660 branch from bbe83e0 to a9682a8 Compare July 24, 2025 12:44
@Mryange
Copy link
Contributor Author

Mryange commented Jul 24, 2025

run buildall

@doris-robot
Copy link

BE UT Coverage Report

Increment line coverage 100.00% (1/1) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 41.90% (11136/26580)
Line Coverage 32.44% (95406/294129)
Region Coverage 31.58% (49282/156069)
Branch Coverage 28.01% (25261/90186)

@Mryange
Copy link
Contributor Author

Mryange commented Jul 24, 2025

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17615	6726	6645	6645
q2	2072	190	178	178
q3	10505	1135	1171	1135
q4	10220	755	692	692
q5	7765	2837	2824	2824
q6	215	131	132	131
q7	984	617	610	610
q8	9581	2026	2019	2019
q9	8221	6390	6438	6390
q10	7050	2274	2302	2274
q11	452	260	262	260
q12	417	208	224	208
q13	17790	2959	2972	2959
q14	240	205	206	205
q15	524	466	463	463
q16	474	385	368	368
q17	975	547	504	504
q18	7122	6568	6593	6568
q19	1403	1148	1093	1093
q20	481	197	202	197
q21	3855	3131	3142	3131
q22	1100	978	990	978
Total cold run time: 109061 ms
Total hot run time: 39832 ms

----- Round 2, with runtime_filter_mode=off -----
q1	6670	6615	6601	6601
q2	330	225	232	225
q3	2987	2935	2917	2917
q4	2092	1892	1814	1814
q5	5695	5722	5732	5722
q6	211	131	134	131
q7	2199	1823	1797	1797
q8	3319	3589	3512	3512
q9	8914	9086	8928	8928
q10	3672	3599	3554	3554
q11	586	512	490	490
q12	858	676	649	649
q13	6704	3169	3187	3169
q14	294	280	292	280
q15	527	475	461	461
q16	494	441	456	441
q17	1865	1627	1616	1616
q18	8160	7599	7717	7599
q19	1668	1561	1562	1561
q20	2078	1867	1864	1864
q21	5091	4964	5003	4964
q22	1167	1027	1029	1027
Total cold run time: 65581 ms
Total hot run time: 59322 ms

@doris-robot
Copy link

BE UT Coverage Report

Increment line coverage 87.50% (28/32) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 41.90% (11137/26581)
Line Coverage 32.44% (95410/294157)
Region Coverage 31.57% (49276/156069)
Branch Coverage 28.01% (25258/90186)

@doris-robot
Copy link

TPC-DS: Total hot run time: 196673 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 191ec056fa3a99c61afca770053b46fcb4e381b5, data reload: false

query1	1291	933	885	885
query2	6267	1930	1848	1848
query3	10950	4446	4615	4446
query4	33207	23450	23302	23302
query5	3404	463	458	458
query6	260	166	174	166
query7	3994	312	324	312
query8	273	207	217	207
query9	9488	2590	2566	2566
query10	490	264	249	249
query11	17798	15203	15204	15203
query12	159	102	102	102
query13	1547	431	427	427
query14	9426	6975	6661	6661
query15	245	184	189	184
query16	8099	533	514	514
query17	1590	602	597	597
query18	2142	315	312	312
query19	231	165	165	165
query20	123	120	110	110
query21	203	114	109	109
query22	4643	4445	4316	4316
query23	34759	34139	34202	34139
query24	12084	2967	2912	2912
query25	696	422	425	422
query26	1920	173	176	173
query27	3025	366	366	366
query28	7571	2186	2197	2186
query29	1048	468	485	468
query30	287	165	162	162
query31	1068	802	810	802
query32	92	54	56	54
query33	761	300	302	300
query34	998	536	524	524
query35	876	728	752	728
query36	1099	948	944	944
query37	282	66	68	66
query38	4154	4037	3968	3968
query39	1543	1452	1450	1450
query40	263	100	105	100
query41	50	52	47	47
query42	115	99	98	98
query43	519	462	486	462
query44	1284	839	833	833
query45	193	169	171	169
query46	1180	742	745	742
query47	2014	1922	1886	1886
query48	515	380	380	380
query49	1066	425	426	425
query50	833	442	434	434
query51	7411	7343	7348	7343
query52	102	88	91	88
query53	262	185	196	185
query54	1276	488	483	483
query55	80	82	78	78
query56	283	279	248	248
query57	1356	1221	1194	1194
query58	231	226	234	226
query59	3133	3017	2921	2921
query60	299	259	264	259
query61	115	115	111	111
query62	866	701	690	690
query63	226	188	189	188
query64	5269	677	627	627
query65	3386	3355	3268	3268
query66	1321	303	308	303
query67	15966	15580	15472	15472
query68	4862	591	578	578
query69	441	277	270	270
query70	1088	1155	1133	1133
query71	343	257	287	257
query72	6196	4097	4011	4011
query73	765	353	373	353
query74	10306	9126	9002	9002
query75	3361	2643	2669	2643
query76	2706	1102	1059	1059
query77	408	280	266	266
query78	10460	9638	9514	9514
query79	2592	628	612	612
query80	1044	436	445	436
query81	535	218	220	218
query82	1308	88	86	86
query83	252	141	137	137
query84	243	78	83	78
query85	1508	304	305	304
query86	468	301	306	301
query87	4424	4305	4277	4277
query88	4200	2419	2394	2394
query89	423	300	292	292
query90	1944	191	191	191
query91	181	147	150	147
query92	60	51	56	51
query93	2611	548	551	548
query94	785	288	296	288
query95	365	265	266	265
query96	621	284	282	282
query97	3274	3145	3134	3134
query98	215	201	196	196
query99	1532	1287	1287	1287
Total cold run time: 307228 ms
Total hot run time: 196673 ms

@doris-robot
Copy link

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

query1	0.03	0.03	0.03
query2	0.07	0.03	0.03
query3	0.23	0.07	0.06
query4	1.64	0.10	0.10
query5	0.51	0.54	0.54
query6	1.13	0.73	0.72
query7	0.04	0.01	0.01
query8	0.04	0.03	0.03
query9	0.57	0.51	0.49
query10	0.57	0.56	0.58
query11	0.14	0.09	0.10
query12	0.14	0.10	0.11
query13	0.62	0.59	0.59
query14	0.78	0.80	0.79
query15	0.84	0.84	0.82
query16	0.38	0.37	0.37
query17	1.07	1.06	0.96
query18	0.23	0.21	0.21
query19	1.96	1.80	1.80
query20	0.01	0.01	0.02
query21	15.40	0.59	0.57
query22	2.24	2.18	2.13
query23	17.19	0.97	0.87
query24	3.02	1.40	1.13
query25	0.25	0.23	0.19
query26	0.26	0.14	0.14
query27	0.05	0.05	0.04
query28	10.09	0.53	0.46
query29	12.56	3.24	3.22
query30	0.24	0.06	0.06
query31	2.85	0.38	0.37
query32	3.26	0.47	0.47
query33	3.00	2.97	3.03
query34	17.15	4.51	4.52
query35	4.59	4.58	4.54
query36	0.64	0.48	0.47
query37	0.09	0.06	0.06
query38	0.05	0.04	0.03
query39	0.04	0.03	0.02
query40	0.15	0.12	0.12
query41	0.08	0.02	0.02
query42	0.04	0.02	0.02
query43	0.03	0.02	0.02
Total cold run time: 104.27 s
Total hot run time: 30.74 s

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 github-actions bot added the approved Indicates a PR has been approved by one committer. label Aug 1, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2025

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

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2025

PR approved by anyone and no changes requested.

@dataroaring dataroaring merged commit 4c33269 into apache:branch-3.0 Aug 1, 2025
21 of 22 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.

4 participants