Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expression: change the round rule for approximate value to round to nearest even (#21324) #21628

Merged
merged 3 commits into from
Jan 26, 2021

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #21324 to release-4.0


What problem does this PR solve?

Issue Number: close #1108

Problem Summary:
The Round rule for approximate-value numbers is “round half away from zero”, which is different from the “round to nearest even” rule used by MySQL.

What is changed and how it works?

What's Changed:
The round rule for approximate-value numbers is changed to “round to nearest even” rule.

How it Works:
select cast(25E-1 as signed)

+-----------------------+
| cast(25E-1 as signed) |
+-----------------------+
| 2 |
+-----------------------+

Related changes

  • Need to cherry-pick to the release branch

Tests

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

Side effects

  • Performance regression
    • Consumes more CPU
    • Consumes more MEM
  • Breaking backward compatibility

Release note

  • Use “round to nearest even” rule instead of “round half away from zero” for approximate-value numbers

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot added component/expression first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. type/4.0-cherry-pick type/bugfix This PR fixes a bug. labels Dec 10, 2020
@ti-srebot ti-srebot added this to the 4.0.0 milestone Dec 10, 2020
@ti-srebot
Copy link
Contributor Author

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

Copy link
Contributor

@wshwsh12 wshwsh12 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 10, 2020
Copy link
Contributor

@ichn-hu ichn-hu left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 10, 2020
Copy link
Member

@wjhuang2016 wjhuang2016 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Dec 10, 2020
@jebter jebter modified the milestones: 4.0.0, v4.0.10 Jan 7, 2021
@jebter jebter modified the milestones: v4.0.10, v4.0.11 Jan 18, 2021
@qw4990
Copy link
Contributor

qw4990 commented Jan 26, 2021

/merge

@ti-srebot
Copy link
Contributor Author

Sorry @qw4990, this branch cannot be merged without an approval of release maintainers.

@ti-srebot ti-srebot added status/LGT4 The PR has already had 4 LGTM. and removed status/LGT3 The PR has already had 3 LGTM. labels Jan 26, 2021
@qw4990
Copy link
Contributor

qw4990 commented Jan 26, 2021

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 26, 2021
@ti-srebot
Copy link
Contributor Author

Your auto merge job has been accepted, waiting for:

  • 22527
  • 22481

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot merged commit 89622d1 into pingcap:release-4.0 Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. status/can-merge Indicates a PR has been approved by a committer. status/LGT4 The PR has already had 4 LGTM. type/bugfix This PR fixes a bug. type/4.0-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants