Re: PG 12 draft release notes

Hi,

On 2019-05-21 15:47:34 -0400, Bruce Momjian wrote:
> On Mon, May 20, 2019 at 03:17:19PM -0700, Andres Freund wrote:
> > Hi,
> >
> > Note that I've added a few questions to individuals involved with
> > specific points. If you're in the To: list, please search for your name.
> >
> >
> > On 2019-05-11 16:33:24 -0400, Bruce Momjian wrote:
> > > I have posted a draft copy of the PG 12 release notes here:
> > >
> > >     http://momjian.us/pgsql_docs/release-12.html
> > > They are committed to git.
> >
> > Thanks!
> >
> >   <title>Migration to Version 12</title>
> >
> > There's a number of features in the compat section that are more general
> > improvements with a side of incompatibility. Won't it be confusing to
> > e.g. have have the ryu floating point conversion speedups in the compat
> > section, but not in the "General Performance" section?
>
> Yes, it can be.  What I did with the btree item was to split out the max
> length change with the larger changes.  We can do the same for other
> items.  As you rightly stated, it is for cases where the incompatibility
> is minor compared to the change.  Do you have a list of the ones that
> need this treatment?

I was concretely thinking of:
- floating point output changes, which are primarily about performance
- recovery.conf changes where I'd merge:
  - Do not allow multiple different recovery_target specificatios
  - Allow some recovery parameters to be changed with reload
  - Cause recovery to advance to the latest timeline by default
  - Add an explicit value of current for guc-recovery-target-time
  into on entry on the feature side.

After having to move recovery settings to a different file, disallowing
multiple targets isn't really a separate config break imo. And all the
other changes are also fallout from the recovery.conf GUCification.


> >      <para>
> >       Remove the special behavior of <link
> >       linkend="datatype-oid">OID</link> columns (Andres Freund,
> >       John Naylor)
> >      </para>
> >
> > Should we mention that tables with OIDs have to have their oids removed
> > before they can be upgraded?
>
> Uh, is that true?  pg_upgrade?  pg_dump?

Yes.


> >      <para>
> >       Refactor <link linkend="functions-geometry">geometric
> >       functions</link> and operators (Emre Hasegeli)
> >      </para>
> >
> >      <para>
> >       This could lead to more accurate, but slightly different, results
> >       from previous releases.
> >      </para>
> >     </listitem>
> >     <listitem>
> > <!--
> > Author: Tomas Vondra <tomas.vondra@postgresql.org>
> > 2018-08-16 [c4c340088] Use the built-in float datatypes to implement geometric
> > -->
> >
> >      <para>
> >       Restructure <link linkend="datatype-geometric">geometric
> >       types</link> to handle NaN, underflow, overflow and division by
> >       zero more consistently (Emre Hasegeli)
> >      </para>
> >     </listitem>
> >
> >     <listitem>
> > <!--
> > Author: Tomas Vondra <tomas.vondra@postgresql.org>
> > 2018-09-26 [2e2a392de] Fix problems in handling the line data type
> > -->
> >
> >      <para>
> >       Improve behavior and error reporting for the <link
> >       linkend="datatype-geometric">line data type</link> (Emre Hasegeli)
> >      </para>
> >     </listitem>
> >
> > Is that sufficient explanation? Feels like we need to expand a bit
> > more. In particular, is it possible that a subset of the changes here
> > require reindexing?
> >
> > Also, aren't three different entries a bit too much?
>
> The 'line' item related to more errors than just the ones listed for the
> geometric data types, so I was not clear how to do that as a single
> entry.  I think there is a much larger compatibility breakage
> possibility with 'line'.
>
> >       <listitem>
> > <!--
> > Author: Alexander Korotkov <akorotkov@postgresql.org>
> > 2018-07-28 [d2086b08b] Reduce path length for locking leaf B-tree pages during
> > Author: Peter Geoghegan <pg@bowt.ie>
> > 2019-03-25 [f21668f32] Add "split after new tuple" nbtree optimization.
> > -->
> >
> >        <para>
> >         Improve speed of btree index insertions (Peter Geoghegan,
> >         Alexander Korotkov)
> >        </para>
> >
> >        <para>
> >         The new code improves the space-efficiency of page splits,
> >         reduces locking overhead, and gives better performance for
> >         <command>UPDATE</command>s and <command>DELETE</command>s on
> >         indexes with many duplicates.
> >        </para>
> >       </listitem>
> >
> >       <listitem>
> > <!--
> > Author: Peter Geoghegan <pg@bowt.ie>
> > 2019-03-20 [dd299df81] Make heap TID a tiebreaker nbtree index column.
> > Author: Peter Geoghegan <pg@bowt.ie>
> > 2019-03-20 [fab250243] Consider secondary factors during nbtree splits.
> > -->
> >
> >        <para>
> >         Have new btree indexes sort duplicate index entries in heap-storage
> >         order (Peter Geoghegan, Heikki Linnakangas)
> >        </para>
> >
> >        <para>
> >         Indexes <application>pg_upgraded</application> from previous
> >         releases will not have this ordering.
> >        </para>
> >       </listitem>
> >
> > I'm not sure that the grouping here is quite right. And the second entry
> > probably should have some explanation about the benefits?
>
> Agreed.
>
> >       <listitem>
> > <!--
> > Author: Peter Eisentraut <peter_e@gmx.net>
> > 2018-11-14 [1b5d797cd] Lower lock level for renaming indexes
> > -->
> >
> >        <para>
> >         Reduce locking requirements for index renaming (Peter Eisentraut)
> >        </para>
> >       </listitem>
> >
> > Should we specify the newly required lock level? Because it's quire
> > relevant for users what exactly they're now able to do concurrently in
> > operation?
>
> Sure.
>
> >       <listitem>
> > <!--
> > Author: Tom Lane <tgl@sss.pgh.pa.us>
> > 2019-02-09 [1fb57af92] Create the infrastructure for planner support functions.
> > -->
> >
> >        <para>
> >         Add support for <link linkend="sql-createfunction">function
> >         selectivity</link> (Tom Lane)
> >        </para>
> >       </listitem>
> >
> > Hm, that message doesn't seem like an accurate description of that
> > commit (if anything it's a391ff3c?). Given that it all requires C
> > hackery, perhaps we ought to move it to the source code section? And
> > isn't the most important part of this set of changes
> >
> > commit 74dfe58a5927b22c744b29534e67bfdd203ac028
> > Author: Tom Lane <tgl@sss.pgh.pa.us>
> > Date:   2019-02-11 21:26:08 -0500
> >
> >     Allow extensions to generate lossy index conditions.
>
> Uh, I missed that as an important item.  Can someone give me some text?
>
> >       <listitem>
> > <!--
> > Author: Tomas Vondra <tomas.vondra@postgresql.org>
> > 2019-01-29 [36a1281f8] Separate per-batch and per-tuple memory contexts in COPY
> > Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
> > 2019-01-25 [9556aa01c] Use single-byte Boyer-Moore-Horspool search even with mu
> > Author: Andres Freund <andres@anarazel.de>
> > 2019-01-26 [a9c35cf85] Change function call information to be variable length.
> > -->
> >
> >        <para>
> >         Greatly reduce memory consumption of <xref linkend="sql-copy"/>
> >         and function calls (Andres Freund, Tomas Vondra, Tom Lane)
> >        </para>
> >       </listitem>
> >
> > Grouping these three changes together makes no sense to me.
> >
> > I think the first commit just ought not to be mentioned separately, it's
> > just a fix for a memory leak in 31f3817402, essentially a 12 only bugfix?
>
> Oh, I was not aware of that.
>
> > The second commit is about position() etc, which seems not to match that
> > description either?
>
> Ugh.
>
> > The third is probably more appropriate to be in the source code
> > section. While it does speed up function calls a bit (in particular
> > plpgsql which is very function call heavy), it also is a breaking change
> > for some external code? Not sure why Tom is listed with this entry?
>
> The order of names is just a guess when multiple commits are merged ---
> this needs help.
>
> >       <listitem>
> > <!--
> > Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
> > 2019-01-25 [9556aa01c] Use single-byte Boyer-Moore-Horspool search even with mu
> > -->
> >
> >        <para>
> >         Improve search performance for multi-byte characters (Heikki
> >         Linnakangas)
> >        </para>
> >       </listitem>
> >
> > That's the second reference to the commit. I suspect this is much better
> > separate, so I'd just remove it from above.
>
> Done.
>
> >       <listitem>
> > <!--
> > Author: Stephen Frost <sfrost@snowman.net>
> > 2019-04-02 [4d0e994ee] Add support for partial TOAST decompression
> > -->
> >
> >        <para>
> >         Allow <link linkend="storage-toast"><literal>TOAST</literal></link>
> >         values to be minimally decompressed (Paul Ramsey)
> >        </para>
> >
> > I'd s/minimal/partial/ - I don't think the code guarantees anything
> > about it being minimal? And "minimally decompressed" also is somewhat
> > confusing, because it sounds like it's about the compression quality
> > rather than only decompressing part of the data.
>
> It is confusing.  Is "partially decompressed" better?
>
> >       <listitem>
> > <!--
> > Author: Michael Paquier <michael@paquier.xyz>
> > 2018-08-10 [f841ceb26] Improve TRUNCATE by avoiding early lock queue
> > -->
> >
> >        <para>
> >         Prevent <xref linkend="sql-truncate"/> from requesting a lock on
> >         tables for which it lacks permission (Michaël Paquier)
> >        </para>
> >
> >        <para>
> >         This prevents unauthorized locking delays.
> >        </para>
> >       </listitem>
> >
> >       <listitem>
> > <!--
> > Author: Michael Paquier <michael@paquier.xyz>
> > 2018-08-27 [a556549d7] Improve VACUUM and ANALYZE by avoiding early lock queue
> > -->
> >
> >        <para>
> >         Prevent <command>VACUUM</command> and <command>ANALYZE</command>
> >         from requesting a lock on tables for which it lacks permission
> >         (Michaël Paquier)
> >        </para>
> >
> >        <para>
> >         This prevents unauthorized locking delays.
> >        </para>
> >       </listitem>
> >
> >
> > I don't think this should be in the <title><acronym>Authentication</acronym></title>
> > section.
>
> I put it in that section since I thought the motivation was to prevent
> people from locking up connecting to the database if someone has a
> pending VACUUM/ANALYZE.  No?
>
> > Also perhaps, s/it/the user/, or "the caller"?
>
> Agreed, "the user".
>
> >       <listitem>
> > <!--
> > Author: Tom Lane <tgl@sss.pgh.pa.us>
> > 2019-03-10 [cbccac371] Reduce the default value of autovacuum_vacuum_cost_delay
> > -->
> >
> >        <para>
> >         Reduce the default value of <xref
> >         linkend="guc-autovacuum-vacuum-cost-delay"/> to 2ms (Tom Lane)
> >        </para>
> >       </listitem>
> >
> > I think this needs to explain that this can increase autovacuum's IO
> > throughput considerably.
>
> Uh, well, do we normally document the effect of a change like this?  It
> will cause vacuum to be more agressive, and increase I/O?  Do we want to
> re-educate on what this paramater does?
>
> >       <listitem>
> > <!--
> > Author: Tom Lane <tgl@sss.pgh.pa.us>
> > 2019-03-10 [caf626b2c] Convert [autovacuum_]vacuum_cost_delay into floating-poi
> > -->
> >
> >        <para>
> >         Allow <xref linkend="guc-vacuum-cost-delay"/> to specify
> >         sub-millisecond delays (Tom Lane)
> >        </para>
> >
> >        <para>
> >         Floating-point values can also now be specified.
> >        </para>
> >       </listitem>
> >
> > And this should be merged with the previous entry?
>
> Uh, I thought the change of default and its range were different enough
> that combining them would add confusion.
>
> >       <listitem>
> > <!--
> > Author: Tom Lane <tgl@sss.pgh.pa.us>
> > 2019-03-10 [caf626b2c] Convert [autovacuum_]vacuum_cost_delay into floating-poi
> > -->
> >
> >        <para>
> >         Allow time-based server variables to use <link
> >         linkend="config-setting">micro-seconds</link> (us) (Tom Lane)
> >        </para>
> >       </listitem>
> >
> >       <listitem>
> > <!--
> > Author: Tom Lane <tgl@sss.pgh.pa.us>
> > 2019-03-11 [1a83a80a2] Allow fractional input values for integer GUCs, and impr
> > -->
> >
> >        <para>
> >         Allow fractional input for integer server variables (Tom Lane)
> >        </para>
> >
> >        <para>
> >         For example, <command>SET work_mem = '30.1GB'</command>.
> >        </para>
> >       </listitem>
> >
> >       <listitem>
> > <!--
> > Author: Tom Lane <tgl@sss.pgh.pa.us>
> > 2019-03-10 [caf626b2c] Convert [autovacuum_]vacuum_cost_delay into floating-poi
> > -->
> >
> >        <para>
> >         Allow units to be specified for floating-point server variables
> >         (Tom Lane)
> >        </para>
> >       </listitem>
> >
> > Can't we combine these? Seems excessively detailed in comparison to the
> > rest of the entries.
>
> See above.  It seems confusing to combine them but please propose text
> if you think it is possible.
>
> >      <listitem>
> > <!--
> > Author: Peter Eisentraut <peter@eisentraut.org>
> > 2019-01-11 [ff8530605] Add value 'current' for recovery_target_timeline
> > -->
> >
> >       <para>
> >        Add an explicit value of <literal>current</literal> for <xref
> >        linkend="guc-recovery-target-time"/> (Peter Eisentraut)
> >       </para>
> >      </listitem>
> >
> > Seems like this should be combined with the earlier "Cause recovery to
> > advance to the latest timeline by default" entry.
>
> The odd part is that the old default was 'current' but there was no way
> to specify current --- you just specified nothing.  That seemed
> confusing enough that having them combined would add confusion, but if
> you have some suggested text?
>
> >      <listitem>
> > <!--
> > Author: Peter Eisentraut <peter@eisentraut.org>
> > 2019-03-30 [fc22b6623] Generated columns
> > -->
> >
> >       <para>
> >        Add support for <link linkend="sql-createtable">generated
> >        columns</link> (Peter Eisentraut)
> >       </para>
> >
> >       <para>
> >        Rather than storing a value only at row creation time, generated
> >        columns are also modified during updates, and can reference other
> >        table columns.
> >       </para>
> >      </listitem>
> >
> > I find this description confusing. How about cribbing from the commit?
> > Roughly like
> >
> >     This allows creating columns that are computed from expressions,
> >     including references to other columns in the same table, rather than
> >     having to be specified by the inserter/updater.
> >
> > Think we also ought to mention that this is only stored generated
> > columns, given that the SQL feature also includes virtual columns?
>
> OK, new text is:
>
>        The content of generated columns are computed from expressions
>        (including references to other columns in the same table)
>        rather than being specified by <command>INSERT</command> or
>        <command>UPDATE</command> commands.
> >
> >      <listitem>
> > <!--
> > Author: Fujii Masao <fujii@postgresql.org>
> > 2019-04-08 [119dcfad9] Add vacuum_truncate reloption.
> > Author: Fujii Masao <fujii@postgresql.org>
> > 2019-05-08 [b84dbc8eb] Add TRUNCATE parameter to VACUUM.
> > -->
> >
> >       <para>
> >        Add <xref linkend="sql-vacuum"/> and <command>CREATE
> >        TABLE</command> options to prevent <command>VACUUM</command>
> >        from truncating trailing empty pages (Tsunakawa Takayuki)
> >       </para>
> >
> >       <para>
> >        The options are <varname>vacuum_truncate</varname> and
> >        <varname>toast.vacuum_truncate</varname>.  This reduces vacuum
> >        locking requirements.
> >       </para>
> >      </listitem>
> >
> > Maybe add something like: "This can be helpful to avoid query
> > cancellations on standby that are not avoided by hot_standby_feedback."?
>
> So you turn off truncate on the primary becaues the replay of the
> truncate on the standby might cause a cancelation?  I was not aware that
> was a common problem.
>
> >      <listitem>
> > <!--
> > Author: Robert Haas <rhaas@postgresql.org>
> > 2019-04-04 [a96c41fee] Allow VACUUM to be run with index cleanup disabled.
> > -->
> >
> >       <para>
> >        Allow vacuum to avoid index cleanup with the
> >        <literal>INDEX_CLEANUP</literal> option (Masahiko Sawada)
> >       </para>
> >      </listitem>
> >
> > I think we ought to expand a bit more on why one would do that,
> > including perhaps some caveat?
>
> I actually have no idea why someone would want to do that.
>
> >      <listitem>
> > <!--
> > Author: Peter Eisentraut <peter@eisentraut.org>
> > 2019-03-19 [590a87025] Ignore attempts to add TOAST table to shared or catalog
> > -->
> >
> >       <para>
> >        Allow modifications of system tables using <xref
> >        linkend="sql-altertable"/> (Peter Eisentraut)
> >       </para>
> >
> >       <para>
> >        This allows modifications of <literal>reloptions</literal> and
> >        autovacuum settings.
> >       </para>
> >      </listitem>
> >
> > I think the first paragraph is a bit dangerous. This does *not*
> > generally allow modifications of system tables using ALTER TABLE.
>
> OK, new text added "options":
>
>        Allow modifications of system table options using <xref
>        linkend="sql-altertable"/> (Peter Eisentraut)
>
> >       <listitem>
> > <!--
> > Author: Tom Lane <tgl@sss.pgh.pa.us>
> > 2018-09-25 [5b7e03670] Avoid unnecessary precision loss for pgbench's - -rate ta
> > -->
> >
> >        <para>
> >         Compute behavior based on pgbench's <option>--rate</option>
> >         value more precisely (Tom Lane)
> >        </para>
> >       </listitem>
> >
> > "Computing behavior" sounds a bit odd. Maybe "Improve precision of
> > pgbench's <option>--rate</option>" option?
>
> Done.
>
> >       <listitem>
> > <!--
> > Author: Thomas Munro <tmunro@postgresql.org>
> > 2018-07-13 [387a5cfb9] Add pg_dump - -on-conflict-do-nothing option.
> > -->
> >
> >        <para>
> >         Allow restoration of an <command>INSERT</command>-statement dump
> >         to skip rows which would cause conflicts (Surafel Temesgen)
> >        </para>
> >
> >        <para>
> >         The <application>pg_dump</application> option is
> >         <option>--on-conflict-do-nothing</option>.
> >        </para>
> >       </listitem>
> >
> > Hm, this doesn't seem that clear. It's not really a restoration time
> > option, and it sounds a bit like that in the above. How about instead saying something
> > like:
> > Allow pg_dump to emit INSERT ... ON CONFLICT DO NOTHING (Surafel).
>
> Done.
>
> >       <listitem>
> > <!--
> > Author: Andrew Dunstan <andrew@dunslane.net>
> > 2019-02-18 [af25bc03e] Provide an extra-float-digits setting for pg_dump / pg_d
> > -->
> >
> >        <para>
> >         Allow the number of float digits to be specified
> >         for <application>pg_dump</application> and
> >         <application>pg_dumpall</application> (Andrew Dunstan)
> >        </para>
> >
> >        <para>
> >         This allows the float digit output to match previous dumps.
> >        </para>
> >
> > Hm, feels like that should be combined with the ryu compat entry?
>
> Uh, but it relates to this specific command, and it is a new feature
> rather than a compatibility.
>
> >       <para>
> >        Add <xref linkend="sql-create-access-method"/> command to create
> >        new table types (Haribabu Kommi, Andres Freund, Álvaro Herrera,
> >        Dimitri Dolgov)
> >       </para>
> >
> > A few points:
> >
> > 1) Is this really source code, given that CREATE ACCESS METHOD TYPE
> >    TABLE is a DDL command, and USING (...) for CREATE TABLE etc is an
> >    option to DDL commands?
>
> I struggled with this.  It is a new command, but it has no use yet to
> users, so if we move it out of "source code" we need to be clear it has
> no useful purpose yet.  Can we do that clearly?
>
>
> > 2) I think the description sounds a bit too much like it's about new
> >    forms of tables, rather than their storage. How about something
> >    roughly like:
> >
> >    Allow different <link linkend="tableam">table access methods</> to be
> >    <link linkend="sql-create-access-method>created</> and <link
> >    linkend="sql-createtable-method">used</>. This allows to develop and
> >    use new ways of storing and accessing table data, optimized for
> >    different use-cases, without having to modify
> >    PostgreSQL. The existing <literal>heap</literal> access method
> >    remains the default.
>
> I added a new detail paragraph:
>
>        This enables the development of new <link linkend="tableam">table
>        access methods</>, which can optimize storage for different
>        use-cases.  The existing <literal>heap</literal> access method
>        remains the default.
>
> > 3) This misses a large set of commits around making tableam possible, in
> >    particular the commits around
> >
> > commit 4da597edf1bae0cf0453b5ed6fc4347b6334dfe1
> > Author: Andres Freund <andres@anarazel.de>
> > Date:   2018-11-16 16:35:11 -0800
> >
> >     Make TupleTableSlots extensible, finish split of existing slot type.
> >
> >    Given that those commits entail an API break relevant for extensions,
> >    should we have them as a separate "source code" note?
>
> I have added this commit to the table-am item.  I don't know if this is
> something that extension people care about, but if so, we should
> certainly add it.
>
> > 4) I think the attribution isn't quite right. For one, a few names with
> >    substantial work are missing (Amit Khandekar, Ashutosh Bapat,
> >    Alexander Korotkov), and the order doesn't quite seem right. On the
> >    latter part I might be somewhat petty, but I spend *many* months of
> >    my life on this.
> >
> >    How about:
> >    Andres Freund, Haribabu Kommi, Alvaro Herrera, Alexander Korotkov, David Rowley, Dimitri Golgov
> >    if we keep 3) separate and
>
> I used the above list since I combined 3 so far.
>
> >    Andres Freund, Haribabu Kommi, Alvaro Herrera, Ashutosh Bapat, Alexander Korotkov, Amit Khandekar, David Rowley,
DimitriGolgov 
> >    otherwise?
> >
> >    I think it might actually make sense to take David off this list,
> >    because his tableam work is essentially part of it's own entry, as
>
> > <!--
> > Author: Peter Eisentraut <peter_e@gmx.net>
> > 2018-08-01 [0d5f05cde] Allow multi-inserts during COPY into a partitioned table
> > -->
> >
> >        <para>
> >         Improve speed of <command>COPY</command> into partitioned tables
> >         (David Rowley)
> >        </para>
> >
> >    since his copy.c portions of 86b85044e823a largely are a rewrite of
> >    the above commit.
> >
>
> OK, David removed.
>
>
>
> > <!--
> > Author: Greg Stark <stark@mit.edu>
> > 2018-10-09 [36e9d413a] Add "B" suffix for bytes to docs
> > -->
> >
> >       <para>
> >        Document that the <literal>B</literal>/bytes units can be specified
> >        for <link linkend="config-setting">server variables</link>
> >        (Greg Stark)
> >       </para>
> >      </listitem>
> >
> > Given how large changes we skip over in the release notes, I don't
> > really see a point in including changes like this. Feels like we'd at
> > the very least also have to include larger changes with typo/grammar
> > fixes etc?
>
> I mentioned it since it was added in a prior release, but was not
> documented, so effectively there was no way for someone to know it was
> possible before, so I thought it made sense to mention it.
>
> I have only corrected a small number of issues above and look for
> guidance to finish the rest.  I will reply to the other emails in this
> thread now.
>
> --
>   Bruce Momjian  <bruce@momjian.us>        http://momjian.us
>   EnterpriseDB                             http://enterprisedb.com
>
> + As you are, so once was I.  As I am, so you will be. +
> +                      Ancient Roman grave inscription +
Greetings,

Andres Freund