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

expression: handle tp.flen overflow in to_base64 function (#20947) #21813

Merged
merged 5 commits into from
Jan 25, 2021
Merged

expression: handle tp.flen overflow in to_base64 function (#20947) #21813

merged 5 commits into from
Jan 25, 2021

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #20947 to release-4.0

What problem does this PR solve?

Issue Number: Fixes #14349

Problem Summary:

The builtin function to_base64 previously returned NULL when the tp.flen exceeded the maxbloblen. But it's possible that the length could exceed this very easily by using a type such as longtext, even though based on TiDB's current limits it wouldn't be possible to construct a row-size this length.

What is changed and how it works?

What's Changed:

This changes the behavior of to_base64 to be similar to other built-ins, where overflow is handled by setting the tp.Flen to the mysql.MaxBlobWidth.

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Integration test

Side effects

  • None expected

Release note

  • When using input types longer than blob (such as a longblob or longtext), the function to_base64 always returned NULL. It now returns the correct value.

@ti-srebot
cherry pick pingcap#20947 to release-4.0
737308e 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot mentioned this pull request Dec 15, 2020
Merged
@ti-srebot ti-srebot added this to the 4.0.0 milestone Dec 15, 2020
@ti-srebot ti-srebot assigned ghost Dec 15, 2020
@ti-srebot
Copy link
Contributor Author

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

wshwsh12
wshwsh12 reviewed Dec 16, 2020
View reviewed changes
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 16, 2020
@ti-srebot
Copy link
Contributor Author

@wshwsh12, @SunRunAway, @XuHuaiyu, @wjhuang2016, @lzmhhh123, PTAL.

2 similar comments
@ti-srebot
Copy link
Contributor Author

@wshwsh12, @SunRunAway, @XuHuaiyu, @wjhuang2016, @lzmhhh123, PTAL.

@ti-srebot
Copy link
Contributor Author

@wshwsh12, @SunRunAway, @XuHuaiyu, @wjhuang2016, @lzmhhh123, PTAL.

lzmhhh123
lzmhhh123 approved these changes Dec 23, 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 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
@ti-srebot
Copy link
Contributor Author

@wshwsh12, @lzmhhh123, @SunRunAway, @XuHuaiyu, @wjhuang2016, PTAL.

@ti-srebot
Copy link
Contributor Author

@wshwsh12, @lzmhhh123, @SunRunAway, @XuHuaiyu, @wjhuang2016, PTAL.

1 similar comment
@ti-srebot
Copy link
Contributor Author

@wshwsh12, @lzmhhh123, @SunRunAway, @XuHuaiyu, @wjhuang2016, PTAL.

@jebter jebter removed this from the 4.0.0 milestone Jan 7, 2021
@jebter jebter added this to the v4.0.10 milestone Jan 7, 2021
@jebter jebter modified the milestones: v4.0.10, v4.0.11 Jan 18, 2021
@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 Jan 25, 2021
@zz-jason
Copy link
Member

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 25, 2021