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: fix builtin IF truncation of type len #20743

Merged
merged 9 commits into from Nov 19, 2020
Merged

expression: fix builtin IF truncation of type len #20743

merged 9 commits into from Nov 19, 2020

Conversation

ghost
Copy link

@ghost ghost commented Oct 30, 2020

What problem does this PR solve?

Issue Number: Fixes #20730

Problem Summary:

When one of the values of the built-in function if is an expression, the type length is calculated incorrectly. This leads to values being truncated in user variables, which could lead to wrong results.

What is changed and how it works?

What's Changed:

If the length is unknown (-1) the length of mysql.MaxRealWidth is returned instead. This is the behavior observed in MySQL.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test

Side effects

  • Breaking backward compatibility

Release note

  • Using a combination of the built-in IF function and user variables could lead to wrong results.

@ghost ghost self-requested a review as a code owner October 30, 2020 18:46
@ghost ghost requested review from qw4990 and removed request for a team October 30, 2020 18:46
@ghost
Copy link
Author

ghost commented Oct 30, 2020

/run-all-tests

@ghost
Copy link
Author

ghost commented Oct 30, 2020

/run-unit-test

1 similar comment
@ghost
Copy link
Author

ghost commented Oct 30, 2020

/run-unit-test

@ghost ghost added the status/PTAL label Nov 10, 2020
@ichn-hu ichn-hu mentioned this pull request Nov 10, 2020
ichn-hu
ichn-hu previously approved these changes Nov 11, 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 the status/LGT1 Indicates that a PR has LGTM 1. label Nov 11, 2020
@ti-srebot
Copy link
Contributor

@qw4990, @ichn-hu, PTAL.

@ti-srebot
Copy link
Contributor

@qw4990, @ichn-hu, PTAL.

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 Nov 19, 2020
@lzmhhh123
Copy link
Contributor

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 19, 2020
@ti-srebot
Copy link
Contributor

Your auto merge job has been accepted, waiting for:

  • 21127
  • 20927
  • 21125
  • 20544

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@nullnotnil merge failed.

@lzmhhh123
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

Your auto merge job has been accepted, waiting for:

  • 20867

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@nullnotnil merge failed.

@lzmhhh123 lzmhhh123 merged commit 42d0208 into pingcap:master Nov 19, 2020
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Nov 19, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #21144

@ghost ghost deleted the fix-builtin-if branch November 19, 2020 16:36
ti-srebot added a commit that referenced this pull request Nov 30, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Builtin IF truncates return type when containing user variables
4 participants