Re: Allow ALTER SYSTEM SET on unrecognized custom GUCs

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, gavinpanella(at)gmail(dot)com
Subject: Re: Allow ALTER SYSTEM SET on unrecognized custom GUCs
Date: 2023-10-23 14:19:54
Message-ID: 5b723f9a-43f8-bad0-3eeb-4038d2bbeda0@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 2023-10-16 Mo 20:19, Tom Lane wrote:
> Currently we have this odd behavior (for a superuser):
>
> regression=# ALTER SYSTEM SET foo.bar TO 'baz';
> ERROR: unrecognized configuration parameter "foo.bar"
> regression=# SET foo.bar TO 'baz';
> SET
> regression=# ALTER SYSTEM SET foo.bar TO 'baz';
> ALTER SYSTEM
>
> That is, you can't ALTER SYSTEM SET a random custom GUC unless there
> is already a placeholder GUC for it, because the find_option call in
> AlterSystemSetConfigFile fails. This is surely pretty inconsistent.
> Either the first ALTER SYSTEM SET ought to succeed, or the second one
> ought to fail too, because we don't have any more knowledge about the
> custom GUC than we did before.
>
> In the original discussion about this [1], I initially leaned towards
> "they should both fail", but I reconsidered: there doesn't seem to be
> any harm in allowing ALTER SYSTEM SET to succeed for any custom GUC
> name, as long as you're superuser.
>
> Hence, attached is a patch for that. Much of it is refactoring to
> avoid duplicating the code that checks for a reserved GUC name, which
> I think should still be done here --- otherwise, we're losing a lot of
> the typo detection that that check was intended to provide. (That is,
> if you have loaded an extension that defines "foo" as a prefix, we
> should honor the extension's opinion about whether "foo.bar" is
> valid.) I also fixed the code for GRANT ON PARAMETER so that it
> follows the same rules and throws the same errors for invalid cases.
>
> There's a chunk of AlterSystemSetConfigFile that now needs indenting
> one more tab stop, but I didn't do that yet for ease of review.
>
> Thoughts?
>
>

Haven't read the patch but in principle I agree.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message David G. Johnston 2023-10-23 14:26:17 Re: Fix output of zero privileges in psql
Previous Message Robert Haas 2023-10-23 14:14:27 Re: Is this a problem in GenericXLogFinish()?