Re: Granting SET and ALTER SYSTE privileges for GUCs

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Joshua Brindle <joshua(dot)brindle(at)crunchydata(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Joe Conway <joe(at)crunchydata(dot)com>
Subject: Re: Granting SET and ALTER SYSTE privileges for GUCs
Date: 2022-04-05 00:01:00
Message-ID: 2701A2FB-F02B-4C30-B33E-9CB215C478E6@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Apr 4, 2022, at 2:26 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Thanks. As I'm working through this, I'm kind of inclined to drop
> the has_parameter_privilege() variants that take an OID as object
> identifier. This gets back to the fact that I don't think
> pg_parameter_acl OIDs have any outside use; we wouldn't even have
> them except that we need a way to track their role dependencies
> in pg_shdepend. AFAICS users will only ever be interested in
> looking up a GUC by name. Any objections?

None.

> Another thought here is that I see you're expending some code
> to store the canonical name of a GUC in pg_parameter_acl, but
> I think that's probably going too far. In particular, for the
> legacy mixed-case names like "DateStyle", what ends up in the
> table is the mixed-case name, which seems problematic. It would
> definitely be problematic if an extension used such a name,
> because we might or might not be aware of the idiosyncratic
> casing at the time a GRANT is issued. I'm thinking that we
> really want to avoid looking up custom GUCs at all during GRANT,
> because that can't do anything except create hazards.

Yikes. It took a few tries to see what you mean. Yes, if the GRANT happens before the LOAD, that can have bad consequences. So I agree something should be changed.

> So I think that instead of what you've got here, we should
> (1) apply the map_old_guc_names[] mapping, which is constant
> (for any one PG release anyway)
> (2) smash to lower case
> (3) verify validity per valid_variable_name.
>
> This also simplifies life on the lookup side, where it's sufficient
> to do steps (1) and (2) before performing a catalog search.
>
> Thoughts?

That sounds right. Do you already have something like that coded, or would you like me to post a patch?


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 David Rowley 2022-04-05 00:04:18 Re: Window Function "Run Conditions"
Previous Message Michael Paquier 2022-04-04 23:46:06 Re: Run pg_amcheck in 002_pg_upgrade.pl and 027_stream_regress.pl?