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

executor: fix wrong behavior of set charset statement (#16984) #17289

Merged
merged 13 commits into from
Jul 31, 2020
Merged

executor: fix wrong behavior of set charset statement (#16984) #17289

merged 13 commits into from
Jul 31, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented May 19, 2020

cherry-pick #16984 to release-4.0

What problem does this PR solve?

Problem Summary:

Previously, we take set charset as set names, and set charsetCollation too. However, it's wrong, as it's stated in here

What is changed and how it works?

Distinguish set names and set charset statements and handle them differently.
Related Parser PR.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • UnitTest

Side effects

Release note

  • Fix wrong behavior of set charset statement

@sre-bot sre-bot requested a review from a team as a code owner May 19, 2020 10:10
@sre-bot
Copy link
Contributor Author

sre-bot commented May 19, 2020

/run-all-tests

@sre-bot sre-bot mentioned this pull request May 19, 2020
Merged
@ghost ghost requested a review from wshwsh12 May 19, 2020 10:10
@sre-bot sre-bot added sig/execution SIG execution type/4.0-cherry-pick type/bugfix This PR fixes a bug. labels May 19, 2020
@ghost ghost removed their request for review May 19, 2020 10:10
@sre-bot sre-bot added this to the v4.0.0-ga milestone May 19, 2020
AilinKid
AilinKid reviewed May 20, 2020
View reviewed changes
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

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

pls fix the conflicts

tangenta
tangenta reviewed May 25, 2020
View reviewed changes
Copy link
Contributor

@tangenta tangenta left a comment

Choose a reason for hiding this comment

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

LGTM

@tangenta
Copy link
Contributor

Please run go mod tidy to fix the error in idc-jenkins-ci-tidb/check_dev.

@wshwsh12 wshwsh12 removed their request for review May 28, 2020 04:30
@SunRunAway SunRunAway modified the milestones: v4.0.0-ga, v4.0.1 May 28, 2020
@bb7133 bb7133 modified the milestones: v4.0.1, v4.0.2 Jun 6, 2020
@SunRunAway SunRunAway removed their request for review June 7, 2020 09:42
@zz-jason zz-jason removed this from the v4.0.2 milestone Jun 19, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@sre-bot merge failed.

@wjhuang2016
fix
fd9ab6c 
Signed-off-by: wjhuang2016 <[email protected]>
@wjhuang2016
fix
177f0a5 
Signed-off-by: wjhuang2016 <[email protected]>
@bb7133
Copy link
Member

bb7133 commented Jul 9, 2020

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@bb7133
Copy link
Member

bb7133 commented Jul 13, 2020

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@sre-bot merge failed.

@bb7133
Copy link
Member

bb7133 commented Jul 13, 2020

/run-all-tests

@winoros winoros modified the milestones: v4.0.3, v4.0.4 Jul 15, 2020
@imtbkcat imtbkcat modified the milestones: v4.0.4, v4.0.5 Jul 28, 2020
@qw4990 qw4990 mentioned this pull request Jul 31, 2020
Merged
@ti-srebot
Copy link
Contributor

@tangenta,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).

@ti-srebot
Copy link
Contributor

@AilinKid,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).

qw4990
qw4990 approved these changes Jul 31, 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 status/LGT1 Indicates that a PR has LGTM 1. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Jul 31, 2020
@qw4990
Copy link
Contributor

qw4990 commented Jul 31, 2020

/run-all-tests

@bb7133 bb7133 merged commit 7b0b0c0 into pingcap:release-4.0 Jul 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djshow832 djshow832 djshow832 left review comments

@qw4990 qw4990 qw4990 approved these changes

@AilinKid AilinKid AilinKid approved these changes

@tangenta tangenta tangenta approved these changes

@bb7133 bb7133 Awaiting requested review from bb7133

@tiancaiamao tiancaiamao Awaiting requested review from tiancaiamao

Assignees

@wjhuang2016 wjhuang2016

Labels
sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. 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.