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

Ian Barwick <ian(dot)barwick(at)2ndquadrant(dot)com> writes:
> On 8/3/19 7:27 AM, Tom Lane wrote:
>> Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
>>> 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.

Ah, you did send a patch, but that tries to maintain the existing behavior
of replacing the last occurrence in-place. I think it's simpler and more
sensible to just make a sweep to delete all matches, and then append the
new setting (if any) at the end, as attached.

A more aggressive patch would try to de-duplicate the entire list, not
just the current target entry ... but I'm not really excited about doing
that in a back-patchable bug fix.

I looked at the TAP test you proposed and couldn't quite convince myself
that it was worth the trouble. A new test within an existing suite
would likely be fine, but a whole new src/test/ subdirectory just for
pg.auto.conf seems a bit much. (Note that the buildfarm and possibly
the MSVC scripts would have to be taught about each such subdirectory.)
At the same time, we lack any better place to put such a test :-(.
Maybe it's time for a "miscellaneous TAP tests" subdirectory?

regards, tom lane