sql: allow non-ALL privileges to be granted WITH GRANT OPTION #75560
Conversation
|
This change is |
ecwall
changed the title
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @ecwall and @rafiss)
pkg/sql/catalog/descpb/privilege.go, line 246 at r1 (raw file):
// Prefer WITH GRANT OPTION syntax. userPriv.WithGrantOption |= bits } else if privilege.ALL.IsSetIn(userPriv.Privileges) || privilege.GRANT.IsSetIn(userPriv.Privileges) {
hm, i still find the old way of having a separate GrantPrivilegeToGrantOption function more clear. it made it obvious which parts were all "temporary" logic, so it was more clear which code we should delete in v22.2
also, now the code for generating the deprecation warning is separated from the code that that is doing the deprecated behavior
we should at least comment here saying which parts are temporary logic that should be deleted.
pkg/sql/catalog/descpb/privilege.go, line 281 at r1 (raw file):
userPriv.WithGrantOption = 0 if !grantOptionFor { userPriv.Privileges = 0
shouldn't this still be p.RemoveUser()?
pkg/sql/logictest/testdata/logic_test/alter_default_privileges_with_grant_option, line 669 at r1 (raw file):
test public t17 admin ALL true test public t17 root ALL true test public t17 testuser ALL false
do you know why these test results changed?
pkg/sql/logictest/testdata/logic_test/pg_catalog_pg_default_acl, line 50 at r1 (raw file):
---- oid defaclrole defaclnamespace defaclobjtype defaclacl 625980300 1791217281 0 r {foo=C*a*d*r*w*/}
do you know why these test results changed? it looks like some substantial changes to default privileges - we want to make sure the temporary implicit logic doeesn't affect default privileges
Release note: None
There was a problem hiding this comment.
lgtm! thanks for figuring out all the edge cases
Reviewed 1 of 9 files at r1, 9 of 9 files at r4, all commit messages.
Reviewable status:
|
bors r=rafiss |
75560: sql: allow non-ALL privileges to be granted WITH GRANT OPTION r=rafiss a=ecwall 75762: sql,roachpb: add plan hash correct value to persisted stats r=maryliag a=maryliag Previously, we didn't have the plan hash/gist values, so a dummy value was being used instead. Now that we have the value, this commit uses those values to be corrected stored. The Plan Hash is saved on its own column and is part of a statement key. A plan gist is a string saved in the metadata and can later on converted back into a logical plan. Partially addresses #72129 Release note (sql change): Saving plan hash/gist to the Statements persisted stats. 75880: server: better error message for tsdump initialization r=liamgillies a=liamgillies Beforehand, the error message for tsdump initialization was unclear and didn't provide enough information to support engineers on how to fix it. To address this, the error message has been revamped with instructions and commands to get tsdump working. Release note (cli change): Added instructions to an error message when initializing tsdump. 76112: sql: Replace `WITH BUCKET_COUNT` with new `bucket_count` storage param. r=chengxiong-ruan a=chengxiong-ruan a follow up of pr #76068 Release note (sql change): We have add support for the new `bucket_count` storage param syntax. We prefer it over the old `WITH BUCKET_COUNT=xxx` syntax. With this change, crdb outputs the new syntax for `SHOW CREATE`. Though for the AST tree formatting, we still respect the old syntax if user used it. 76129: geo: add pgerror codes for errors.Newf r=rafiss a=otan Release note (sql change): Added PG error codes to the majority of spatial related functions. 76242: sql/logictest: fix flakey new_schema_changer r=ajwerner a=ajwerner IDs are not deterministic due to the non-transactional nature of the descriptor ID allocator sequence. The ID wasn't really adding value to the test anyway but rather the name was interesting. Fixes #76237 Release note: None Co-authored-by: Evan Wall <[email protected]> Co-authored-by: Marylia Gutierrez <[email protected]> Co-authored-by: Liam Gillies <[email protected]> Co-authored-by: Chengxiong Ruan <[email protected]> Co-authored-by: Oliver Tan <[email protected]> Co-authored-by: Andrew Werner <[email protected]>
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
No description provided.