Re: Possibility to disable `ALTER SYSTEM`

On Wed, Mar 20, 2024 at 8:52 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Wed, Mar 20, 2024 at 3:17 PM Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > Right, what I meant is that making it a packaging decision is the better
> place. Wherever it goes, allowing the administrator to choose what fits
> them should be made possible.
>
> +1. Which is also the justification for this patch, when it comes
> right down to it. The administrator gets to decide how the contents of
> postgresql.conf are to be managed on their particular installation.
>

Not really. The administrator can *already* do that. It's trivial.

This patch is about doing it in a way that doesn't produce as ugly a
message.But if we're "delegating" it to packagers and "os administrators",
then the problem is already solved. This patch is about trying to solve it
*without* involving the packagers or OS administrators.

Not saying we shouldn't do it, but I'd argue the exact opposite of yours
aboe, which is that it's very much not the justification of the patch :)

> They can decide that postgresql.conf should be writable by the same
> user that runs PostgreSQL, or not. And they should also be able to
> decide that ALTER SYSTEM is an OK way to change configuration, or that
> it isn't. How we enable them to make that decision is a point for
> discussion, and how exactly we phrase the documentation is a point for
> discussion, but we have no business trying to impose conditions, as if
> they're only allowed to make that decision if they conform to some
> (IMHO ridiculous) requirements that we dictate from on high. It's
> their system, not ours.
>

Agreed on all those except they can already do this. It's just that the
error message is ugly. The path of least resistance would be to just
specifically detect a permissions error on the postgresql.auto.conf file
when you try to do ALTER SYSTEM, and throw at least an error hint about
"you must allow writing to this file for the feature to work".

So this patch isn't at all about enabling this functionality. It's about
making it more user friendly.

I mean, for crying out loud, users can set enable_seqscan=off in
> postgresql.conf and GLOBALLY DISABLE SEQUENTIAL SCANS. They can set
>

This is actually a good example, because it's kind of like this patch. It
doesn't *actually* disable the ability to run sequential scans, it just
disables the "usual way". Just like this patch doesn't prevent the
superuser from editing the config, but it does prevent them droin doing it
"the usual way".

> zero_damaged_pages=on in postgresql.conf and silently remove vast
> quantities of data without knowing that they're doing anything. We
> don't even question that stuff ... although we probably should be
>

I like how you got this far and didn't even mention fsync=off :)

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>