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
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()? |