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

types: Regard TypeNewDecimal as not a hasVariantFieldLength type. (#21849) #21960

Merged
merged 3 commits into from
Jan 8, 2021

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #21849 to release-4.0


What problem does this PR solve?

Issue Number: close #21804

Problem Summary:

Tiflash will check whether Decimal's flen is between 1 and 65. But TiDB enable will push down a Decimal constant with flen -1 from the environment parameter.

What is changed and how it works?

What's Changed:

When creating a decimal variable from Parameter, don't set it's flen to -1.

Other types in hasVariantFieldLength don't have this problem.

Related changes

  • Need to cherry-pick to the release 4.0

Check List

Tests

  • Unit test
    (will be added to tiflash repo)
  • Integration test
  • Manual test (add detailed scripts or steps below)
create table t (c_decimal decimal);
alter table t set tiflash replica 1; -- wait ready.
insert into t values (9), (10);
prepare s1 from 'select * from t where ? <= c_decimal';
set @v1 = 9.330;
execute s1 using @v1;
+-----------+
| c_decimal |
+-----------+
|        10 |
+-----------+

test for issue/8644

mysql> create table t(d decimal(3,2));
mysql> alter table t set tiflash replica 1; --wait ready

mysql> insert into t values (1.11), (2.222);
Query OK, 2 rows affected, 1 warning (0.03 sec) -- the warning is: Truncated incorrect DECIMAL value: '2.222'
mysql> insert into t values (33.3);
ERROR 1264 (22003): Out of range value for column 'd' at row 1

mysql> insert into t values ('1.11'), ('2.222');
Query OK, 2 rows affected, 1 warning (0.01 sec)
mysql> insert into t values ('33.3');
ERROR 1264 (22003): Out of range value for column 'd' at row 1

mysql> prepare s1 from 'insert t (d) values (?), (?)';
mysql> prepare s2 from 'insert t (d) values (?)';

mysql> set @a='1.11', @b='2.222', @c='33.3';
mysql> execute s1 using @a, @b;
Query OK, 2 rows affected, 1 warning (0.01 sec)
mysql> execute s2 using @c;
ERROR 1264 (22003): Out of range value for column 'd' at row 1

mysql> set @a=1.11, @b=2.222, @c=33.3;
mysql> execute s1 using @a, @b;
Query OK, 2 rows affected, 1 warning (0.02 sec)
mysql> execute s2 using @c;
ERROR 1264 (22003): Out of range value for column 'd' at row 1

Side effects

  • N/A

Release note

  • Fixed the issue that using prepare statement to cache execution plan caused tiflash to return Wrong precision.

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

/run-all-tests

@ti-srebot
Copy link
Contributor Author

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

@LittleFall
Copy link
Contributor

/run-unit-test

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

@eurekaka eurekaka 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
Copy link
Contributor Author

@eurekaka, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: execution(slack).

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 added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 23, 2020
@LittleFall
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor Author

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

@jebter jebter modified the milestones: 4.0.0, v4.0.10 Jan 7, 2021
@eurekaka
Copy link
Contributor

eurekaka commented Jan 8, 2021

/merge

@ti-srebot
Copy link
Contributor Author

Sorry @eurekaka, this branch's release version is in progress, please contact zhouqiang-cl,shuke987,jebter,you06 for more details.

@eurekaka
Copy link
Contributor

eurekaka commented Jan 8, 2021

/run-all-tests

@eurekaka eurekaka merged commit c2c767a into pingcap:release-4.0 Jan 8, 2021
@eurekaka eurekaka deleted the release-4.0-0370fbf6a389 branch January 8, 2021 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression status/LGT2 Indicates that a PR has LGTM 2. 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