From: Ian Barwick <ian.barwick@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions
Date: Mon, 5 Aug 2019 15:42:30 +0900
Message-ID: <fa87170f-ca92-a93c-e166-d8bd57fc53f3@2ndquadrant.com> (raw)
In-Reply-To: <16337.1564859628@sss.pgh.pa.us>
References: <CA+TgmobVjrvVQB_gVytgBcPRmBvptN3wHQy5T6=dYCDZ5n5MGA@mail.gmail.com>
<20372.1561134638@sss.pgh.pa.us>
<20190621164028.GO2480@tamriel.snowman.net>
<21353.1561136112@sss.pgh.pa.us>
<CA+TgmoZVx-vco-39EN9MxO9BaG-oVY+NuDeZPGdS=_bX5VH70w@mail.gmail.com>
<20190624185647.GW2480@tamriel.snowman.net>
<15032.1561403529@sss.pgh.pa.us>
<20190624192014.GB2480@tamriel.snowman.net>
<56aff4f3-dec8-f462-7feb-0938411b04a6@2ndquadrant.com>
<CAA4eK1+JMm=tvx720y6jhD8pdzt=HdS1-TwQwkARib2pwK8EvQ@mail.gmail.com>
<20190802222254.qsvqjtkwte464lu3@development>
<31697.1564784856@sss.pgh.pa.us>
<c87c6599-8f60-754f-620d-c30faa0dceea@2ndquadrant.com>
<16337.1564859628@sss.pgh.pa.us>
On 8/4/19 4:13 AM, Tom Lane wrote:
> Ian Barwick <ian.barwick@2ndquadrant.com> writes:
>> On 8/3/19 7:27 AM, Tom Lane wrote:
>>> Tomas Vondra <tomas.vondra@2ndquadrant.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.
Yes, that is less convoluted.
> 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 thought about doing that but it's more of a nice-to-have and not essential
to fix the issue, as any other duplicate entries will get removed the next
time ALTER SYSTEM is run on the entry in question. Maybe as part of a future
improvement.
> 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.)
Didn't know that, but couldn't find anywhere obvious to put the test.
> At the same time, we lack any better place to put such a test :-(.
> Maybe it's time for a "miscellaneous TAP tests" subdirectory?
Sounds reasonable.
Regards
Ian Barwick
--
Ian Barwick https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ian.barwick@2ndquadrant.com, tgl@sss.pgh.pa.us, tomas.vondra@2ndquadrant.com, amit.kapila16@gmail.com, sfrost@snowman.net, robertmhaas@gmail.com
Subject: Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions
In-Reply-To: <fa87170f-ca92-a93c-e166-d8bd57fc53f3@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox