On 3/15/22 16:59, Mark Dilger wrote: >> On Mar 6, 2022, at 3:27 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> >> Mark Dilger <mark.dil...@enterprisedb.com> writes: >>> The existing patch allows grants on unknown gucs, because it can't know >>> what guc an upgrade script will introduce, and the grant statement may need >>> to execute before the guc exists. >> Yeah, that's the problematic case. It might mostly work to assume that >> an unknown GUC has an empty default ACL. This could fail to retain the >> default PUBLIC SET permission if it later turns out the GUC is USERSET > On further reflection, I concluded this isn't needed. No current extension, > whether in-core or third party, expects to be able to create a new GUC and > then grant or revoke permissions on it. They can already specify the guc > context (PGC_USERS, etc). Introducing a feature that depends on the dubious > assumption that unrecognized GUCs will turn out to be USERSET doesn't seem > warranted.
Agreed. > > The patch attributes all grants of setting privileges to the bootstrap > superuser. Only superusers can grant or revoke privileges on settings, and > all settings are implicitly owned by the bootstrap superuser because there is > no explicit owner associated with settings. Consequently, > select_best_grantor(some_superuser, ..., BOOTSTRAP_SUPERUSERID, ...) always > chooses the bootstrap superuser. I don't see a problem with this, but > wouldn't mind a second opinion. Some people might find it surprising when > viewing the pg_setting_acl.setacl field. I think it's OK as long as we document it. An alternative might be to invent a pseudo-superuser called, say, 'postgres_system', but that seems like overkill to solve what is in effect a cosmetic problem. Generally I think this is now in fairly good shape, I've played with it and it seems to do what I expect in every case, and the things I found surprising are gone. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com