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: fix wrong result when select with collation (#18665) #18735

Merged
merged 2 commits into from
Jul 27, 2020
Merged

expression: fix wrong result when select with collation (#18665) #18735

merged 2 commits into from
Jul 27, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #18665 to release-4.0

What problem does this PR solve?

Issue Number: close #18662

What changed

add newBaseBuiltinFuncWithFieldType, this function create a new basebuiltinFunc without check.

How it works

remove unnecessary mix collation check, use collation decode from pb directly, do not re-calculate it. pb does not contains coercibility, re-calculate collation will get wrong result.

Tests

  • Unit test
  • Integration test

Release note

  • fix incorrect collator when getSignatureByPB and remove unnecessary recover

@xiongjiwei @ti-srebot
cherry pick pingcap#18665 to release-4.0
2a6ae38 
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 Jul 23, 2020
Merged
@ti-srebot ti-srebot added component/expression contribution This PR is from a community contributor. type/4.0-cherry-pick type/bugfix This PR fixes a bug. labels Jul 23, 2020
@ti-srebot ti-srebot added this to the v4.0.4 milestone Jul 23, 2020
@ti-srebot ti-srebot self-assigned this Jul 23, 2020
@bb7133
Copy link
Member

bb7133 commented Jul 27, 2020

LGTM

@ti-srebot
Copy link
Contributor Author

@bb7133,Thanks for your review. However, LGTM is restricted to Reviewers or higher roles.See the corresponding SIG page for more information. Related SIGs: execution(slack).

@bb7133
Copy link
Member

bb7133 commented Jul 27, 2020

/run-all-tests

qw4990
qw4990 approved these changes Jul 27, 2020
View reviewed changes
Copy link
Contributor

@qw4990 qw4990 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 Jul 27, 2020
@bb7133 bb7133 merged commit 75171f0 into pingcap:release-4.0 Jul 27, 2020
@jackysp jackysp removed the contribution This PR is from a community contributor. label Jul 27, 2020
@imtbkcat imtbkcat modified the milestones: v4.0.4, v4.0.5 Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qw4990 qw4990 qw4990 approved these changes

@bb7133 bb7133 Awaiting requested review from bb7133

@wjhuang2016 wjhuang2016 Awaiting requested review from wjhuang2016

Assignees

@ti-srebot ti-srebot

Labels
component/expression status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug. type/4.0-cherry-pick
Projects
None yet
Milestone
v4.0.5
Development

Successfully merging this pull request may close these issues.
6 participants
@ti-srebot @bb7133 @qw4990 @jackysp @imtbkcat @xiongjiwei