public inbox for [email protected]
help / color / mirror / Atom feedFrom: Mark Dilger <[email protected]>
To: Tom Lane <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: Joshua Brindle <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Jeff Davis <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Joe Conway <[email protected]>
Subject: Re: Granting SET and ALTER SYSTE privileges for GUCs
Date: Wed, 16 Mar 2022 11:59:06 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<CA+TgmobUt_tSF-9tz-eRvWzT1pbe_ssvxfnPniGGEgLzems40Q@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAGB+Vh4_UJsYHCWborm-y4CFt8gVvY_KbcNrpWcpXAk-GcwOtA@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<43857434-3f9b-3! [email protected]>
<[email protected]>
> On Mar 16, 2022, at 11:47 AM, Tom Lane <[email protected]> wrote:
>
> Stepping back a bit ... do we really want to institutionalize the
> term "setting" for GUC variables? I realize that the view pg_settings
> exists, but the documentation generally prefers the term "configuration
> parameters". Where config.sgml uses "setting" as a noun, it's usually
> talking about a specific concrete value for a parameter, and you can
> argue that the view's name comports with that meaning. But you can't
> GRANT a parameter's current value.
>
> I don't have a better name to offer offhand --- I surely am not proposing
> that we change the syntax to be "GRANT ... ON CONFIGURATION PARAMETER x",
> because that's way too wordy. But now is the time to bikeshed if we're
> gonna bikeshed, or else admit that we're not interested in precise
> vocabulary.
Informally, we often use "GUC" on this list, but that isn't used formally, leaving "configuration parameter" and "setting" as the two obvious choices. I preferred "configuration parameter" originally and was argued out of it. My take on "setting" was also that it more naturally refers to the choice of setting, not the thing being set, such that "work_mem = 8192" means the configuration parameter "work_mem" has the setting "8192". I'm happy to change the patch along these lines, but I vaguely recall it being Robert who liked the "setting" language. Robert? (Sorry if I misremember that...)
> I'm also fairly allergic to the way that this patch has decided to assign
> multi-word names to privilege types (ie SET VALUE, ALTER SYSTEM). There
> is no existing precedent for that, and I think it's going to break
> client-side code that we don't need to break. It's not coincidental that
> this forces weird changes in rules about whitespace in the has_privilege
> functions, for example; and if you think that isn't going to cause
> problems I think you are wrong. Perhaps we could just use "SET" and
> "ALTER", or "SET" and "SYSTEM"?
I chose those particular multi-word names to avoid needing to promote any keywords. That's been a while ago, and I don't recall exactly where the shift-reduce or reduce-reduce errors were coming from. I'll play with it to see what I can find.
> I also agree with the upthread criticism that this is abusing the
> ObjectPostAlterHook API unreasonably much. In particular, it's
> trying to use pg_setting_acl OIDs as identities for GUCs, which
> they are not, first because they're unstable and second because
> most GUCs won't even have an entry there. I therefore judge the
> hook calls added to ExecSetVariableStmt and AlterSystemSetConfigFile
> to be 100% useless, in fact probably counterproductive because they
> introduce a boatload of worries about whether the right things happen
> if the hook errors out or does something guc.c isn't expecting.
I think Joshua was planning to use these hooks for security purposes. The hooks are supposed to check whether the Oid is valid, and if not, still be able to make choices based on the other information. Joshua, any comment on this?
> I suggest that what might be saner is to consider that the "objects"
> that the hook calls are concerned with are the pg_setting_acl entries,
> not the underlying GUCs, and thus that the hooks need be invoked only
> when creating, destroying or altering those entries.
That's certainly a thing we could do, but I got the impression that Joshua wanted to hook into SET, RESET, and ALTER SYSTEM SET, not merely into GRANT and REVOKE.
> If we do have
> a need for a hook editorializing on GUC value settings, that would
> have to be an independent API --- but I have heard no calls for
> the ability to have such a hook, and I don't think that this patch
> is the place to introduce one.
Well, the call for it was in this thread, but I'm ok with yanking out that part of the patch if it seems half-baked.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
view thread (24+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Granting SET and ALTER SYSTE privileges for GUCs
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox