Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions

Hi,

On 2019-08-02 18:38:46 -0400, Stephen Frost wrote:
> On Fri, Aug 2, 2019 at 18:27 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> > Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
> > > There seems to be a consensus that this this not a pg_basebackup issue
> > > (i.e. duplicate values don't make the file invalid), and it should be
> > > handled in ALTER SYSTEM.
> >
> > Yeah. I doubt pg_basebackup is the only actor that can create such
> > situations.
> >
> > > The proposal seems to be to run through the .auto.conf file, remove any
> > > duplicates, and append the new entry at the end. That seems reasonable.
> >
> > +1

> I disagree that this should only be addressed in alter system, as I’ve said
> before and as others have agreed with. Having one set of code that can be
> used to update parameters in the auto.conf and then have that be used by
> pg_basebackup, alter system, and external tools, is the right approach.
>
> The idea that alter system should be the only thing that doesn’t just
> append changes to the file is just going to lead to confusion and bugs down
> the road.

To me that seems like an alternative that needs a good chunk more work
than just having ALTER SYSTEM fix things up, and isn't actually likely
to prevent such scenarios from occurring in practice. Providing a
decent API to change conflict files from various places, presumably
including a commandline utility to do so, would be a nice feature, but
it seems vastly out of scope for v12. My vote is to fix this via ALTER
SYSTEM in v12, and then for whoever is interested enough to provide
better tools down the road.

> As I said before, an alternative could be to make alter system simply
> always append and declare that to be the way to update parameters in the
> auto.conf.

Why would that be a good idea? We'd just take longer and longer to parse
it. There's people that change database settings on a regular and
automated basis using ALTER SYSTEm.

> > There was a discussion whether to print warnings about the duplicates. I
> > > personally see not much point in doing that - if we consider duplicates
> > > to be expected, and if ALTER SYSTEM has the license to rework the config
> > > file any way it wants, why warn about it?
> >
> > Personally I agree that warnings are unnecessary.

+1

Greetings,

Andres Freund