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: the quote function should treat null expr as NULL literal string instead of NULL #11592

Merged
merged 5 commits into from
Aug 5, 2019
Merged

Conversation

gaoxingliang
Copy link
Contributor

What problem does this PR solve?

Fix #11556 QUOTE(null) should return 'NULL' string instead of NULL value

What is changed and how it works?

if the expr is NULL, then the value for quote(expr) will be "NULL" instead of NULL.

Check List

Tests

  • Unit test - added more cases.

Side effects

  • Increased code complexity - maybe difficult to understand. but that's just what mysql does.

@sre-bot
Copy link
Contributor

sre-bot commented Aug 2, 2019

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Aug 2, 2019
@gaoxingliang gaoxingliang changed the title Quoteproblem expression: the quote function should treat null expr as NULL literal string instead of NULL Aug 2, 2019
@codecov
Copy link

codecov bot commented Aug 2, 2019

Codecov Report

Merging #11592 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #11592   +/-   ##
===========================================
  Coverage   81.2404%   81.2404%           
===========================================
  Files           426        426           
  Lines         92033      92033           
===========================================
  Hits          74768      74768           
  Misses        11893      11893           
  Partials       5372       5372

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added component/expression status/LGT1 Indicates that a PR has LGTM 1. status/PTAL labels Aug 5, 2019
@zz-jason zz-jason requested review from qw4990 and SunRunAway August 5, 2019 09:27
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

@XuHuaiyu XuHuaiyu added status/LGT2 Indicates that a PR has LGTM 2. status/can-merge Indicates a PR has been approved by a committer. needs-cherry-pick-2.1 and removed status/LGT1 Indicates that a PR has LGTM 1. status/PTAL status/can-merge Indicates a PR has been approved by a committer. labels Aug 5, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Aug 5, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Aug 5, 2019

@gaoxingliang merge failed.

@sre-bot
Copy link
Contributor

sre-bot commented Aug 5, 2019

/run-all-tests

@zz-jason zz-jason merged commit 5f9fe27 into pingcap:master Aug 5, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Aug 5, 2019

cherry pick to release-2.1 failed

@sre-bot
Copy link
Contributor

sre-bot commented Aug 5, 2019

cherry pick to release-3.0 in PR #11619

sre-bot added a commit that referenced this pull request Aug 5, 2019
@aytrack
Copy link
Contributor

aytrack commented Nov 22, 2019

cherry pick to release-2.1 failed

Could anyone resolve this?

@SunRunAway
Copy link
Contributor

cherry pick to release-2.1 failed

Could anyone resolve this?

Thanks for your remind, see #11592

sre-bot added a commit to SunRunAway/tidb that referenced this pull request Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression contribution This PR is from a community contributor. 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.

QUOTE(null) should return 'NULL' string instead of NULL value
6 participants