Re: Granting SET and ALTER SYSTE privileges for GUCs

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Granting SET and ALTER SYSTE privileges for GUCs
Date: 2021-11-16 21:02:44
Message-ID: 7DFFECF0-373B-40B0-BCA0-A5ADD9266FFA@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Nov 16, 2021, at 12:38 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
> Your original and fairly simple set of patches used hardcoded role names
> and sets of GUCs they could update via ALTER SYSTEM. I suggested to you
> privately that a more flexible approach would be to drive this from a
> catalog table. I had in mind a table of more or less <roleid, guc_name>.

Right. I prefer a table of <guc_name, acl> matching the structure of most of the rest of the grantable objects, so that entries from pg_depend or pg_shdepend can track the dependencies in the usual way and prevent dangling references when DROP ROLE is executed. Is there a reason to prefer an ad hoc tables structure?

> You could prepopulate it with the roles / GUCs from your original patch
> set. I don't think it needs to be initially empty. But DBAs would be
> able to modify and extend the settings. I agree with Tom that we
> shouldn't try to cover all GUCs in the table - any GUC without an entry
> can only be updated by a superuser.

The patch already posted on this thread already works that way. Robert and Tom seemed to infer that all gucs need to be present in the catalog, but in fact, not entering them in the catalog simply means that they aren't grantable. I think the confusion arose from the fact that I created a mechanism for extension authors to enter additional gucs into the catalog, which gave the false impression that such entries were required. They are not. I didn't bother explaining that before, because Robert and Tom both seem to expect all gucs to be grantable, an expectation you do not appear to share.

> To support pg_dump and pg_upgrade, it might be better to have an
> enabled/disabled flag rather than to delete rows.

I'm not really sure what this means. Are you suggesting that the <guc_name, acl> (or in your formulation <roleid, guc_name>) should have a "bool enabled" field, and when the guc gets dropped, the "enabled" field gets set to false?


Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2021-11-16 21:30:27 Re: RecoveryInProgress() has critical side effects
Previous Message Andres Freund 2021-11-16 20:42:23 Re: RecoveryInProgress() has critical side effects