Skip to content

[fix](function) Round Decimal values with zero decimal places - #68010

Merged
Mryange merged 1 commit into
apache:masterfrom
Mryange:fix-format-round-zero-decimals
Sep 17, 2026
Merged

Mryange merged 1 commit into
apache:masterfrom
Mryange:fix-format-round-zero-decimals

Conversation

@Mryange

@Mryange Mryange commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

format_round skipped rounding for Decimal values when decimal_places was zero because the rounding branch required a positive decimal-place count. Values such as 1.9 and -1.9 therefore produced truncated results instead of 2 and -2. The condition now performs rounding whenever the input scale exceeds the requested decimal places, including zero, and regression coverage verifies positive, negative, and table-column Decimal values.

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: N/A

Problem Summary: FORMAT_ROUND skipped rounding for Decimal inputs when decimal_places was zero because the rounding branch required a positive decimal-place count. Round whenever the input scale exceeds the requested decimal places and add regression coverage for zero decimal places.

### Release note

Fix FORMAT_ROUND for Decimal inputs with zero decimal places.

### Check List (For Author)

- Test: Regression test
- Behavior changed: Yes; Decimal values are rounded correctly when decimal_places is zero.
- Does this need documentation: No
@Mryange

Mryange commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

/review

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

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Static review opinion: LGTM; no PR-introduced blocking issue found.

Critical checkpoint conclusions:

  • Goal and correctness: the changed predicate now runs the existing Decimal fractional reduction when zero places are requested. For DecimalV2/32/64/128, the zero-place quotient is one digit, half-away-from-zero behavior is preserved, sign/carry handling is correct, and the carry plus output buffer remain in range.
  • Scope and parallel paths: this is a focused one-condition fix. Double and integer behavior is unchanged, and all implemented Decimal widths share the corrected helper. The registered Decimal256 executor gap was independently verified as real but pre-existing at the base SHA for every place count; Decimal256 never reaches the changed helper, so it is not a finding against this PR.
  • Tests: the changed expected results are correct and cover positive/negative Decimal32 carry/no-carry plus a Decimal128 column path. Exact-half, sub-unit, DecimalV2/64, all-9s, and row-varying-place cases would strengthen coverage, but static tracing found no defect in those paths. Per the review contract, no build or test was run here, so the regression result is not independently executed.
  • Concurrency, lifecycle, compatibility, and operations: no shared state, locking, ownership, configuration, FE-BE protocol, persistence, storage format, transaction, observability, or ABI behavior changes. Existing null and Status propagation remain intact.
  • Performance: the added constant-time exponent lookup and division occur only where required for correct zero-place Decimal rounding; no material allocation or asymptotic regression was found.
  • User focus: no additional focus was supplied.

Review completion: the main risk scan and two normal/risk convergence rounds completed; both Round 2 reviewers returned NO_NEW_VALUABLE_FINDINGS, every candidate was adjudicated, the final duplicate fence is empty, and the live PR head was verified at 1c2a288 before submission.

@Mryange

Mryange commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@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 76.21% (34340/45058)
Line Coverage 61.13% (385577/630707)
Region Coverage 57.55% (324722/564233)
Branch Coverage 58.33% (147849/253478)

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17562	3033	3034	3033
q2	2115	262	226	226
q3	10212	884	509	509
q4	4673	255	202	202
q5	7681	647	402	402
q6	136	117	93	93
q7	536	503	394	394
q8	9229	839	882	839
q9	3458	2434	2454	2434
q10	6503	895	736	736
q11	393	206	183	183
q12	614	257	195	195
q13	18144	1569	1164	1164
q14	159	152	141	141
q15	q16	439	405	382	382
q17	1281	813	731	731
q18	3172	2321	2313	2313
q19	1119	940	801	801
q20	376	279	201	201
q21	5370	1723	1853	1723
q22	329	274	221	221
Total cold run time: 93501 ms
Total hot run time: 16923 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3366	3311	3286	3286
q2	523	401	373	373
q3	2255	2416	2158	2158
q4	1243	1191	908	908
q5	2249	2197	2185	2185
q6	172	119	88	88
q7	1030	906	867	867
q8	1596	1376	1392	1376
q9	3216	3216	3198	3198
q10	1933	1855	1673	1673
q11	366	271	255	255
q12	457	435	345	345
q13	1487	1556	1162	1162
q14	180	177	174	174
q15	q16	395	400	369	369
q17	3672	3412	3284	3284
q18	4919	4566	4876	4566
q19	990	853	861	853
q20	1029	995	836	836
q21	3887	3242	3239	3239
q22	404	350	308	308
Total cold run time: 35369 ms
Total hot run time: 31503 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 83273 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 1c2a288bf18f06f063a0aaf6e896d2f2e406fa68, data reload: false

query5	4252	434	338	338
query6	388	138	127	127
query7	4926	410	237	237
query8	285	122	118	118
query9	8681	2928	2912	2912
query10	406	250	183	183
query11	5399	1041	926	926
query12	133	71	69	69
query13	1184	443	313	313
query14	6145	2237	2126	2126
query14_1	2028	2011	2020	2011
query15	169	119	114	114
query16	925	366	371	366
query17	778	439	337	337
query18	2333	317	235	235
query19	179	137	110	110
query20	82	71	72	71
query21	200	107	88	88
query22	5678	5483	5452	5452
query23	6966	6473	6217	6217
query23_1	6188	6315	6341	6315
query24	7329	1098	750	750
query24_1	769	759	777	759
query25	431	299	249	249
query26	1216	226	133	133
query27	2792	383	264	264
query28	4704	1514	1493	1493
query29	949	424	347	347
query30	257	164	132	132
query31	815	422	347	347
query32	135	75	71	71
query33	495	222	177	177
query34	1008	865	470	470
query35	420	395	355	355
query36	576	584	525	525
query37	120	84	72	72
query38	1010	862	826	826
query39	493	479	494	479
query39_1	504	481	484	481
query40	210	93	82	82
query41	58	58	56	56
query42	75	77	74	74
query43	240	241	215	215
query44	1012	541	538	538
query45	121	107	101	101
query46	749	836	543	543
query47	769	765	714	714
query48	315	304	237	237
query49	539	254	193	193
query50	749	290	201	201
query51	8159	8103	8066	8066
query52	73	78	97	78
query53	196	191	146	146
query54	218	151	154	151
query55	75	62	55	55
query56	200	167	160	160
query57	721	700	679	679
query58	200	170	171	170
query59	1254	1271	1127	1127
query60	247	180	152	152
query61	111	151	120	120
query62	346	208	179	179
query63	176	139	138	138
query64	2766	743	613	613
query65	1714	1677	1659	1659
query66	1780	260	222	222
query67	10275	9848	9820	9820
query68	3039	1224	700	700
query69	338	217	201	201
query70	663	630	621	621
query71	259	165	164	164
query72	2256	1640	1506	1506
query73	635	581	341	341
query74	2006	1251	1166	1166
query75	1215	1128	988	988
query76	2402	715	564	564
query77	265	243	219	219
query78	4252	3749	3258	3258
query79	1245	819	570	570
query80	1205	316	281	281
query81	492	155	132	132
query82	657	129	98	98
query83	289	210	196	196
query84	297	114	86	86
query85	777	322	277	277
query86	397	179	168	168
query87	1047	986	914	914
query88	2766	2113	2092	2092
query89	291	197	170	170
query90	1984	126	128	126
query91	127	119	95	95
query92	79	73	69	69
query93	1241	1077	752	752
query94	731	257	235	235
query95	513	335	233	233
query96	808	591	298	298
query97	1104	1045	1029	1029
query98	141	138	135	135
query99	414	350	308	308
Total cold run time: 178063 ms
Total hot run time: 83273 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 14.81 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 1c2a288bf18f06f063a0aaf6e896d2f2e406fa68, data reload: false

query1	0.01	0.00	0.00
query2	0.08	0.04	0.03
query3	0.26	0.10	0.11
query4	1.62	0.10	0.11
query5	0.18	0.16	0.15
query6	1.22	0.67	0.69
query7	0.03	0.01	0.00
query8	0.05	0.03	0.03
query9	0.29	0.22	0.22
query10	0.36	0.35	0.35
query11	0.16	0.12	0.11
query12	0.15	0.12	0.12
query13	0.30	0.31	0.31
query14	0.46	0.46	0.46
query15	0.36	0.36	0.37
query16	0.24	0.24	0.24
query17	0.73	0.70	0.71
query18	0.18	0.16	0.17
query19	1.24	1.19	1.21
query20	0.01	0.01	0.01
query21	15.49	0.15	0.11
query22	5.01	0.05	0.04
query23	16.13	0.26	0.10
query24	2.92	0.30	0.26
query25	0.11	0.04	0.03
query26	0.80	0.16	0.12
query27	0.04	0.03	0.02
query28	3.62	0.56	0.29
query29	12.45	3.25	2.56
query30	0.25	0.11	0.12
query31	2.75	0.40	0.16
query32	3.51	0.33	0.23
query33	1.55	1.42	1.44
query34	15.39	2.23	1.79
query35	1.81	1.78	1.75
query36	0.47	0.30	0.29
query37	0.06	0.04	0.04
query38	0.04	0.03	0.03
query39	0.03	0.02	0.03
query40	0.13	0.08	0.07
query41	0.07	0.03	0.02
query42	0.03	0.02	0.02
query43	0.04	0.03	0.02
Total cold run time: 90.63 s
Total hot run time: 14.81 s

@Mryange
Mryange merged commit 8002067 into apache:master Sep 17, 2026
37 checks passed
github-actions Bot pushed a commit that referenced this pull request Sep 23, 2026
`format_round` skipped rounding for Decimal values when `decimal_places`
was zero because the rounding branch required a positive decimal-place
count. Values such as `1.9` and `-1.9` therefore produced truncated
results instead of `2` and `-2`. The condition now performs rounding
whenever the input scale exceeds the requested decimal places, including
zero, and regression coverage verifies positive, negative, and
table-column Decimal values.


### Release note

None

### Check List (For Author)

- Test <!-- At least one of them must be included. -->
    - [ ] 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 <!-- Add your reason?  -->

- Behavior changed:
    - [ ] No.
    - [ ] Yes. <!-- Explain the behavior change -->

- Does this need documentation?
    - [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
apache/doris-website#1214 -->

### Check List (For Reviewer who merge this PR)

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
yiguolei pushed a commit that referenced this pull request Sep 24, 2026
…aces #68010 (#68412)

Cherry-picked from #68010

Co-authored-by: Mryange <yanxuecheng@selectdb.com>
yiguolei pushed a commit that referenced this pull request Sep 25, 2026
…aces #68010 (#68412)

Cherry-picked from #68010

Co-authored-by: Mryange <yanxuecheng@selectdb.com>
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