On Fri, Jun 21, 2019 at 11:24 AM Stephen Frost <sfrost@snowman.net> wrote:
> That's not quite accurate, given that it isn't how the ALTER SYSTEM call
> itself works, and clearly isn't how the authors of that feature expected
> things to work or they would have actually made it work. They didn't,
> and it doesn't actually work.
>
> The notion that pg_basebackup was correct in this, when it wasn't tested
> at all, evidently, even after the concern was raised, and ALTER SYSTEM
> was wrong, even though it worked just fine before some later patch
> started making changes to the file, based on the idea that it's the
> "natural approach" doesn't make sense to me.
>
> If the change to pg_basebackup had included a change to ALTER SYSTEM to
> make it work the *same* way that pg_basebackup now does, or at least to
> work with the changes that pg_basebackup were making, then maybe
> everything would have been fine.
This argument boils down to: two people patches don't play nicely
together, and we should assume that the first patch had it right and
the second patch had it wrong, because the first patch was first.
I don't think it works like that. I think we should decide which patch
had it right by looking at what the nicest behavior actually is, not
by which one came first. In my mind having ALTER SYSTEM drop
duplicate that other tools may have introduced is a clear winner with
basically no downside. You are arguing that it will produce confusion,
but I don't really understand who is going to be confused or why they
are going to be confused. We can document whatever we do, and it
should be fine. Humans aren't generally supposed to be examining this
file anyway, so they shouldn't get confused very often.
In my view, the original ALTER SYSTEM patch just has a bug -- it
doesn't modify the right copy of the setting when multiple copies are
present -- and we should just fix the bug.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company