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: Support NO_AUTO_CREATE_USER sql mode #8160

Merged
merged 10 commits into from
Nov 12, 2018
Merged

executor: Support NO_AUTO_CREATE_USER sql mode #8160

merged 10 commits into from
Nov 12, 2018

Conversation

morgo
Copy link
Contributor

@morgo morgo commented Nov 2, 2018 •
edited
Loading

What problem does this PR solve?

Provides NO_AUTO_CREATE_USER sql mode. Fixes #8128

What is changed and how it works?

It depends on parser changes:
pingcap/parser#10

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Code changes

  • Has exported function/method change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Increased code complexity
  • Breaking backward compatibility (* when enabled by default: a followup patch)

Related changes

  • Need to be included in the release note

@morgo morgo changed the title Support NO_AUTO_CREATE_USER sql mode executor: Support NO_AUTO_CREATE_USER sql mode Nov 2, 2018
@morgo
Copy link
Contributor Author

morgo commented Nov 2, 2018

The build error is related to the parser changes not merged. It should otherwise work.

@morgo morgo requested a review from tiancaiamao November 2, 2018 20:03
@morgo
Copy link
Contributor Author

morgo commented Nov 2, 2018

PTAL @tiancaiamao

XuHuaiyu
XuHuaiyu reviewed Nov 5, 2018
View reviewed changes
executor/grant.go Outdated
@@ -38,6 +39,14 @@ var (
_ Executor = (*GrantExec)(nil)
)

const (
codePasswordNoMatch = 1133
Copy link
Contributor

Choose a reason for hiding this comment

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

It better to put line42~line48 to executor/errors.go

executor/grant_test.go Show resolved Hide resolved
@morgo
Copy link
Contributor Author

morgo commented Nov 5, 2018

Thx for the feedback - I've made the changes, but the CI will still show as 'X' because of the parser dependency.

@XuHuaiyu
Copy link
Contributor

XuHuaiyu commented Nov 6, 2018

LGTM
PTAL @tiancaiamao

@XuHuaiyu XuHuaiyu added status/LGT1 Indicates that a PR has LGTM 1. component/privilege type/enhancement The issue or PR belongs to an enhancement. labels Nov 6, 2018
@tiancaiamao
Copy link
Contributor

Try GO111MODULE=on go get -u github.com/pingcap/parser and make dev again
@morgo

@tiancaiamao
Copy link
Contributor

Well, GO111MODULE=on go get -u github.com/pingcap/parser can't work temporarily.
Please wait for pingcap/parser#23

@morgo
Copy link
Contributor Author

morgo commented Nov 10, 2018

/run-all-tests

@shenli
Copy link
Member

shenli commented Nov 10, 2018

/run-integration-ddl-test

@morgo morgo mentioned this pull request Nov 11, 2018
Merged
@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao merged commit ce6a715 into pingcap:master Nov 12, 2018
@morgo morgo deleted the noautocreateuser branch November 12, 2018 07:16
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XuHuaiyu XuHuaiyu XuHuaiyu left review comments

@tiancaiamao tiancaiamao tiancaiamao approved these changes

Assignees
No one assigned
Labels
component/privilege contribution This PR is from a community contributor. status/LGT1 Indicates that a PR has LGTM 1. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support NO_AUTO_CREATE_USER sql_mode
5 participants
@morgo @XuHuaiyu @tiancaiamao @shenli @sre-bot