Re: Allow ALTER SYSTEM SET on unrecognized custom GUCs

From: Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
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-18 04:55:48
Message-ID: 41738c67-46a3-427f-9d6e-3a2d6778d673@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 17/10/2023 07: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?

I have reviewed this patch. It looks good in general. Now, we can change
the placeholder value with the SET command and have one more tool (which
may be unusual) to pass some data through the session.
Keeping away from the reason why DBMS allows such behaviour, I have one
question:
"SET foo.bar TO 'smth'" can immediately alter the placeholder's value.
But what is the reason that "ALTER SYSTEM SET foo.bar TO 'smth'" doesn't
do the same?

--
regards,
Andrey Lepikhov
Postgres Professional

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-10-18 05:15:11 Re: Allow ALTER SYSTEM SET on unrecognized custom GUCs
Previous Message Amit Kapila 2023-10-18 04:50:09 Re: Synchronizing slots from primary to standby