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

On 8/3/19 7:27 AM, Tom Lane 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
>
>> 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.

Having played around with the pg.auto.conf stuff for a while, my feeling is
that ALTER SYSTEM does indeed have a license to rewrite it (which is what
currently happens anyway, with comments and include directives [1] being silently
removed) so it seems reasonable to remove duplicate entries and ensure
the correct one is processed.

[1] suprisingly any include directives present are honoured, which seems crazy
to me, see: https://www.postgresql.org/message-id/flat/8c8bcbca-3bd9-dc6e-8986-04a5abdef142%402ndquadrant.com

>> The main issue however is that no code was written yet.
>
> Seems like it ought to be relatively simple ... but I didn't look.

The patch I originally sent does exactly this.

The thread then drifted off into a discussion about providing ways for
applications to properly write to pg.auto.conf while PostgreSQL is not
running; I have a patch for that which I can submit later (though it
is a thing of considerable ugliness).

Regards

Ian Barwick

--
Ian Barwick https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services