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

*: support "show create user" #9240

Merged
merged 19 commits into from Feb 21, 2019

Conversation

@lnhote
Copy link
Contributor

commented Feb 1, 2019

What problem does this PR solve?

Support SHOW CREATE USER as in MySQL 5.7.
fix #7733

What is changed and how it works?

  1. add GetEncodedPassword function to Manager interface.
  2. add support for ShowCreateTable in buildShowSchema function.
  3. add support for ShowCreateUser in ShowExec.fetchAll function

Check List

Tests

  • Unit test

Code changes

  • exported function/method change
  • Has exported variable/fields change
  • Has interface methods change

lnhote and others added some commits Jan 7, 2019

Merge branch 'master' into support_show_create_user
# Conflicts:
#	go.mod
#	go.sum
#	planner/core/planbuilder.go
@shenli

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

@lnhote Thanks!
Please fix the CI.

@shenli shenli added the contribution label Feb 5, 2019

@morgo

This comment has been minimized.

Copy link
Contributor

commented Feb 8, 2019

@lnhote Thank you for working on this! It looks like it just needs go fmt to be run:

go fmt privilege/privileges/privileges.go
@lnhote

This comment has been minimized.

Copy link
Contributor Author

commented Feb 10, 2019

Thanks for the help. Please take another look.

@codecov-io

This comment has been minimized.

Copy link

commented Feb 10, 2019

Codecov Report

Merging #9240 into master will increase coverage by 0.13%.
The diff coverage is 51.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9240      +/-   ##
==========================================
+ Coverage   67.02%   67.15%   +0.13%     
==========================================
  Files         373      373              
  Lines       78150    78191      +41     
==========================================
+ Hits        52378    52512     +134     
+ Misses      21094    20977     -117     
- Partials     4678     4702      +24
Impacted Files Coverage Δ
util/testkit/testkit.go 14.16% <0%> (-1.02%) ⬇️
planner/core/planbuilder.go 49.35% <33.33%> (-0.04%) ⬇️
privilege/privileges/privileges.go 57.31% <45.45%> (-1.84%) ⬇️
executor/show.go 45.1% <78.94%> (+1.22%) ⬆️
ddl/delete_range.go 75.13% <0%> (-2.65%) ⬇️
infoschema/infoschema.go 76.31% <0%> (-1.32%) ⬇️
store/tikv/lock_resolver.go 41.7% <0%> (-0.95%) ⬇️
executor/distsql.go 72.24% <0%> (+0.45%) ⬆️
executor/join.go 79.42% <0%> (+0.52%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a24081...81b503e. Read the comment docs.

@morgo

This comment has been minimized.

Copy link
Contributor

commented Feb 10, 2019

@lnhote can you please add tests for:

  • a user that doesn't exist
  • the user exists but the host portion doesn't match

I am getting the following behavior on a fresh install. Both are different results from MySQL:

mysql> SET PASSWORD='abcd';
2019/02/10 02:35:19.953 session.go:1639: [info] [CRUCIAL OPERATION] con:1 schema_ver:15 set password for user <nil> (by root@127.0.0.1).
Query OK, 0 rows affected (0.00 sec)

mysql> SHOW CREATE USER 'root'@'%'; <-- correct
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| CREATE USER for root@%                                                                                                                                            |
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| CREATE USER 'root'@'%' IDENTIFIED WITH 'mysql_native_password' AS '*A154C52565E9E7F94BFC08A1FE702624ED8EFFDA' REQUIRE NONE PASSWORD EXPIRE DEFAULT ACCOUNT UNLOCK |
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

mysql> SHOW CREATE USER 'root'@'nowhere'; <--- incorrect
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| CREATE USER for root@nowhere                                                                                                                                            |
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| CREATE USER 'root'@'nowhere' IDENTIFIED WITH 'mysql_native_password' AS '*A154C52565E9E7F94BFC08A1FE702624ED8EFFDA' REQUIRE NONE PASSWORD EXPIRE DEFAULT ACCOUNT UNLOCK |
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

mysql> show create user 'aaa'@'aaa'; <-- incorrect
2019/02/10 02:35:39.737 privileges.go:65: [error] Get user privilege record fail: user aaa, host aaa
+---------------------------------------------------------------------------------------------------------------------------+
| CREATE USER for aaa@aaa                                                                                                   |
+---------------------------------------------------------------------------------------------------------------------------+
| CREATE USER 'aaa'@'aaa' IDENTIFIED WITH 'mysql_native_password' AS '' REQUIRE NONE PASSWORD EXPIRE DEFAULT ACCOUNT UNLOCK |
+---------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)
@morgo

This comment has been minimized.

Copy link
Contributor

commented Feb 10, 2019

Expected behavior:

mysql [localhost:5724] {msandbox} ((none)) > show grants for 'root'@'nowhere';
ERROR 1141 (42000): There is no such grant defined for user 'root' on host 'nowhere'
mysql [localhost:5724] {msandbox} ((none)) > show grants for 'aaa'@'aaa';
ERROR 1141 (42000): There is no such grant defined for user 'aaa' on host 'aaa'
Show resolved Hide resolved executor/builder.go Outdated

@zz-jason zz-jason changed the title Support show create user *: support "show create user" Feb 15, 2019

lnhote added some commits Feb 15, 2019

Merge branch 'master' into support_show_create_user
# Conflicts:
#	privilege/privileges/privileges.go
Show resolved Hide resolved executor/show.go
@morgo

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

@lnhote I left two review comments. It looks good otherwise. Thanks!

@morgo

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

LGTM

@morgo morgo added the status/LGT1 label Feb 15, 2019

@morgo

This comment has been minimized.

Copy link
Contributor

commented Feb 19, 2019

@lnhote please resolve conflicts.

@lnhote

This comment has been minimized.

Copy link
Contributor Author

commented Feb 20, 2019

@morgo PTAL.

@morgo

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2019

LGTM
@jackysp PTAL

@@ -16,13 +16,13 @@ package executor_test
import (
"context"
"fmt"

This comment has been minimized.

Copy link
@jackysp

jackysp Feb 20, 2019

Member

Please do not remove this line, keep the sys libs separate with the 3rd party libs.

This comment has been minimized.

Copy link
@lnhote

lnhote Feb 20, 2019

Author Contributor

Adding the blank line will cause check_dev to fail.

This comment has been minimized.

Copy link
@jackysp

jackysp Feb 21, 2019

Member

Why check_dev could pass in master branch? I think you've added a blank line with some white spaces, try to keep it as it used to be.

This comment has been minimized.

Copy link
@lnhote

lnhote Feb 21, 2019

Author Contributor

I fixed it.

lnhote added some commits Feb 20, 2019

@jackysp

This comment has been minimized.

Copy link
Member

commented Feb 21, 2019

/run-all-tests

@jackysp

This comment has been minimized.

Copy link
Member

commented Feb 21, 2019

/run-integration-common-test

@jackysp

This comment has been minimized.

Copy link
Member

commented Feb 21, 2019

/run-integration-ddl-test

@jackysp
Copy link
Member

left a comment

LGTM

@jackysp jackysp merged commit aabd330 into pingcap:master Feb 21, 2019

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

yu34po added a commit to yu34po/tidb that referenced this pull request Feb 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.