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.

Sign up for GitHub

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

Jump to bottom

types: fix FLOAT data overflow check (#20067) #20158

Merged
merged 5 commits into from
Sep 27, 2020
Merged

types: fix FLOAT data overflow check (#20067) #20158

merged 5 commits into from
Sep 27, 2020

Conversation

newcworld
Copy link
Contributor

What problem does this PR solve?

Issue Number: close #20067

Problem Summary:

Execute

DROP TABLE IF EXISTS t_5_1_1_1_float;
CREATE TABLE t_5_1_1_1_float ( col1 FLOAT, col2 FLOAT(10,2), col3 DOUBLE, col4 DOUBLE(10,2), col5 DECIMAL, col6  DECIMAL(10,2) ) ;
INSERT INTO t_5_1_1_1_float VALUES (-3.402823466E+68, -34028234.6611, -1.7976931348623157E+308, -17976921.34, -9999999999, -99999999.99);

TiDB 4.0.5

mysql> INSERT INTO t_5_1_1_1_float VALUES (-3.402823466E+68, -34028234.6611, -1.7976931348623157E+308, -17976921.34, -9999999999, -99999999.99);
Query OK, 1 row affected (0.01 sec)
mysql> SELECT * FROM t_5_1_1_1_float;
+------+--------------+-------------------------+--------------+-------------+--------------+
| col1 | col2         | col3                    | col4         | col5        | col6         |
+------+--------------+-------------------------+--------------+-------------+--------------+
| -Inf | -34028236.00 | -1.7976931348623157e308 | -17976921.34 | -9999999999 | -99999999.99 |
+------+--------------+-------------------------+--------------+-------------+--------------+
1 row in set (0.00 sec)

MySQL 5.7.29

mysql> INSERT INTO t_5_1_1_1_float VALUES (-3.402823466E+68, -34028234.6611, -1.7976931348623157E+308, -17976921.34, -9999999999, -99999999.99);
ERROR 1264 (22003): Out of range value for column 'col1' at row 1

What is changed and how it works?

What's Changed:

How it Works:

​ when the target column is TypeFloat, and the inserted value overflow (greater then math.MaxFloat32 or less than -math.MaxFloat32) return error

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Integration test

Release note

  • fix FLOAT data type: out of range data should not be inserted

@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

More

Tip : About reward you can refs to reward-command.

Warning: None

@ti-srebot ti-srebot added the contribution This PR is from a community contributor. label Sep 22, 2020
@CLAassistant
Copy link

CLAassistant commented Sep 22, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

More

Tip : About reward you can refs to reward-command.

Warning: None

@newcworld newcworld marked this pull request as ready for review September 22, 2020 17:52
@newcworld newcworld requested a review from a team as a code owner September 22, 2020 17:52
@newcworld newcworld requested review from fzhedu and removed request for a team September 22, 2020 17:52
@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

More

Tip : About reward you can refs to reward-command.

Warning: None

3 similar comments
@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

More

Tip : About reward you can refs to reward-command.

Warning: None

@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

More

Tip : About reward you can refs to reward-command.

Warning: None

@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

More

Tip : About reward you can refs to reward-command.

Warning: None

@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

More

Tip : About reward you can refs to reward-command.

Warning: None

1 similar comment
@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

More

Tip : About reward you can refs to reward-command.

Warning: None

SunRunAway
SunRunAway reviewed Sep 25, 2020
View reviewed changes
Copy link
Contributor

@SunRunAway SunRunAway 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 Sep 25, 2020
@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

More

Tip : About reward you can refs to reward-command.

Warning: None

@ti-challenge-bot
Copy link

There is no reward for this challenge pull request, so you can request a reward from @SunRunAway.

More

Tip : About reward you can refs to reward-command.

Warning: None

@SunRunAway
Copy link
Contributor

/reward 300

@ti-challenge-bot
Copy link

Reward success.

lzmhhh123
lzmhhh123 reviewed Sep 27, 2020
View reviewed changes
Copy link
Contributor

@lzmhhh123 lzmhhh123 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 removed the status/LGT1 Indicates that a PR has LGTM 1. label Sep 27, 2020
@SunRunAway
Copy link
Contributor

/run-check_dev_2

@SunRunAway
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

Your auto merge job has been accepted, waiting for:

  • 20190
  • 20238

@SunRunAway
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

Your auto merge job has been accepted, waiting for:

  • 20190
  • 20238
  • 20190

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit fd0a18a into pingcap:master Sep 27, 2020
@ti-challenge-bot
Copy link

@pengdaqian2020, Congratulations, you get 300 in this PR, and your total score is 300 in high-performance challenge program.

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Sep 27, 2020
@newcworld @ti-srebot
cherry pick pingcap#20158 to release-3.0
238e732 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Sep 27, 2020
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-3.0 in PR #20251

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Sep 27, 2020
@newcworld @ti-srebot
cherry pick pingcap#20158 to release-4.0
d2c03e9 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Sep 27, 2020
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #20252

zz-jason pushed a commit that referenced this pull request Oct 3, 2020
@ti-srebot
types: fix FLOAT data overflow check (#20067) (#20158) (#20252)
9518e90
zz-jason pushed a commit that referenced this pull request Oct 4, 2020
@ti-srebot
types: fix FLOAT data overflow check (#20067) (#20158) (#20251)
6566ba4
SunRunAway pushed a commit to SunRunAway/tidb that referenced this pull request Oct 15, 2020
@newcworld @SunRunAway
types: fix FLOAT data overflow check (pingcap#20067) (pingcap#20158)
e9495ab
@SunRunAway SunRunAway mentioned this pull request Oct 15, 2020
Merged
bb7133 added a commit to bb7133/tidb that referenced this pull request Oct 23, 2020
@bb7133
types: fix FLOAT data overflow check (pingcap#20067) (pingcap#20158)
bbb3705
This was referenced Nov 9, 2020
Merged
Closed
@lzmhhh123 lzmhhh123 mentioned this pull request Nov 9, 2020
Closed
This was referenced Dec 23, 2020
Closed
Merged
@erwadba erwadba mentioned this pull request Jan 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lzmhhh123 lzmhhh123 lzmhhh123 left review comments

@SunRunAway SunRunAway SunRunAway left review comments

@ti-srebot ti-srebot ti-srebot approved these changes

@fzhedu fzhedu Awaiting requested review from fzhedu

Assignees
No one assigned
Labels
component/expression contribution This PR is from a community contributor. sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FLOAT data type: out of range data should not be inserted
5 participants
@newcworld @CLAassistant @SunRunAway @lzmhhh123 @ti-srebot