From: Tom Lane <tgl@sss.pgh.pa.us>
To: Stephen Frost <sfrost@snowman.net>
cc: Ian Barwick <ian.barwick@2ndquadrant.com>,
        Robert Haas <robertmhaas@gmail.com>,
        Tomas Vondra <tomas.vondra@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
In-reply-to: <20190806214355.GA29202@tamriel.snowman.net>
References: <30179.1565029753@sss.pgh.pa.us>
 <CA+TgmoYEQyLE8Mtgko9-WSVWm=hLNGi3FO4xNLTrLpwi2GgPHw@mail.gmail.com>
 <30823.1565030571@sss.pgh.pa.us>
 <CA+TgmobRpgiqCZ_FMUeDW4RM1W+RzT4HA2xSQy=QMcfv=X47jw@mail.gmail.com>
 <32103.1565032053@sss.pgh.pa.us> <20190806005225.GM29202@tamriel.snowman.net>
 <6cba0dff-485b-6781-d659-0a376b966c89@2ndquadrant.com>
 <20190806021616.GP29202@tamriel.snowman.net>
 <54620b18-60b7-4b9b-7d1d-90a6464ab333@2ndquadrant.com>
 <5879.1565121423@sss.pgh.pa.us> <20190806214355.GA29202@tamriel.snowman.net>
Comments: In-reply-to Stephen Frost <sfrost@snowman.net>
	message dated "Tue, 06 Aug 2019 17:43:55 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <29731.1565810098.0@sss.pgh.pa.us>
Date: Wed, 14 Aug 2019 15:15:42 -0400
Message-ID: <29770.1565810142@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <29731.1565810098.1@sss.pgh.pa.us>

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> Perhaps we could put some of these details into the Notes section of the
>> ALTER SYSTEM ref page.  But I wonder how much of this is needed at all.

> I'd be alright with that too, but I'd be just as fine with even a README
> or something that we feel other hackers and external tool developers
> would be likely to find.  I agree that all of this isn't something that
> your run-of-the-mill DBA needs to know, but they are things that I'm
> sure external tool authors will care about (including myself, David S,
> probably the other backup/restore tool maintainers, and at least the
> author of pg_conftool, presumably).

In hopes of moving this along, I've pushed Ian's last code change,
as there seems to be no real argument about that anymore.

As for the doc changes, how about the attached revision of what
I wrote previously?  It gives some passing mention to what ALTER
SYSTEM will do, without belaboring it or going into things that
are really implementation details.

As an example of the sort of implementation detail that I *don't*
want to document, I invite you to experiment with the difference
between
	ALTER SYSTEM SET TimeZone = 'America/New_York';
	ALTER SYSTEM SET "TimeZone" = 'America/New_York';

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="doc-alter-system-better-2.patch";
	charset="us-ascii"
Content-ID: <29731.1565810098.2@sss.pgh.pa.us>
Content-Description: doc-alter-system-better-2.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index cdc30fa..e99482d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -153,6 +153,8 @@ shared_buffers =3D 128MB
      identifiers or numbers must be single-quoted.  To embed a single
      quote in a parameter value, write either two quotes (preferred)
      or backslash-quote.
+     If the file contains multiple entries for the same parameter,
+     all but the last one are ignored.
     </para>
 =

     <para>
@@ -185,18 +187,29 @@ shared_buffers =3D 128MB
      In addition to <filename>postgresql.conf</filename>,
      a <productname>PostgreSQL</productname> data directory contains a fi=
le
      <filename>postgresql.auto.conf</filename><indexterm><primary>postgre=
sql.auto.conf</primary></indexterm>,
-     which has the same format as <filename>postgresql.conf</filename> bu=
t should
-     never be edited manually.  This file holds settings provided through
-     the <xref linkend=3D"sql-altersystem"/> command.  This file is autom=
atically
-     read whenever <filename>postgresql.conf</filename> is, and its setti=
ngs take
-     effect in the same way.  Settings in <filename>postgresql.auto.conf<=
/filename>
-     override those in <filename>postgresql.conf</filename>.
+     which has the same format as <filename>postgresql.conf</filename> bu=
t
+     is intended to be edited automatically not manually.  This file hold=
s
+     settings provided through the <xref linkend=3D"sql-altersystem"/> co=
mmand.
+     This file is read whenever <filename>postgresql.conf</filename> is,
+     and its settings take effect in the same way.  Settings
+     in <filename>postgresql.auto.conf</filename> override those
+     in <filename>postgresql.conf</filename>.
+    </para>
+
+    <para>
+     External tools may also
+     modify <filename>postgresql.auto.conf</filename>.  It is not
+     recommended to do this while the server is running, since a
+     concurrent <command>ALTER SYSTEM</command> command could overwrite
+     such changes.  Such tools might simply append new settings to the en=
d,
+     or they might choose to remove duplicate settings and/or comments
+     (as <command>ALTER SYSTEM</command> will).
     </para>
 =

     <para>
      The system view
      <link linkend=3D"view-pg-file-settings"><structname>pg_file_settings=
</structname></link>
-     can be helpful for pre-testing changes to the configuration file, or=
 for
+     can be helpful for pre-testing changes to the configuration files, o=
r for
      diagnosing problems if a <systemitem>SIGHUP</systemitem> signal did =
not have the
      desired effects.
     </para>

------- =_aaaaaaaaaa0--