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: unicode_ci support when infer collation and charset information #19142

Merged
merged 16 commits into from
Aug 26, 2020
Merged

expression: unicode_ci support when infer collation and charset information #19142

merged 16 commits into from
Aug 26, 2020

Conversation

xiongjiwei
Copy link
Contributor

@xiongjiwei xiongjiwei commented Aug 12, 2020 •
edited
Loading

What problem does this PR solve?

This PR is part of #17596, aim to support unicide_ci when infer collation

What is changed and how it works?

  • unicode_ci has same behiver with general_ci if this two collation not mixed
  • if unicode_ci and general_ci in same expression and same coercibility, infer coercilibity is None
    • if function is able derive to None, infer collation _bin
    • if function cannot derive to None, raise illegal mix collation error

Check List

Tests

  • Unit test
  • Integration test

Release note

  • No release note

@ti-srebot ti-srebot added the contribution This PR is from a community contributor. label Aug 12, 2020
@tiancaiamao tiancaiamao mentioned this pull request Aug 12, 2020
Merged
@codecov
Copy link

codecov bot commented Aug 12, 2020 •
edited
Loading

Codecov Report

Merging #19142 into master will decrease coverage by 0.0730%.
The diff coverage is 97.5609%.

@@               Coverage Diff                @@
##             master     #19142        +/-   ##
================================================
- Coverage   79.1324%   79.0593%   -0.0731%     
================================================
  Files           549        549                
  Lines        149816     148849       -967     
================================================
- Hits         118553     117679       -874     
+ Misses        21736      21665        -71     
+ Partials       9527       9505        -22

@xiongjiwei xiongjiwei marked this pull request as ready for review August 12, 2020 05:42
@xiongjiwei xiongjiwei requested a review from a team as a code owner August 12, 2020 05:42
@xiongjiwei xiongjiwei requested review from wshwsh12 and removed request for a team August 12, 2020 05:42
@xiongjiwei xiongjiwei marked this pull request as draft August 12, 2020 11:53
@xiongjiwei
Copy link
Contributor Author

/run-all-tests

@xiongjiwei xiongjiwei marked this pull request as ready for review August 19, 2020 11:33
wjhuang2016
wjhuang2016 reviewed Aug 20, 2020
View reviewed changes
expression/collation.go Outdated Show resolved Hide resolved
expression/builtin.go Show resolved Hide resolved
expression/builtin.go Outdated Show resolved Hide resolved
expression/collation.go Outdated Show resolved Hide resolved
@xiongjiwei
Copy link
Contributor Author

/run-all-tests

@xiongjiwei
Copy link
Contributor Author

/run-all-tests

wjhuang2016
wjhuang2016 reviewed Aug 21, 2020
View reviewed changes
Copy link
Member

@wjhuang2016 wjhuang2016 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 Aug 21, 2020
qw4990
qw4990 approved these changes Aug 26, 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 removed the status/LGT1 Indicates that a PR has LGTM 1. label Aug 26, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Aug 26, 2020
@qw4990
Copy link
Contributor

qw4990 commented Aug 26, 2020

/merge

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

/run-all-tests

@ti-srebot ti-srebot merged commit d355b82 into pingcap:master Aug 26, 2020
@xiongjiwei xiongjiwei deleted the infer-collation branch August 26, 2020 03:12
@xiongjiwei xiongjiwei mentioned this pull request Aug 26, 2020
Merged
@wjhuang2016 wjhuang2016 added the sig/sql-infra SIG: SQL Infra label Aug 28, 2020
@xiongjiwei
Copy link
Contributor Author

/label needs-cherry-pick-4.0

@xiongjiwei
Copy link
Contributor Author

/run-cherry-pick

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Jan 27, 2021
@xiongjiwei @ti-srebot
cherry pick pingcap#19142 to release-4.0
27456f6 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Jan 27, 2021
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #22581

ti-srebot added a commit that referenced this pull request Jan 28, 2021
@ti-srebot
expression: unicode_ci support when infer collation and charset infor…
187ef72 
…mation (#19142) (#22581)

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Jan 29, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjhuang2016 wjhuang2016 wjhuang2016 left review comments

@qw4990 qw4990 qw4990 approved these changes

@ti-srebot ti-srebot ti-srebot approved these changes

@wshwsh12 wshwsh12 Awaiting requested review from wshwsh12

Assignees
No one assigned
Labels
component/expression contribution This PR is from a community contributor. sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@xiongjiwei @qw4990 @ti-srebot @wjhuang2016