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: return binary charset and collation if there is no string-type argument in DeriveCollationFromExprs #15250

Merged
merged 7 commits into from
Mar 10, 2020

Conversation

qw4990
Copy link
Contributor

@qw4990 qw4990 commented Mar 10, 2020

What problem does this PR solve?

Update the function DeriveCollationFromExprs to let it return binary charset and collation if there is no string-type argument.

Check List

Tests

  • Unit test

@qw4990 qw4990 requested review from bb7133 and wjhuang2016 March 10, 2020 03:43
@qw4990 qw4990 requested a review from a team as a code owner March 10, 2020 03:43
@ghost ghost requested review from wshwsh12 and XuHuaiyu and removed request for a team March 10, 2020 03:43
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, Please fix the ci

@codecov
Copy link

codecov bot commented Mar 10, 2020

Codecov Report

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

@@             Coverage Diff             @@
##             master     #15250   +/-   ##
===========================================
  Coverage   80.5354%   80.5354%           
===========================================
  Files           503        503           
  Lines        134398     134398           
===========================================
  Hits         108238     108238           
  Misses        17685      17685           
  Partials       8475       8475

@qw4990
Copy link
Contributor Author

qw4990 commented Mar 10, 2020

Fixed, PTAL @bb7133 @wjhuang2016

@wjhuang2016
Copy link
Member

/run-unit-test

@wjhuang2016 wjhuang2016 added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 10, 2020
@qw4990 qw4990 requested a review from winoros March 10, 2020 07:49
Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@wjhuang2016 wjhuang2016 added status/LGT2 Indicates that a PR has LGTM 2. status/can-merge Indicates a PR has been approved by a committer. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 10, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 10, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Mar 10, 2020

@qw4990 merge failed.

@qw4990
Copy link
Contributor Author

qw4990 commented Mar 10, 2020

/run-unit-test

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.

5 participants