executor: avoid sum from avg overflow (#30010) by ti-srebot · Pull Request #30378 · pingcap/tidb

ti-srebot · 2021-12-03T05:46:22Z

cherry-pick #30010 to release-5.2
You can switch your code base to this Pull Request by using git-extras:

# In tidb repo:
git pr https://github.com/pingcap/tidb/pull/30378

After apply modifications, you can push your change to this PR via:

git push git@github.com:ti-srebot/tidb.git pr/30378:release-5.2-9aa756336e7b

What problem does this PR solve?

Issue Number: close #29952

Problem Summary:

the sum from avg may overflow, we should infer the type as the normal sum.
the previous PR expression, planner: fix decimal results for aggregate functions #20017 is a hack to fix the issue , we should let the final agg output the target return type which is inferred in TypeInfe() for Agg.
typeInfer4xxx should valid the precision and scale of decimal type.

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-chi-bot · 2021-12-03T05:46:23Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

hawkingrei
windtalker

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

ti-srebot · 2021-12-03T05:46:25Z

/run-all-tests

ti-srebot · 2021-12-03T05:46:31Z

@fzhedu you're already a collaborator in bot's repo.

hawkingrei · 2021-12-03T06:45:34Z

 	"github.com/pingcap/parser/ast"
 	"github.com/pingcap/parser/mysql"
+=======
+>>>>>>> 9aa756336... executor: avoid sum from avg overflow (#30010)


need to remove it.

fzhedu · 2022-01-05T03:41:12Z

/run-check_dev

fzhedu · 2022-01-06T05:48:12Z

/run-mysql-test

fzhedu · 2022-01-07T06:05:57Z

/run-mysql-test

fzhedu · 2022-01-07T08:03:01Z

/run-mysql-test

hawkingrei · 2022-01-07T08:21:15Z

/run-mysql-test

hawkingrei · 2022-01-07T08:22:55Z

/run-mysql-test

windtalker

LGTM

fzhedu · 2022-01-07T08:49:34Z

/merge

ti-chi-bot · 2022-01-07T08:49:37Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 48fd09a

fzhedu · 2022-01-07T09:04:27Z

/run-mysql-test

hawkingrei · 2022-01-07T09:06:53Z

/run-mysql-test

purelind · 2022-01-07T09:36:55Z

/run-mysql-test

purelind · 2022-01-07T09:41:42Z

/run-mysql-test

purelind · 2022-01-07T10:13:07Z

/run-mysql-test

fzhedu · 2022-01-07T14:41:53Z

/run-mysql-test

fzhedu · 2022-01-10T06:30:40Z

/run-mysql-test

LittleFall · 2022-09-19T07:30:10Z

/rebuild

LittleFall · 2022-09-19T07:40:16Z

/merge

ti-chi-bot · 2022-09-19T07:40:18Z

@LittleFall: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

Details

In response to this:

/merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

LittleFall · 2022-09-19T07:49:11Z

/rebuild

cherry pick pingcap#30010 to release-5.2

1e133f4

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-chi-bot added do-not-merge/cherry-pick-not-approved do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 3, 2021

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 3, 2021

ti-srebot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/5.2-cherry-pick labels Dec 3, 2021

ti-srebot mentioned this pull request Dec 3, 2021

executor: avoid sum from avg overflow #30010

Merged

12 tasks

ti-srebot requested review from XuHuaiyu, lzmhhh123, windtalker and winoros December 3, 2021 05:46

ti-srebot added this to the v5.2.2 milestone Dec 3, 2021

ti-srebot assigned fzhedu Dec 3, 2021

ti-chi-bot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 3, 2021

ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 3, 2021

hawkingrei reviewed Dec 3, 2021

View reviewed changes

fzhedu added 3 commits January 4, 2022 13:37

Update integration_serial_suite_out.json

702abf5

Update builder.go

05b2f49

updated

87a5164

fzhedu requested a review from hawkingrei January 4, 2022 06:00

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 4, 2022

update test format

c4dff97

enlarge timeout

050e2e4

solve mysql tests

48fd09a

windtalker approved these changes Jan 7, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jan 7, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 7, 2022

VelocityLight added cherry-pick-approved Cherry pick PR approved by release team. and removed do-not-merge/cherry-pick-not-approved labels Sep 19, 2022

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 19, 2022

Merge branch 'release-5.2' into release-5.2-9aa756336e7b

c1a5054

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 19, 2022

LittleFall approved these changes Sep 19, 2022

View reviewed changes

ti-chi-bot merged commit 1582c54 into pingcap:release-5.2 Sep 19, 2022

LittleFall deleted the release-5.2-9aa756336e7b branch September 19, 2022 08:00

Conversation

ti-srebot commented Dec 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Dec 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ti-srebot commented Dec 3, 2021

Uh oh!

ti-srebot commented Dec 3, 2021

Uh oh!

hawkingrei Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

fzhedu Jan 4, 2022

Choose a reason for hiding this comment

Uh oh!

fzhedu commented Jan 5, 2022

Uh oh!

fzhedu commented Jan 6, 2022

Uh oh!

fzhedu commented Jan 7, 2022

Uh oh!

fzhedu commented Jan 7, 2022

Uh oh!

hawkingrei commented Jan 7, 2022

Uh oh!

hawkingrei commented Jan 7, 2022

Uh oh!

windtalker left a comment

Choose a reason for hiding this comment

Uh oh!

fzhedu commented Jan 7, 2022

Uh oh!

ti-chi-bot commented Jan 7, 2022

Uh oh!

fzhedu commented Jan 7, 2022

Uh oh!

hawkingrei commented Jan 7, 2022

Uh oh!

purelind commented Jan 7, 2022

Uh oh!

purelind commented Jan 7, 2022

Uh oh!

purelind commented Jan 7, 2022

Uh oh!

fzhedu commented Jan 7, 2022

Uh oh!

fzhedu commented Jan 10, 2022

Uh oh!

LittleFall commented Sep 19, 2022

Uh oh!

LittleFall commented Sep 19, 2022

Uh oh!

ti-chi-bot commented Sep 19, 2022

Uh oh!

LittleFall commented Sep 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ti-srebot commented Dec 3, 2021 •

edited

Loading

ti-chi-bot commented Dec 3, 2021 •

edited

Loading