privilege/privileges: sort user records in privilege cache #7211
Conversation
| return 0 | ||
| } | ||
|
|
||
| // For other case, the order is nondeterministic. |
There was a problem hiding this comment.
192.168.% is larger than 192.168.199.%.
There was a problem hiding this comment.
Add a check by length.
privilege/privileges/cache.go
Outdated
| case xEnd && !yEnd: | ||
| return 1 | ||
| case xEnd && yEnd: | ||
| // 192.168.199.% smaller than 192.168.% |
There was a problem hiding this comment.
what about '192.%.128.%' and '192.168.1.%' ?
create user 'test'@'192.%.1.%' will be ok in MySQL.
There was a problem hiding this comment.
The result will be nondeterministic
There was a problem hiding this comment.
OK
We may add a FIXME label here.
There was a problem hiding this comment.
I add a TODO in at the beginning of this function.
privilege/privileges/cache.go
Outdated
| @@ -194,6 +194,7 @@ func (s sortedUserRecord) Less(i, j int) bool { | |||
| } | |||
|
|
|||
| // compareHost compares two host string using some special rules, return value 1, 0, -1 means > = <. | |||
| // TODO: Check how MySQL do it exactly, instead of guess its rules. | |||
There was a problem hiding this comment.
create an issue for this TODO, and add the link here
|
@XuHuaiyu I've add a TODO comment at the start of function |
|
PTAL @shenli |
| @@ -47,7 +48,8 @@ func computePrivMask(privs []mysql.PrivilegeType) mysql.PrivilegeType { | |||
| return mask | |||
| } | |||
|
|
|||
| type userRecord struct { | |||
| // UserRecord is used to represent a user record in privilege cache. | |||
| type UserRecord struct { | |||
There was a problem hiding this comment.
maybe no need to make this public if want to test it.
maybe try this trick...https://github.com/lysu/tidb/blob/87ce884b2e0dc679554cb666b05f3347b3c72957/types/export_test.go#L17
There was a problem hiding this comment.
cache_test.go use a different package, so this trick can't work.
privilege/privileges/cache.go
Outdated
| type sortedUserRecord []UserRecord | ||
|
|
||
| func (s sortedUserRecord) Len() int { | ||
| return len([]UserRecord(s)) |
privilege/privileges/cache.go
Outdated
|
|
||
| func (s sortedUserRecord) Less(i, j int) bool { | ||
| x := ([]UserRecord(s))[i] | ||
| y := ([]UserRecord(s))[j] |
There was a problem hiding this comment.
x := s[i]
y := s[j]
works too~
privilege/privileges/cache.go
Outdated
|
|
||
| func (s sortedUserRecord) Swap(i, j int) { | ||
| s1 := []UserRecord(s) | ||
| s1[i], s1[j] = s1[j], s1[i] |
| } | ||
|
|
||
| // compareHost compares two host string using some special rules, return value 1, 0, -1 means > = <. | ||
| // TODO: Check how MySQL do it exactly, instead of guess its rules. |
There was a problem hiding this comment.
There was a problem hiding this comment.
It may be overkill, if some user really meet the complex cases and complain, I'll consider.
If you are interested, you can self-assign this one #7229
|
/run-all-tests |
tiancaiamao
added
status/LGT2
What have you changed? (mandatory)
Sort the user records according to MySQL rules.
What is the type of the changes? (mandatory)
How has this PR been tested? (mandatory)
unit test
Does this PR affect documentation (docs/docs-cn) update? (mandatory)
no
Does this PR affect tidb-ansible update? (mandatory)
no
Does this PR need to be added to the release notes? (mandatory)
no
Refer to a related PR or issue link (optional)
fix #7182