You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "feiniaofeiafei (via GitHub)" <gi...@apache.org> on 2026/10/15 03:21:19 UTC
[PR] [fix](eager-agg) Preserve decimal scale in eager SUM compensation [doris]
feiniaofeiafei opened a new pull request, #69093:
URL: https://github.com/apache/doris/pull/69093
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Eager aggregation restores join multiplicity by multiplying partial SUM values by opposite-side counts. Ordinary decimal multiplication can reduce the scale according to `decimal_overflow_scale`.
This causes either:
- An output type mismatch when an upper projection still expects the original SUM type.
- Silent precision loss when the reduced-scale result reaches the query output.
For example, a valid SUM of `0.0000000004` could become `0.000000`.
This Pr fix this problem:
- Introduce an internal decimal multiplication expression that preserves the SUM result type.
- Cast the compensation input to the original SUM type when necessary.
- Preserve multiplication statistics estimation and prevent arithmetic simplification or FE constant folding from reverting to ordinary decimal multiplication semantics.
- Reuse the existing BE multiplication and overflow checks. No BE changes are required.
### 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: https://github.com/apache/doris-website/pull/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 -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](eager-agg) Preserve decimal scale in eager SUM compensation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69093:
URL: https://github.com/apache/doris/pull/69093#issuecomment-6184681847
<details>
<summary>ClickBench: <b>Total hot run time: 24.52 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 20668955808a9fa8d8664a0955fcc0a6f7c1ff60, data reload: false
query1 0.01 0.01 0.00
query2 0.10 0.04 0.04
query3 0.27 0.12 0.12
query4 1.61 0.13 0.12
query5 0.28 0.24 0.24
query6 1.19 0.68 0.68
query7 0.03 0.01 0.01
query8 0.05 0.04 0.04
query9 0.40 0.35 0.34
query10 0.57 0.58 0.55
query11 0.20 0.14 0.14
query12 0.18 0.14 0.14
query13 0.49 0.51 0.50
query14 1.01 1.03 1.01
query15 0.64 0.59 0.60
query16 0.35 0.35 0.34
query17 1.14 1.15 1.15
query18 0.22 0.20 0.18
query19 2.17 2.00 2.02
query20 0.02 0.01 0.01
query21 15.46 0.25 0.14
query22 4.52 0.06 0.05
query23 16.30 0.32 0.12
query24 3.23 0.62 0.39
query25 0.11 0.06 0.06
query26 1.11 0.25 0.15
query27 0.04 0.03 0.03
query28 1.91 0.60 0.41
query29 12.50 4.14 3.33
query30 0.28 0.14 0.13
query31 2.77 0.56 0.33
query32 3.22 0.62 0.52
query33 3.20 3.15 3.26
query34 15.34 4.02 3.39
query35 3.37 3.34 3.37
query36 0.54 0.45 0.43
query37 0.10 0.07 0.07
query38 0.05 0.04 0.03
query39 0.04 0.03 0.03
query40 0.17 0.14 0.15
query41 0.08 0.03 0.03
query42 0.03 0.02 0.02
query43 0.04 0.03 0.04
Total cold run time: 95.34 s
Total hot run time: 24.52 s
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](eager-agg) Preserve decimal scale in eager SUM compensation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69093:
URL: https://github.com/apache/doris/pull/69093#issuecomment-6184513640
<details>
<summary>TPC-H: <b>Total hot run time: 29406 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 20668955808a9fa8d8664a0955fcc0a6f7c1ff60, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17616 5687 5555 5555
q2 2076 307 261 261
q3 10260 1434 860 860
q4 4684 483 349 349
q5 7441 808 534 534
q6 189 186 161 161
q7 768 809 604 604
q8 9246 1378 1087 1087
q9 5490 4340 4275 4275
q10 6824 1307 1013 1013
q11 444 266 243 243
q12 638 402 281 281
q13 18119 3645 3059 3059
q14 275 276 244 244
q15 q16 741 729 679 679
q17 1214 754 575 575
q18 7182 6271 6203 6203
q19 1250 964 633 633
q20 389 357 234 234
q21 5258 2286 2418 2286
q22 377 316 270 270
Total cold run time: 100481 ms
Total hot run time: 29406 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 6190 6154 6162 6154
q2 697 587 573 573
q3 5254 5721 4936 4936
q4 2040 2168 1601 1601
q5 5290 5568 5986 5568
q6 263 202 162 162
q7 2398 2146 1897 1897
q8 3373 2985 2835 2835
q9 8126 8092 8142 8092
q10 3780 3718 3368 3368
q11 688 465 406 406
q12 678 711 513 513
q13 3600 3944 3276 3276
q14 317 323 295 295
q15 q16 748 719 651 651
q17 8393 7628 7427 7427
q18 13140 12387 13114 12387
q19 923 797 802 797
q20 2188 2151 1947 1947
q21 5971 4766 4982 4766
q22 503 460 394 394
Total cold run time: 74560 ms
Total hot run time: 68045 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](eager-agg) Preserve decimal scale in eager SUM compensation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69093:
URL: https://github.com/apache/doris/pull/69093#issuecomment-6185912235
# FE Regression Coverage Report
Increment line coverage ` 66.67% (12/18)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69093_20668955808a9fa8d8664a0955fcc0a6f7c1ff60_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69093_20668955808a9fa8d8664a0955fcc0a6f7c1ff60_merge_fe/report/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](eager-agg) Preserve decimal scale in eager SUM compensation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69093:
URL: https://github.com/apache/doris/pull/69093#issuecomment-6184719142
# FE UT Coverage Report
Increment line coverage ` 78.57% (11/14)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69093_20668955808a9fa8d8664a0955fcc0a6f7c1ff60/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69093_20668955808a9fa8d8664a0955fcc0a6f7c1ff60/fe_report/index.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](eager-agg) Preserve decimal scale in eager SUM compensation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69093:
URL: https://github.com/apache/doris/pull/69093#issuecomment-6183644209
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://doris.apache.org/community/how-to-contribute/review-and-ci).
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](eager-agg) Preserve decimal scale in eager SUM compensation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69093:
URL: https://github.com/apache/doris/pull/69093#issuecomment-6184627355
<details>
<summary>TPC-DS: <b>Total hot run time: 153196 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 20668955808a9fa8d8664a0955fcc0a6f7c1ff60, data reload: false
query5 4294 599 441 441
query6 424 203 203 203
query7 4806 447 235 235
query8 321 167 153 153
query9 8776 4042 4023 4023
query10 457 331 266 266
query11 5918 3774 3513 3513
query12 146 91 88 88
query13 1251 458 327 327
query14 6180 4069 3886 3886
query14_1 3559 3549 3585 3549
query15 215 195 182 182
query16 939 440 421 421
query17 862 662 547 547
query18 2426 418 321 321
query19 195 172 138 138
query20 90 84 83 83
query21 209 136 121 121
query22 12993 13108 12868 12868
query23 13540 12879 12797 12797
query23_1 12523 12679 12678 12678
query24 7125 817 488 488
query24_1 476 488 474 474
query25 544 424 366 366
query26 1275 262 147 147
query27 2771 441 273 273
query28 4617 1960 1963 1960
query29 1589 594 452 452
query30 297 224 191 191
query31 1016 928 831 831
query32 152 107 93 93
query33 518 308 243 243
query34 911 838 523 523
query35 755 798 646 646
query36 812 824 737 737
query37 131 97 92 92
query38 1801 1742 1701 1701
query39 795 813 751 751
query39_1 757 738 740 738
query40 213 113 97 97
query41 65 64 66 64
query42 86 86 84 84
query43 365 393 336 336
query44 1331 705 696 696
query45 185 180 160 160
query46 841 948 575 575
query47 2996 2984 2952 2952
query48 310 303 211 211
query49 577 398 316 316
query50 687 272 208 208
query51 10705 10441 10480 10441
query52 78 79 69 69
query53 189 205 150 150
query54 250 200 204 200
query55 77 68 74 68
query56 227 241 208 208
query57 1603 1581 1563 1563
query58 292 267 287 267
query59 2093 2168 1967 1967
query60 281 239 221 221
query61 147 141 136 136
query62 394 337 292 292
query63 206 163 167 163
query64 2745 920 791 791
query65 3321 3264 3250 3250
query66 1775 425 345 345
query67 20646 20178 19795 19795
query68 2971 1003 619 619
query69 385 307 253 253
query70 867 800 754 754
query71 288 233 201 201
query72 2793 2622 2354 2354
query73 504 534 303 303
query74 4597 4431 4302 4302
query75 2334 2321 1944 1944
query76 2287 1025 613 613
query77 355 402 280 280
query78 9943 9728 9211 9211
query79 1023 840 520 520
query80 535 432 336 336
query81 544 318 275 275
query82 576 137 113 113
query83 296 199 185 185
query84 315 124 101 101
query85 821 427 377 377
query86 320 247 231 231
query87 2017 1951 1806 1806
query88 3619 2716 2676 2676
query89 350 300 267 267
query90 1908 179 183 179
query91 162 149 126 126
query92 99 87 86 86
query93 1024 1006 587 587
query94 462 291 305 291
query95 618 361 391 361
query96 671 513 234 234
query97 2455 2450 2315 2315
query98 165 153 146 146
query99 773 765 645 645
Total cold run time: 235027 ms
Total hot run time: 153196 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](eager-agg) Preserve decimal scale in eager SUM compensation [doris]
Posted by "feiniaofeiafei (via GitHub)" <gi...@apache.org>.
feiniaofeiafei commented on PR #69093:
URL: https://github.com/apache/doris/pull/69093#issuecomment-6490711936
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](eager-agg) Preserve decimal scale in eager SUM compensation [doris]
Posted by "feiniaofeiafei (via GitHub)" <gi...@apache.org>.
feiniaofeiafei commented on PR #69093:
URL: https://github.com/apache/doris/pull/69093#issuecomment-6183651966
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](eager-agg) Preserve decimal scale in eager SUM compensation [doris]
Posted by "924060929 (via GitHub) via commits" <co...@doris.apache.org>.
924060929 commented on code in PR #69093:
URL: https://github.com/apache/doris/pull/69093#discussion_r4439965509
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/DecimalMultiplyForSum.java:
##########
@@ -0,0 +1,62 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.expressions;
+
+import org.apache.doris.analysis.ArithmeticExpr.Operator;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.DecimalV3Type;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+
+import java.util.List;
+
+/**
+ * Restore SUM's join multiplicity without reducing its decimal scale. Unlike ordinary
+ * multiplication, multiplying a partial sum by an integer count must preserve SUM's type.
+ * Keep this separate from Multiply so arithmetic simplification cannot rebuild it with
+ * the ordinary decimal overflow-scale rules. BE executes the existing multiply function.
+ */
+public class DecimalMultiplyForSum extends BinaryArithmetic {
+ public DecimalMultiplyForSum(Expression sum, Expression count) {
+ super(ImmutableList.of(sum, count), Operator.MULTIPLY);
+ }
+
+ @Override
+ public Expression withChildren(List<Expression> children) {
+ Preconditions.checkArgument(children.size() == 2);
+ return new DecimalMultiplyForSum(children.get(0), children.get(1));
+ }
+
+ @Override
+ public DecimalV3Type getDataTypeForDecimalV3(DecimalV3Type sumType, DecimalV3Type countType) {
+ Preconditions.checkArgument(countType.getScale() == 0);
+ return sumType;
Review Comment:
[P2] Preserve intermediate range until the final SUM
With eager_aggregation_mode=1, enable_decimal256=false, check_overflow_for_decimal=true and decimal_overflow_scale=6, consider l(g,k,v DECIMAL(38,18)) containing (G,a,+60000000000000000000) and (G,b,-60000000000000000000), and r(k,w) containing two rows with w=1 for each key. `SELECT l.g,SUM(l.v),SUM(r.w) FROM l JOIN [shuffle] r ON l.k=r.k GROUP BY l.g` should sum to zero. Returning sumType makes each compensation product ±120000000000000000000 undergo the DECIMAL(38,18) range check in BE multiply.cpp:487, throwing before cancellation in the final aggregate. The products still fit Int128; the merge-base multiplication rescales to DECIMAL(38,6) and this case can complete. Preserve sufficient intermediate range through aggregation, or decline the rewrite when that cannot be guaranteed, and add a cancellation regression. This is a static finding; no tests were run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org