On Wed, Mar 20, 2024 at 10:31 PM Magnus Hagander <mag...@hagander.net> wrote: > > On Wed, Mar 20, 2024 at 8:52 PM Robert Haas <robertmh...@gmail.com> wrote: >> >> On Wed, Mar 20, 2024 at 3:17 PM Magnus Hagander <mag...@hagander.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 :) >
And yet somehow query hints are more scary than ALL of these things. Go figure! Robert Treat https://xzilla.net