Horton

Thread

Commits

  1. Allow ALTER SYSTEM to set unrecognized custom GUCs.

  1. "20.16. Customized Options" – cannot be set by `ALTER SYSTEM`

    The Post Office <noreply@postgresql.org> — 2023-10-16T13:34:57Z

    The following documentation comment has been logged on the website:

Page: https://www.postgresql.org/docs/16/runtime-config-custom.html
Description:

As far as I can tell, the following statement:

> PostgreSQL will accept a setting for any two-part parameter name

does not hold when creating a *new* setting with `ALTER SYSTEM`, e.g.

  ALTER SYSTEM SET foo.bar TO 'baz';

will elicit an error.

However, if `foo.bar` is defined in `postgresql.conf` or
`postgresql.auto.conf` – put there by hand – then it can be altered, i.e.
the `ALTER SYSTEM` command above will succeed.

I don't know if this is something that should be mentioned in the
documentation or if it's an inconsistency in the implementation.

  2. Re: "20.16. Customized Options" – cannot be set by `ALTER SYSTEM`

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-16T16:29:09Z

    PG Doc comments form <noreply@postgresql.org> writes:
> As far as I can tell, the following statement:

>> PostgreSQL will accept a setting for any two-part parameter name

> does not hold when creating a *new* setting with `ALTER SYSTEM`, e.g.

>   ALTER SYSTEM SET foo.bar TO 'baz';

> will elicit an error.

ALTER SYSTEM requires the variable to be known, so that (a) it can
figure out whether you have permissions to set it at system level,
and (b) it can check the validity of the value.  It does not seem
like a good idea to allow unchecked values to be pushed into the
config file, because a mistake would prevent future server restarts
from succeeding.

If you just do "SET foo.bar = whatever", the action is transiently
allowed because nothing very interesting will happen until/unless some
extension loads a definition of the variable into your session, and we
can figure out at that point whether your setting should be accepted.
It would be too much of a mess to make that work for ALTER SYSTEM
though, not least because the config files don't record who set the
variable.

I do see an issue here:

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

and now we have

$ cat $PGDATA/postgresql.auto.conf
# Do not edit this file manually!
# It will be overwritten by the ALTER SYSTEM command.
foo.bar = 'baz'

So that feels like a bug: we should not allow ALTER SYSTEM to execute
against a placeholder GUC definition, because the placeholder can't
tell us whether the value is valid.  I wonder though if forbidding
this would break any legitimate usage patterns.

			regards, tom lane

  3. Re: "20.16. Customized Options" – cannot be set by `ALTER SYSTEM`

    Laurenz Albe <laurenz.albe@cybertec.at> — 2023-10-16T17:37:46Z

    On Mon, 2023-10-16 at 12:29 -0400, Tom Lane wrote:
> I do see an issue here:
> 
> 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
> 
> and now we have
> 
> $ cat $PGDATA/postgresql.auto.conf
> # Do not edit this file manually!
> # It will be overwritten by the ALTER SYSTEM command.
> foo.bar = 'baz'
> 
> So that feels like a bug: we should not allow ALTER SYSTEM to execute
> against a placeholder GUC definition, because the placeholder can't
> tell us whether the value is valid.  I wonder though if forbidding
> this would break any legitimate usage patterns.

I feel the same.  However, the lack of any "variables" in SQL (as proposed
in [1]) leads a lot of people to abuse placeholder parameters as variables
to hold application state.  I am sure that that is where this complaint
comes from.  We maintain that doing so is not a valid use case, but that claim
sounds increasingly like a grammarian declaring that sentences should not
end with a preposition, when everybody does it all the time.

Yours,
Laurenz Albe

 [1]: https://postgr.es/m/CAFj8pRDY%2Bm9OOxfO10R7J0PAkCCauM-TweaTrdsrsLGMb1VbEQ%40mail.gmail.com

  4. Re: "20.16. Customized Options" – cannot be set by `ALTER SYSTEM`

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-16T20:21:09Z

    Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Mon, 2023-10-16 at 12:29 -0400, Tom Lane wrote:
>> So that feels like a bug: we should not allow ALTER SYSTEM to execute
>> against a placeholder GUC definition, because the placeholder can't
>> tell us whether the value is valid.  I wonder though if forbidding
>> this would break any legitimate usage patterns.

> I feel the same.  However, the lack of any "variables" in SQL (as proposed
> in [1]) leads a lot of people to abuse placeholder parameters as variables
> to hold application state.  I am sure that that is where this complaint
> comes from.  We maintain that doing so is not a valid use case, but that claim
> sounds increasingly like a grammarian declaring that sentences should not
> end with a preposition, when everybody does it all the time.

Yeah, and we have been slowly removing the issues that made us not want
to recommend using them like that.

Anyway, I realized that I was wrong to claim that we need ALTER SYSTEM
to defend us against bogus values of extension parameters in the config
file.  Checking is an important thing to do for core parameters, but
a faulty extension parameter doesn't stop the system from booting.
That's because we'll apply all the config file entries before we load
any extensions, even ones listed in shared_preload_libraries.  When
we do load an extension, if it doesn't like what it finds in a placeholder
then you get a WARNING and the parameter's default value is substituted.
So there's no risk of an unstartable system.

So maybe we should allow ALTER SYSTEM for unrecognized parameters,
as long as the parameter name is syntactically legit and you're a
superuser.

			regards, tom lane

  5. Re: "20.16. Customized Options" – cannot be set by `ALTER SYSTEM`

    Laurenz Albe <laurenz.albe@cybertec.at> — 2023-10-17T01:12:18Z

    On Mon, 2023-10-16 at 16:21 -0400, Tom Lane wrote:
> So maybe we should allow ALTER SYSTEM for unrecognized parameters,
> as long as the parameter name is syntactically legit and you're a
> superuser.

That seems more consistent than the current behavior, so +1.

Yours,
Laurenz Albe