Date: Fri, 21 Jun 2019 13:01:33 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Ian Barwick <ian.barwick@2ndquadrant.com>,
	Amit Kapila <amit.kapila16@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions
Message-ID: <20190621170133.GQ2480@tamriel.snowman.net>
References: <aed6cc9f-98f3-2693-ac81-52bb0052307e@2ndquadrant.com>
 <20190614160835.GF2480@tamriel.snowman.net>
 <CAA4eK1JW9UfLOtMEJq5b_YcogbWdxzAGUzNi72F2Xx4r1_h6QA@mail.gmail.com>
 <20190616171610.GO2480@tamriel.snowman.net>
 <27dc9f8d-8d8f-9dd1-0494-5d8bd2f49a45@2ndquadrant.com>
 <CA+TgmobVjrvVQB_gVytgBcPRmBvptN3wHQy5T6=dYCDZ5n5MGA@mail.gmail.com>
 <20372.1561134638@sss.pgh.pa.us>
 <20190621164028.GO2480@tamriel.snowman.net>
 <21353.1561136112@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="k9ssVBY1NpawPNl1"
Content-Disposition: inline
In-Reply-To: <21353.1561136112@sss.pgh.pa.us>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--k9ssVBY1NpawPNl1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> >> I haven't been paying too close attention to this thread, but isn't
> >> that exactly what it does now and always has?  guc.c, at least, certai=
nly
> >> is going to interpret duplicate entries that way.
>=20
> > The issue isn't with reading them and interpreting them, it's what
> > happens when you run ALTER SYSTEM and it goes and modifies the file.
> > Presently, it basically operates on the first entry it finds when
> > performing a SET or a RESET.
>=20
> Ah, got it.  So it seems like the correct behavior might be for
> ALTER SYSTEM to
> (a) run through the whole file and remove any conflicting lines;
> (b) append new setting at the end.

Sure- and every other tool that modifies that file should know that
*that* is how you do it, and therefore, if everyone is doing it right,
you don't ever end up with duplicates in the file.  If you do, someone's
doing it wrong, and we should issue a warning.

That's more-or-less the conclusion on the other thread, as I understood
it.

> If you had some fancy setup with comments associated with entries,
> you might not be pleased with that.  But I can't muster a lot of
> sympathy for tools putting comments in postgresql.auto.conf anyway;
> it's not intended to be a human-readable file.

If we were to *keep* the duplicates, then I could see value in including
information about prior configuration entries (I mean, that's what a lot
of external tools do with our postgresql.conf file- put it into git or
some other configuration management tool...).  If we aren't keeping the
dups, then I agree that there doesn't seem much point.

> If anybody does complain, my first reaction would be to make ALTER
> SYSTEM strip all comment lines too.

Uh, I believe it already does?

Thanks,

Stephen

--k9ssVBY1NpawPNl1
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJdDQ1tAAoJEO1sijiDR2RV3PUQAIvDbXY3kpCu6oFzlRZZkilJ
4zMLA5gNCp27HYnpepeMFt8wZ2VkNPx4m5v84sXc/6C2ivhgSPId2ifx+bA4Erd4
/3mkn7pgq58tXway4JijtFY4FeY1ATrV/HoTOHmU/lZ36mIKk1D58o8arTOi8WTA
98GcRXb/63VzmvkCtEwW+rdrwdtz0Yz+vepxGzHLzJKUb+XJuesbiFy6JFegPNH9
EnhrVDTBSKjnPFol8obJq7tidQwNOpOpnz/1G+MIUEZBeEIuSfz6nn1HMTbJsAGG
RiFjwbkiK/5kM7On0ln7lmOyRtgKyqNfjXzkQkVMJRJ1tgl2uR1MZOaivrU163ZJ
hkHYI93IJHGSXUkBkx5ZZNFzC9YRVgTuG6p50vKOm4XKLzwbFrz/z4o7gY6uac3J
0DFGRc7E6wefOsDGpR+GvBWYp/o3OwKQ+RRhF5IhKMEtBjJH3CKXLYgvATXbvbyR
6CZUD+dspoRqzZYRmNYjCmmdV0ny2DYVJYk+uu6fkHJVYAbWICp4PHzB5s4jbcNf
3rot43dZwPv1tYQNUkyC30c8vsOIx413BLSzjIiEWOAzSkU+pYDi38t00vVWvCNF
QnfgVcqksoI1BJgGAaK3psgps3wmQFUFFRgU0fFw5loSPJHLdAzpq4lvo3q6nAUP
oNnfgNvvf0jc3S26+Efl
=Xbw8
-----END PGP SIGNATURE-----

--k9ssVBY1NpawPNl1--