sql: update SHOW GRANTS ON TABLE to include grant options #75226
Conversation
|
This change is |
| @@ -157,7 +158,7 @@ func TestPrivilege(t *testing.T) { | |||
| tcNum, descriptor, show, tc.show) | |||
| } | |||
| for i := 0; i < len(show); i++ { | |||
| if show[i].User != tc.show[i].User || show[i].PrivilegeString() != tc.show[i].PrivilegeString() { | |||
| if !reflect.DeepEqual(show[i], tc.show[i]) { | |||
There was a problem hiding this comment.
PrivilegeString() was only being used by this test.
Instead of coming up with a string representation for the struct, I changed the test to use DeepEqual.
ecwall
changed the title
There was a problem hiding this comment.
this is looking good! i had an idea for an additional test to add, which will probably end up requiring a small tweak to the logic that populates the table
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @ecwall, @otan, and @RichardJCai)
pkg/sql/logictest/testdata/logic_test/grant_table, line 2012 at r2 (raw file):
database_name schema_name table_name grantee privilege_type is_grantable c public t admin ALL false c public t millie ALL true
we should introduce a new test case for non-admin user who has ALL privileges, and does not have grant option on ALL, and does have the grant option on something else (like SELECT). this should lead to the privs that do have grant option being broken out into separate rows.
for example, a small snippet from the original issue description:
the output should be:
root@:26257/defaultdb> grant all on t to u3;
GRANT
root@:26257/defaultdb> grant select on t to u3 with grant option;
GRANT
root@:26257/defaultdb> grant insert on t to u3 with grant option;
GRANT
root@:26257/defaultdb> show grants on table t;
database_name | schema_name | table_name | grantee | privilege_type | is_grantable
----------------+-------------+------------+---------+----------------+------------------
defaultdb | public | t | admin | ALL | True
defaultdb | public | t | root | ALL | True
defaultdb | public | t | u3 | ALL | False
defaultdb | public | t | u3 | INSERT | True
defaultdb | public | t | u3 | SELECT | True
There was a problem hiding this comment.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/grant_table, line 2012 at r2 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
we should introduce a new test case for non-admin user who has ALL privileges, and does not have grant option on ALL, and does have the grant option on something else (like SELECT). this should lead to the privs that do have grant option being broken out into separate rows.
for example, a small snippet from the original issue description:
the output should be:root@:26257/defaultdb> grant all on t to u3; GRANT root@:26257/defaultdb> grant select on t to u3 with grant option; GRANT root@:26257/defaultdb> grant insert on t to u3 with grant option; GRANT root@:26257/defaultdb> show grants on table t; database_name | schema_name | table_name | grantee | privilege_type | is_grantable ----------------+-------------+------------+---------+----------------+------------------ defaultdb | public | t | admin | ALL | True defaultdb | public | t | root | ALL | True defaultdb | public | t | u3 | ALL | False defaultdb | public | t | u3 | INSERT | True defaultdb | public | t | u3 | SELECT | True
discussed offline; will add this test in a future PR
pkg/sql/privilege/privilege.go
Outdated
| mask := kind.Mask() | ||
| if kindBits&mask != 0 { | ||
| grantOption := grantOptionBits&mask != 0 | ||
| ret = append(ret, Privilege{ | ||
| Kind: kind, | ||
| GrantOption: grantOption, | ||
| }) |
There was a problem hiding this comment.
uber-nit: you can use fewer lines
if mask := kind.Mask(); kindBits&mask != 0 {
ret = append(ret, Privilege{
Kind: kind,
GrantOption: grantOptionBits&mask != 0,
})There was a problem hiding this comment.
Good call, still getting used to this.
|
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
|
GitHub status checks took too long to complete, so bors is giving up. You can adjust bors configuration to have it wait longer if you like. |
|
Canceled. |
|
Canceled. |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build failed: |
ref #73394 Release note (sql): SHOW GRANTS ON TABLE includes is_grantable column
|
bors r=rafiss,ajwerner |
|
Build failed (retrying...): |
|
Build succeeded: |
ref #73394
Release note: SHOW GRANTS ON TABLE includes is_grantable column