Expose the default for channel_binding in PQconndefaults().

If there's a static default value for a connection option,
it should be shown in the PQconninfoOptions array.

Daniele Varrazzo

Discussion: https://postgr.es/m/CA+mi_8Zo8Rgn7p+6ZRY7QdDu+23ukT9AvoHNyPbgKACxwgGhZA@mail.gmail.com

Further fix thinko in plpgsql memory  fix.

There's a second call of get_eval_mcontext() that should also be
get_stmt_mcontext().  This is actually dead code, since no
interesting allocations happen before switching back to the
original context, but we should keep it in sync with the other
call to forestall possible future bugs.

Discussion: https://postgr.es/m/f075f7be-c654-9aa8-3ffc-e9214622f02a@enterprisedb.com

Fix thinko in plpgsql memory  fix.

Commit a6b1f5365 intended to place the transient "target" list of
a CALL statement in the function's statement-lifespan context,
but I fat-fingered that and used get_eval_mcontext() instead of
get_stmt_mcontext().  The eval_mcontext belongs to the "simple
expression" infrastructure, which is destroyed at transaction end.
The net effect is that a CALL in a procedure to another procedure
that has OUT or INOUT parameters would fail if the called procedure
did a COMMIT.

Per report from Peter Eisentraut.  Back- to v11, like the
prior .

Discussion: https://postgr.es/m/f075f7be-c654-9aa8-3ffc-e9214622f02a@enterprisedb.com

Fix inconsistent code with shared invalidations of snapshots

The code in charge of processing a single invalidation message has been
using since 568d413 the structure for relation mapping messages.  This
had fortunately no consequence as both locate the database ID at the
same location, but it could become a problem in the future if this area
of the code changes.

Author: Konstantin Knizhnik
Discussion: https://postgr.es/m/8044c223-4d3a-2cdb-42bf-29940840ce94@postgrespro.ru
Back-through: 9.5

postgres_fdw: Fix connection .

In postgres_fdw, the cached connections to foreign servers will not be
closed until the local session exits if the user mappings or foreign servers
that those connections depend on are dropped. Those connections can be
.

To fix that connection  issue, after a change to a pg_foreign_server
or pg_user_mapping catalog entry, this commit makes postgres_fdw close
the connections depending on that entry immediately if current
transaction has not used those connections yet. Otherwise, mark those
connections as invalid and then close them at the end of current transaction,
since they cannot be closed in the midst of the transaction using them.
Closed connections will be remade at the next opportunity if necessary.

Back- to all supported branches.

Author: Bharath Rupireddy
Reviewed-by: Zhihong Yu, Zhijie Hou, Fujii Masao
Discussion: https://postgr.es/m/CALj2ACVNcGH_6qLY-4_tXz8JLvA+4yeBThRfxMz7Oxbk1aHcpQ@mail.gmail.com

Revert "Add key management system" (978f869b99) & later commits

The  needs test cases, reorganization, and cfbot testing.
Technically reverts commits 5c31afc49d..e35b2bad1a (exclusive/inclusive)
and 08db7c63f3..ccbe34139b.

Reported-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/[email protected]

Second attempt to stabilize 05c02589.

Removing the EXPLAIN test to stabilize the buildfarm. The execution
test should still be effective to catch the bug even if the plan is
slightly different on different platforms.

Stabilize test introduced in 05c02589, per buildfarm.

In passing, make the capitalization match the rest of the file.

Reported-by: Tom Lane

Fix bug #16784 in Disk-based Hash Aggregation.

Before processing tuples, agg_refill_hash_table() was setting all
pergroup pointers to NULL to signal to advance_aggregates() that it
should not attempt to advance groups that had spilled.

The problem was that it also set the pergroups for sorted grouping
sets to NULL, which caused rescanning to fail.

Instead, change agg_refill_hash_table() to only set the pergroups for
hashed grouping sets to NULL; and when compiling the expression, pass
doSort=false.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16784-7ff169bf2c3d1588%40postgresql.org
Back-through: 13

initdb:  document that -K requires an argument

Reported-by: "Shinoda, Noriyoshi"
Discussion: https://postgr.es/m/TU4PR8401MB1152E92B4D44C81E496D6032EEDB0@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM

Author: "Shinoda, Noriyoshi"

Back-through: msater

auth commands:  list specific commands to install in Makefile

Previously I used Makefile functions.

Back-through: master

pg_alterckey:  remove TAP check rules from Makefile

Reported-by: Pavel Stehule, Michael Paquier
Discussion: https://postgr.es/m/CAFj8pRBRNo4co5bqCx4BLx1ZZ45Z_T-opPxA+u7SLp7gAtBpNA@mail.gmail.com

Back-through: master

Add scripts for retrieving the cluster file encryption key

Scripts are passphrase, direct, AWS, and two Yubikey ones.

Back-through: master

doc:  fix SGML markup for pg_alterckey from commit 62afb42a7f

Back-through: master

pg_alterckey: adjust doc build and Win32 sleep/open build fails

Fix for commit 62afb42a7f.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/1252111.1608953815@sss.pgh.pa.us

Back-through: master

Fix function call typo in frontend Win32 code, commit 978f869b99

Reported-by: buildfarm member walleye
Back-through: master

Allow ssl_passphrase_command to prompt the terminal

Previously the command could not access the terminal for a passphrase.

Back-through: master

Add pg_alterckey utility to change the cluster key

This can change the key that encrypts the data encryption keys used for
cluster file encryption.

Discussion: https://postgr.es/m/20201202213814[email protected]

Back-through: master

remove missing reference to crypto test from  978f869b99

Reported-by: Tom Lane
Discussion: https://postgr.es/m/1205031.1608925990@sss.pgh.pa.us

Back-through: master

Really fix the dummy implementations in cipher.c.

945083b2f wasn't enough to silence compiler warnings.

fix no-return function call in cipher.c from commit 978f869b99

Reported-by: buildfarm member sifaka
Back-through: master

fixes docs and missing initdb help option for commit 978f869b99

Reported-by: Erik Rijkers
Discussion: https://postgr.es/m/a27e7bb60fc4c4a1fe960f7b055ba822@xs4all.nl

Back-through: master

Invalidate acl.c caches when pg_authid changes.

This makes existing sessions reflect "ALTER ROLE ... [NO]INHERIT" as
quickly as they have been reflecting "GRANT role_name".  Back- to
9.5 (all supported versions).

Reviewed by Nathan Bossart.

Discussion: https://postgr.es/m/20201221095028[email protected]

remove uint128 requirement from  978f869b99 (CFE)

Used char[16] instead.

Reported-by: buildfarm member florican
Back-through: master

Fix return value and const declaration from commit 978f869b99

This fixes the non-OpenSSL compile case.

Reported-by: buildfarm member sifaka
Back-through: master

Add key management system

This adds a key management system that stores (currently) two data
encryption keys of length 128, 192, or 256 bits.  The data keys are
AES256 encrypted using a key encryption key, and validated via GCM
cipher mode.  A command to obtain the key encryption key must be
specified at initdb time, and will be run at every database server
start.  New parameters allow a file descriptor open to the terminal to
be passed.  pg_upgrade support has also been added.

Discussion: https://postgr.es/m/CA+fd4k7q5o6Nc_AaX6BcYM9yqTbC6_pnH-6nSD=54Zp6NBQTCQ@mail.gmail.com
Discussion: https://postgr.es/m/20201202213814[email protected]

Author: Masahiko Sawada, me, Stephen Frost

Avoid time-of-day-dependent failure in log rotation test.

Buildfarm members pogona and petalura have shown a failure when
pg_ctl/t/004_logrotate.pl starts just before local midnight.
The default rotate-at-midnight behavior occurs just before the
Perl script examines current_logfiles, so it figures that the
rotation it's already requested has occurred ... but in reality,
that rotation happens just after it looks, so the expected new
log data goes into a different file than the one it's examining.

In HEAD, src/test/kerberos/t/001_auth.pl has acquired similar code
that evidently has a related failure mode.  Besides being quite new,
few buildfarm critters run that test, so it's unsurprising that
we've not yet seen a failure there.

Fix both cases by setting log_rotation_age = 0 so that no time-based
rotation can occur.  Also absorb 004_logrotate.pl's decision to
set lc_messages = 'C' into the kerberos test, in hopes that it will
work in non-English prevailing locales.

Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pogona&dt=2020-12-24%2022%3A10%3A04
Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=petalura&dt=2020-02-01%2022%3A20%3A04

revert removal of hex_decode() from ecpg from commit c3826f831e

ecpglib on certain platforms can't handle the pg_log_fatal calls from
libraries.  This was reported by the buildfarm.  It needs a refactoring
and return value change if it is later removed.

Back-through: master

move hex_decode() to /common so it can be called from frontend

This allows removal of a copy of hex_decode() from ecpg, and will be
used by the soon-to-be added pg_alterckey command.

Back-through: master

Fix race condition between shutdown and unstarted background workers.

If a database shutdown (smart or fast) is commanded between the time
some process decides to request a new background worker and the time
that the postmaster can launch that worker, then nothing happens
because the postmaster won't launch any bgworkers once it's exited
PM_RUN state.  This is fine ... unless the requesting process is
waiting for that worker to finish (or even for it to start); in that
case the requestor is stuck, and only manual intervention will get us
to the point of being able to shut down.

To fix, cancel pending requests for workers when the postmaster sends
shutdown (SIGTERM) signals, and similarly cancel any new requests that
arrive after that point.  (We can optimize things slightly by only
doing the cancellation for workers that have waiters.)  To fit within
the existing bgworker APIs, the "cancel" is made to look like the
worker was started and immediately stopped, causing deregistration of
the bgworker entry.  Waiting processes would have to deal with
premature worker exit anyway, so this should introduce no bugs that
weren't there before.  We do have a side effect that registration
records for restartable bgworkers might disappear when theoretically
they should have remained in place; but since we're shutting down,
that shouldn't matter.

Back- to v10.  There might be value in putting this into 9.6
as well, but the management of bgworkers is a bit different there
(notably see 8ff518699) and I'm not convinced it's worth the effort
to validate the  for that branch.

Discussion: https://postgr.es/m/661570.1608673226@sss.pgh.pa.us

Improve client error messages for immediate-stop situations.

Up to now, if the DBA issued "pg_ctl stop -m immediate", the message
sent to clients was the same as for a crash-and-restart situation.
This is confusing, not least because the message claims that the
database will soon be up again, something we have no business
predicting.

Improve things so that we can generate distinct messages for the two
cases (and also recognize an ad-hoc SIGQUIT, should somebody try that).
To do that, add a field to pmsignal.c's shared memory data structure
that the postmaster sets just before broadcasting SIGQUIT to its
children.  No interlocking seems to be necessary; the intervening
signal-sending and signal-receipt should sufficiently serialize accesses
to the field.  Hence, this isn't any riskier than the existing usages
of pmsignal.c.

We might in future extend this idea to improve other
postmaster-to-children signal scenarios, although none of them
currently seem to be as badly overloaded as SIGQUIT.

Discussion: https://postgr.es/m/559291.1608587013@sss.pgh.pa.us

Fix typos and grammar in docs and comments

This fixes several areas of the documentation and some comments in
matters of style, grammar, or even format.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20201222041153[email protected]

dummy commit

Fix portability issues with parsing of recovery_target_xid

The parsing of this parameter has been using strtoul(), which is not
portable across platforms.  On most Unix platforms, unsigned long has a
size of 64 bits, while on Windows it is 32 bits.  It is common in
recovery scenarios to rely on the output of txid_current() or even the
newer pg_current_xact_id() to get a transaction ID for setting up
recovery_target_xid.  The value returned by those functions includes the
epoch in the computed result, which would cause strtoul() to fail where
unsigned long has a size of 32 bits once the epoch is incremented.

WAL records and 2PC data include only information about 32-bit XIDs and
it is not possible to have XIDs across more than one epoch, so
discarding the high bits from the transaction ID set has no impact on
recovery.  On the contrary, the use of strtoul() prevents a consistent
behavior across platforms depending on the size of unsigned long.

This commit changes the parsing of recovery_target_xid to use
pg_strtouint64() instead, available down to 9.6.  There is one TAP test
stressing recovery with recovery_target_xid, where a tweak based on
pg_reset{xlog,wal} is added to bump the XID epoch so as this change gets
tested, as per an idea from Alexander Lakhin.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16780-107fd0c0385b1035@postgresql.org
Back-through: 9.6

Improve autoprewarm's handling of early-shutdown scenarios.

Bad things happen if the DBA issues "pg_ctl stop -m fast" before
autoprewarm finishes loading its list of blocks to prewarm.
The current worker process successfully terminates early, but
(if this wasn't the last database with blocks to prewarm) the
leader process will just try to launch another worker for the
next database.  Since the postmaster is now in PM_WAIT_BACKENDS
state, it ignores the launch request, and the leader just sits
until it's killed manually.

This is mostly the fault of our half-baked design for launching
background workers, but a proper fix for that is likely to be
too invasive to be back-able.  To ameliorate the situation,
fix apw_load_buffers() to check whether SIGTERM has arrived
just before trying to launch another worker.  That leaves us with
only a very narrow window in each worker launch where SIGTERM
could occur between the launch request and successful worker start.

Another issue is that if the leader process does manage to exit,
it unconditionally rewrites autoprewarm.blocks with only the
blocks currently in shared buffers, thus forgetting any blocks
that we hadn't reached yet while prewarming.  This seems quite
unhelpful, since the next database start will then not have the
expected prewarming benefit.  Fix it to not modify the file if
we shut down before the initial load attempt is complete.

Per bug #16785 from John Thompson.  Back- to v11 where
the autoprewarm code was introduced.

Discussion: https://postgr.es/m/16785-c0207d8c67fb5f25@postgresql.org

Increase timeout in 021_row_visibility.pl.

Commit 7b28913bc figured 30 seconds is long enough for anybody,
but in contexts like valgrind runs, it isn't necessarily.

Improve find_em_expr_usable_for_sorting_rel comment

Clarify the relationship between find_em_expr_usable_for_sorting_rel and
prepare_sort_from_pathkeys, i.e. what restrictions need to be shared
between those two places.

Author: James Coleman
Reviewed-by: Tomas Vondra
Back-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com

Don't search for volatile expr in find_em_expr_usable_for_sorting_rel

While prepare_sort_from_pathkeys has to be concerned about matching up
a volatile expression to the proper tlist entry, we don't need to do
that in find_em_expr_usable_for_sorting_rel becausee such a sort will
have to be postponed anyway.

Author: James Coleman
Reviewed-by: Tomas Vondra
Back-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com

Disallow SRFs when considering sorts below Gather Merge

While we do allow SRFs in ORDER BY, scan/join processing should not
consider such cases - such sorts should only happen via final Sort atop
a ProjectSet. So make sure we don't try adding such sorts below Gather
Merge, just like we do for expressions that are volatile and/or not
parallel safe.

Back to PostgreSQL 13, where this code was introduced as part of
the Incremental Sort .

Author: James Coleman
Reviewed-by: Tomas Vondra
Back-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
Discussion: https://postgr.es/m/295524.1606246314%40sss.pgh.pa.us

Remove "invalid concatenation of jsonb objects" error case.

The jsonb || jsonb operator arbitrarily rejected certain combinations
of scalar and non-scalar inputs, while being willing to concatenate
other combinations.  This was of course quite undocumented.  Rather
than trying to document it, let's just remove the restriction,
creating a uniform rule that unless we are handling an object-to-object
concatenation, non-array inputs are converted to one-element arrays,
resulting in an array-to-array concatenation.  (This does not change
the behavior for any case that didn't throw an error before.)

Per complaint from Joel Jacobson.  Back- to all supported branches.

Discussion: https://postgr.es/m/163099.1608312033@sss.pgh.pa.us

Check parallel safety in generate_useful_gather_paths

Commit ebb7ae839d ensured we ignore pathkeys with volatile expressions
when considering adding a sort below a Gather Merge. Turns out we need
to care about parallel safety of the pathkeys too, otherwise we might
try sorting e.g. on results of a correlated subquery (as demonstrated
by a report from Luis Roberto).

Initial investigation by Tom Lane,  by James Coleman. Back
to 13, where the code was instroduced (as part of Incremental Sort).

Reported-by: Luis Roberto
Author: James Coleman
Reviewed-by: Tomas Vondra
Back-through: 13
Discussion: https://postgr.es/m/622580997.37108180.1604080457319.JavaMail.zimbra%40siscobra.com.br
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com

Consider unsorted paths in generate_useful_gather_paths

generate_useful_gather_paths used to skip unsorted paths (without any
pathkeys), but that is unnecessary - the later code actually can handle
such paths just fine by adding a Sort node. This is clearly a thinko,
preventing construction of useful plans.

Back to 13, where Incremental Sort was introduced.

Author: James Coleman
Reviewed-by: Tomas Vondra
Back-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com

Fix compiler warning in multirange_constructor0()

Discussion: https://postgr.es/m/X%2BBP8XE0UpIB6Yvh%40paquier.xyz
Author: Michael Paquier

Refactor logic to check for ASCII-only characters in string

The same logic was present for collation commands, SASLprep and
pgcrypto, so this removes some code.

Author: Michael Paquier
Reviewed-by: Stephen Frost, Heikki Linnakangas
Discussion: https://postgr.es/m/[email protected]

Fix typalign in rangetypes statistics

6df7a9698b introduces multirange types, whose typanalyze function shares
infrastructure with range types typanalyze function.  Since 6df7a9698b,
information about type gathered by statistics is filled from typcache.
But typalign is mistakenly always set to double.  This commit fixes this
oversight.

Doc: fix description of how to use src/tutorial files.

The separate "cd" command before invoking psql made sense (or at least
I thought so) when it was added in commit ed1939332.  But 4e3a61635
removed the supporting text that explained when to use it, making it
just confusing.  So drop it.

Also switch from four-dot to three-dot filler for the unsupplied
part of the path, since at least one person has read the four-dot
filler as a typo for "../..".  And fix these/those inconsistency.

Discussion: https://postgr.es/m/160837647714.673.5195186835607800484@wrigleys.postgresql.org

Doc: improve description of pgbench script weights.

Point out the workaround to be used if you want to write a script
file name that includes "@".  Clean up the text a little.

Fabien Coelho, additional wordsmithing by me

Discussion: https://postgr.es/m/1c4e81550d214741827a03292222db8d@G08CNEXMBPEKD06.g08.fujitsu.local

Avoid memcpy() with same source and destination in pgstat_recv_replslot.

Same type of issue as in commit 53d4f5fef and earlier fixes; also
found by apparently-more-picky-than-the-buildfarm valgrind testing.
This one is an oversight in commit 986816750.  Since that's new in
HEAD, no need for a back-.

Fix compiler warning introduced in 6df7a9698b

Fixes for pg_dump.c regarding multiranges

This commit fixes two wrong version number checks and one wrong check for null.

Multirange datatypes

Multiranges are basically sorted arrays of non-overlapping ranges with
set-theoretic operations defined over them.

Since v14, each range type automatically gets a corresponding multirange
datatype.  There are both manual and automatic mechanisms for naming multirange
types.  Once can specify multirange type name using multirange_type_name
attribute in CREATE TYPE.  Otherwise, a multirange type name is generated
automatically.  If the range type name contains "range" then we change that to
"multirange".  Otherwise, we add "_multirange" to the end.

Implementation of multiranges comes with a space-efficient internal
representation format, which evades extra paddings and duplicated storage of
oids.  Altogether this format allows fetching a particular range by its index
in O(n).

Statistic gathering and selectivity estimation are implemented for multiranges.
For this purpose, stored multirange is approximated as union range without gaps.
This field will likely need improvements in the future.

Catversion is bumped.

Discussion: https://postgr.es/m/CALNJ-vSUpQ_Y%3DjXvTxt1VYFztaBSsWVXeF1y6gTYQ4bOiWDLgQ%40mail.gmail.com
Discussion: https://postgr.es/m/a0b8026459d1e6167933be2104a6174e7d40d0ab.camel%40j-davis.com#fe7218c83b08068bfffb0c5293eceda0
Author: Paul Jungwirth, revised by me
Reviewed-by: David Fetter, Corey Huinker, Jeff Davis, Pavel Stehule
Reviewed-by: Alvaro Herrera, Tom Lane, Isaac Morland, David G. Johnston
Reviewed-by: Zhihong Yu, Alexander Korotkov

Remove now-useless ALWAYS_SUBDIRS entry in src/test/Makefile.

Commit 257836a75 added the "locale" subdirectory to SUBDIRS,
but neglected to remove it from ALWAYS_SUBDIRS.  This oversight
had no functional effect because the filter-out function would
remove it anyway.  Still, it's confusing to readers to list a
subdirectory in both places, especially because it makes the
associated comment into a partial lie.

Update comment atop of ReorderBufferQueueMessage().

The comments atop of this function describes behaviour in case of a
transactional WAL message only, but it accepts both transactional and
non-transactional WAL messages. Update the comments to describe
behaviour in case of non-transactional WAL message as well.

Ashutosh Bapat, rephrased by Amit Kapila
Discussion: https://postgr.es/m/CAGEoWWTTzNzHOi8bj0wfAo1siGi-YEh6wqH1oaz4DrkTJ6HbTQ@mail.gmail.com

Add a couple of missed .gitignore entries.

Any subdirectory that's ignoring /output_iso/ should also
ignore /tmp_check_iso/, which could be left behind by a
failed pg_isolation_regress_check run.

I think these have been wrong for awhile, but it doesn't
seem important to fix in back branches.

Avoid memcpy() with same source and destination during relmapper init.

A narrow reading of the C standard says that memcpy(x,x,n) is undefined,
although it's hard to envision an implementation that would really
misbehave.  However, analysis tools such as valgrind might whine about
this; accordingly, let's band-aid relmapper.c to not do it.

See also 5b630501e, d3f4e8a8a, ad7b48ea0, and other similar fixes.
Apparently, none of those folk tried valgrinding initdb?  This has been
like this for long enough that I'm surprised it hasn't been reported
before.

Back-, just in case anybody wants to use a back branch on a platform
that complains about this; we back-ed those earlier fixes too.

Discussion: https://postgr.es/m/161790.1608310142@sss.pgh.pa.us

pg_stat_statements: Track time at which all statistics were last reset.

This commit adds "stats_reset" column into the pg_stat_statements_info
view. This column indicates the time at which all statistics in the
pg_stat_statements view were last reset.

Per discussion, this commit also changes pg_stat_statements_info code
so that "dealloc" column is reset at the same time as "stats_reset" is reset,
i.e., whenever all pg_stat_statements entries are removed, for the sake
of consistency. Previously "dealloc" was reset only when
pg_stat_statements_reset(0, 0, 0) is called and was not reset when
pg_stat_statements_reset() with non-zero value argument discards all
entries. This was confusing.

Author: Naoki Nakamichi, Yuki Seino
Reviewed-by: Yuki Seino, Kyotaro Horiguchi, Li Japin, Fujii Masao
Discussion: https://postgr.es/m/c102cf3180d0ee73c1c5a0f7f8558322@oss.nttdata.com

Revert "Get rid of the dedicated latch for signaling the startup process".

Revert ac22929a26, as well as the followup fix 113d3591b8. Because it broke
the assumption that the startup process waiting for the recovery conflict
on buffer pin should be waken up only by buffer unpin or the timeout enabled
in ResolveRecoveryConflictWithBufferPin(). It caused, for example,
SIGHUP signal handler or walreceiver process to wake that startup process
up unnecessarily frequently.

Additionally, add the comments about why that dedicated latch that
the reverted  tried to get rid of should not be removed.

Thanks to Kyotaro Horiguchi for the discussion.

Author: Fujii Masao
Discussion: https://postgr.es/m/d8c0c608-021b-3c73-fffd-3240829ee986@oss.nttdata.com

Fix varchar_2.out to match reality in cs_CZ locale.

Seems to be a copy-and-pasteo in c06d6aa4c.  Per buildfarm.

doc: Fix explanation related to pg_shmem_allocations

Offsets are shown as NULL only for anonymous allocations.

Author: Benoit Lobréau
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CAPE8EZ5Lnoyqoz7aZpvQM0E8sW+hw+k6G2NULe+m4arFRrA1aA@mail.gmail.com
Back-through: 13

doc:  clarify COPY TO for partitioning/inheritance

It was not clear how COPY TO behaved with partitioning/inheritance
because the paragraphs were so far apart.  Also reword to simplify.

Discussion: https://postgr.es/m/20201203211723[email protected]

Author: Justin Pryzby

Back-through: 10

Remove obsolete btrescan() comment.

"Ordering stuff" refered to a _bt_first() call to _bt_orderkeys().
However, the _bt_orderkeys() function was renamed to
_bt_preprocess_keys() by commit fa5c8a055a0.

_bt_preprocess_keys() is directly referenced just after the removed
comment already, which seems sufficient.

Remove useless variable stores

Mistakenly introduced in 4cbe3ac3e867; bug repaired in 148e632c0541 but
the stores were accidentally.

Error out when Gather Merge input is not sorted

To build Gather Merge path, the input needs to be sufficiently sorted.
Ensuring this is the responsibility of the code constructing the paths,
but create_gather_merge_plan tried to handle unsorted paths by adding
an explicit Sort. In light of the recent issues related to Incremental
Sort, this is rather fragile. Some of the expressions may be volatile
or parallel unsafe, in which case we can't add the Sort here.

We could do more checks and add the Sort in at least some cases, but
it seems cleaner to just error out and make it clear this is a bug in
code constructing those paths.

Author: James Coleman
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
Discussion: https://postgr.es/m/CAJGNTeNaxpXgBVcRhJX%2B2vSbq%2BF2kJqGBcvompmpvXb7pq%2BoFA%40mail.gmail.com

Clean up ancient test style

Many older tests where written in a style like

    SELECT '' AS two, i.* FROM INT2_TBL

where the first column indicated the number of expected result rows.
This has gotten increasingly out of date, as the test data fixtures
have expanded, so a lot of these were wrong and misleading.  Moreover,
this style isn't really necessary, since the psql output already shows
the number of result rows.

To clean this up, remove all those extra columns.

Discussion: https://www.postgresql.org/message-id/flat/1a25312b-2686-380d-3c67-7a69094a999f%40enterprisedb.com

Improve hash_create()'s API for some added robustness.

Invent a new flag bit HASH_STRINGS to specify C-string hashing, which
was formerly the default; and add assertions insisting that exactly
one of the bits HASH_STRINGS, HASH_BLOBS, and HASH_FUNCTION be set.
This is in hopes of preventing recurrences of the type of oversight
fixed in commit a1b8aa1e4 (i.e., mistakenly omitting HASH_BLOBS).

Also, when HASH_STRINGS is specified, insist that the keysize be
more than 8 bytes.  This is a heuristic, but it should catch
accidental use of HASH_STRINGS for integer or pointer keys.
(Nearly all existing use-cases set the keysize to NAMEDATALEN or
more, so there's little reason to think this restriction should
be problematic.)

Tweak hash_create() to insist that the HASH_ELEM flag be set, and
remove the defaults it had for keysize and entrysize.  Since those
defaults were undocumented and basically useless, no callers
omitted HASH_ELEM anyway.

Also, remove memset's zeroing the HASHCTL parameter struct from
those callers that had one.  This has never been really necessary,
and while it wasn't a bad coding convention it was confusing that
some callers did it and some did not.  We might as well save a few
cycles by standardizing on "not".

Also improve the documentation for hash_create().

In passing, improve reinit.c's usage of a hash table by storing
the key as a binary Oid rather than a string; and, since that's
a temporary hash table, allocate it in CurrentMemoryContext for
neatness.

Discussion: https://postgr.es/m/590625.1607878171@sss.pgh.pa.us

Revert "Cannot use WL_SOCKET_WRITEABLE without WL_SOCKET_READABLE."

This reverts commit 3a9e64aa0d96c8ffb6c682b082d0f72b1d373327.

Commit 4bad60e3 fixed the root of the problem that 3a9e64aa worked
around.

This enables proper pipelining of commands after terminating
replication, eliminating an undocumented limitation.

Discussion: https://postgr.es/m/3d57bc29-4459-578b-79cb-7641baf53c57%40iki.fi
Back-through: 9.5

Improve some code around cryptohash functions

This adjusts some code related to recent changes for cryptohash
functions:
- Add a variable in md5.h to track down the size of a computed result,
moved from pgcrypto.  Note that pg_md5_hash() assumed a result of this
size already.
- Call explicit_bzero() on the hashed data when freeing the context for
fallback implementations.  For MD5, particularly, it would be annoying
to leave some non-zeroed data around.
- Clean up some code related to recent changes of uuid-ossp.  .gitignore
still included md5.c and a comment was incorrect.

Discussion: https://postgr.es/m/[email protected]

Add some checkpoint/restartpoint status to ps display

This is done for end-of-recovery and shutdown checkpoints/restartpoints
(end-of-recovery restartpoints don't exist) rather than all types of
checkpoints, in cases where it may not be possible to rely on
pg_stat_activity to get a status from the startup or checkpointer
processes.

For example, at the end of a crash recovery, this is useful to know if a
checkpoint is running in the startup process, while previously the ps
display may only show some information about "recovering" something,
that can be confusing while a checkpoint runs.

Author: Justin Pryzby
Reviewed-by: Nathan Bossart, Kirk Jamison, Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/20200818225238[email protected]

Use HASH_BLOBS for xidhash.

This caused BufFile errors on buildfarm member sungazer, and SIGSEGV was
possible.  Conditions for reaching those symptoms were more frequent on
big-endian systems.

Discussion: https://postgr.es/m/20201129214441[email protected]

Correct behavior descriptions in comments, and correct a test name.

initdb:  complete getopt_long alphabetization

Back-through: 9.5

initdb:  properly alphabetize getopt_long options in C string

Back-through: 9.5

Allow subscripting of hstore values.

This is basically a finger exercise to prove that it's possible for
an extension module to add subscripting ability.  Subscripted fetch
from an hstore is not different from the existing "hstore -> text"
operator.  Subscripted update does seem to be a little easier to
use than the traditional update method using hstore concatenation,
but it's not a fundamentally new ability.

However, there may be some value in the code as sample code, since
it shows what's basically the minimum-complexity way to implement
subscripting when one needn't consider nested container objects.

Discussion: https://postgr.es/m/3724341.1607551174@sss.pgh.pa.us

Allow ALTER TYPE to update an existing type's typsubscript value.

This is essential if we'd like to allow existing extension data types
to support subscripting in future, since dropping and recreating the
type isn't a practical thing for an extension upgrade script, and
direct manipulation of pg_type isn't a great answer either.

There was some discussion about also allowing alteration of typelem,
but it's less clear whether that's a good idea or not, so for now
I forebore.

Discussion: https://postgr.es/m/3724341.1607551174@sss.pgh.pa.us

Provide an error cursor for "can't subscript" error messages.

Commit c7aba7c14 didn't add this, but after more fooling with the
feature I feel that it'd be useful.  To make this possible, refactor
getSubscriptingRoutines() so that the caller is responsible for
throwing any error.  (In clauses.c, I just chose to make the
most conservative assumption rather than throwing an error.  We don't
expect failures there anyway really, so the code space for an error
message would be a poor investment.)

pg_dump: Don't use enums for defining bit mask values

This usage would mean that values of the enum type are potentially not
one of the enum values.  Use macros instead, like everywhere else.

Discussion: https://www.postgresql.org/message-id/14dde730-1d34-260e-fa9d-7664df2d6313@enterprisedb.com

Fix compilation of uuid-ossp

This module had a dependency on pgcrypto's md5.c that got removed by
b67b57a.  Instead of the code from pgcrypto, this code can just use the
new cryptohash routines for MD5 as a drop-in replacement, so let's just
do this switch.  This has also the merit to simplify a bit the
compilation of uuid-ossp.

This requires --with-uuid to be reproduced, and I have used e2fs as a
way to reproduce the failure, then test this commit.

Per reports from buildfarm members longfin, florican and sifaka.

Discussion: https://postgr.es/m/[email protected]

Refactor MD5 implementations according to new cryptohash infrastructure

This commit heavily reorganizes the MD5 implementations that exist in
the tree in various aspects.

First, MD5 is added to the list of options available in cryptohash.c and
cryptohash_openssl.c.  This means that if building with OpenSSL, EVP is
used for MD5 instead of the fallback implementation that Postgres had
for ages.  With the recent refactoring work for cryptohash functions,
this change is straight-forward.  If not building with OpenSSL, a
fallback implementation internal to src/common/ is used.

Second, this reduces the number of MD5 implementations present in the
tree from two to one, by moving the KAME implementation from pgcrypto to
src/common/, and by removing the implementation that existed in
src/common/.  KAME was already structured with an init/update/final set
of routines by pgcrypto (see original pgcrypto/md5.h) for compatibility
with OpenSSL, so moving it to src/common/ has proved to be a
straight-forward move, requiring no actual manipulation of the internals
of each routine.  Some benchmarking has not shown any performance gap
between both implementations.

Similarly to the fallback implementation used for SHA2, the fallback
implementation of MD5 is moved to src/common/md5.c with an internal
header called md5_int.h for the init, update and final routines.  This
gets then consumed by cryptohash.c.

The original routines used for MD5-hashed passwords are moved to a
separate file called md5_common.c, also in src/common/, aimed at being
shared between all MD5 implementations as utility routines to keep
compatibility with any code relying on them.

Like the SHA2 changes, this commit had its round of tests on both Linux
and Windows, across all versions of OpenSSL supported on HEAD, with and
even without OpenSSL.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20201106073434[email protected]

Support subscripting of arbitrary types, not only arrays.

This  generalizes the subscripting infrastructure so that any
data type can be subscripted, if it provides a handler function to
define what that means.  Traditional variable-length (varlena) arrays
all use array_subscript_handler(), while the existing fixed-length
types that support subscripting use raw_array_subscript_handler().
It's expected that other types that want to use subscripting notation
will define their own handlers.  (This  provides no such new
features, though; it only lays the foundation for them.)

To do this, move the parser's semantic processing of subscripts
(including coercion to whatever data type is required) into a
method callback supplied by the handler.  On the execution side,
replace the ExecEvalSubscriptingRef* layer of functions with direct
calls to callback-supplied execution routines.  (Thus, essentially
no new run-time overhead should be caused by this .  Indeed,
there is room to remove some overhead by supplying specialized
execution routines.  This  does a little bit in that line,
but more could be done.)

Additional work is required here and there to remove formerly
hard-wired assumptions about the result type, collation, etc
of a SubscriptingRef expression node; and to remove assumptions
that the subscript values must be integers.

One useful side-effect of this is that we now have a less squishy
mechanism for identifying whether a data type is a "true" array:
instead of wiring in weird rules about typlen, we can look to see
if pg_type.typsubscript == F_ARRAY_SUBSCRIPT_HANDLER.  For this
to be bulletproof, we have to forbid user-defined types from using
that handler directly; but there seems no good reason for them to
do so.

This  also removes assumptions that the number of subscripts
is limited to MAXDIM (6), or indeed has any hard-wired limit.
That limit still applies to types handled by array_subscript_handler
or raw_array_subscript_handler, but to discourage other dependencies
on this constant, I've moved it from c.h to utils/array.h.

Dmitry Dolgov, reviewed at various times by Tom Lane, Arthur Zakirov,
Peter Eisentraut, Pavel Stehule

Discussion: https://postgr.es/m/CA+q6zcVDuGBv=M0FqBYX8DPebS3F_0KQ6OVFobGJPM507_SZ_w@mail.gmail.com
Discussion: https://postgr.es/m/CA+q6zcVovR+XY4mfk-7oNk-rF91gH0PebnNfuUjuuDsyHjOcVA@mail.gmail.com

Change get_constraint_index() to use pg_constraint.conindid

It was still using a scan of pg_depend instead of using the conindid
column that has been added since.

Since it is now just a catalog lookup wrapper and not related to
pg_depend, move from pg_depend.c to lsyscache.c.

Reviewed-by: Matthias van de Meent <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4688d55c-9a2e-9a5a-d166-5f24fe0bf8db%40enterprisedb.com

Simplify code for getting a unicode codepoint's canonical class.

Three places of unicode_norm.c use a similar logic for getting the
combining class from a codepoint.  Commit 2991ac5 has added the function
get_canonical_class() for this purpose, but it was only called by the
backend.  This commit refactors the code to use this function in all
the places where the combining class is retrieved from a given
codepoint.

Author: John Naylor
Discussion: https://postgr.es/m/CAFBsxsHUV7s7YrOm6hFz-Jq8Sc7K_yxTkfNZxsDV-DuM-k-gwg@mail.gmail.com

jit: Reference function pointer types via llvmjit_types.c.

It is error prone (see 5da871bfa1b) and verbose to manually create function
types. Add a helper that can reference a function pointer type via
llvmjit_types.c and and convert existing instances of manual creation.

Author: Andres Freund <[email protected]>
Reviewed-By: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/20201207212142[email protected]

Teach contain__vars that assignment SubscriptingRefs are y.

array_get_element and array_get_slice qualify as proof, since
they will silently return NULL for bogus subscripts.  But
array_set_element and array_set_slice throw errors for such cases,
making them clearly not proof.  contain__vars was evidently
written with only the former case in mind, as it gave the wrong answer
for assignment SubscriptingRefs (nee ArrayRefs).

This would be a live security bug, were it not that assignment
SubscriptingRefs can only occur in INSERT and UPDATE target lists,
while we only care about proofness for qual expressions; so the
wrong answer can't occur in practice.  Still, that's a rather shaky
answer for a security-related question; and maybe in future somebody
will want to ask about proofness of a tlist.  So it seems wise to
fix and even back- this correction.

(We would need some change here anyway for the upcoming
generic-subscripting , since extensions might make different
tradeoffs about whether to throw errors.  Commit 558d77f20 attempted
to lay groundwork for that by asking check_functions_in_node whether a
SubscriptingRef contains y functions; but that idea fails now that
the implementation methods of a SubscriptingRef are not SQL-visible
functions that could be marked proof or not.)

Back- to 9.6.  While 9.5 has the same issue, the code's a bit
different.  It seems quite unlikely that we'd introduce any actual bug
in the short time 9.5 has left to live, so the work/risk/reward balance
isn't attractive for changing 9.5.

Discussion: https://postgr.es/m/3143742.1607368115@sss.pgh.pa.us

Remove operator_precedence_warning.

This GUC was always intended as a temporary solution to help with
finding 9.4-to-9.5 migration issues.  Now that all pre-9.5 branches
are out of support, and 9.5 will be too before v14 is released,
it seems like it's okay to drop it.  Doing so allows removal of
several hundred lines of poorly-tested code in parse_expr.c,
which have been a fertile source of bugs when people did use this.

Discussion: https://postgr.es/m/2234320.1607117945@sss.pgh.pa.us

Improve estimation of ANDs under ORs using extended statistics.

Formerly, extended statistics only handled clauses that were
RestrictInfos. However, the restrictinfo machinery doesn't create
sub-AND RestrictInfos for AND clauses underneath OR clauses.
Therefore teach extended statistics to handle bare AND clauses,
looking for compatible RestrictInfo clauses underneath them.

Dean Rasheed, reviewed by Tomas Vondra.

Discussion: https://postgr.es/m/CAEZATCW=J65GUFm50RcPv-iASnS2mTXQbr=CfBvWRVhFLJ_fWA@mail.gmail.com

Improve estimation of OR clauses using multiple extended statistics.

When estimating an OR clause using multiple extended statistics
objects, treat the estimates for each set of clauses for each
statistics object as independent of one another. The overlap estimates
produced for each statistics object do not apply to clauses covered by
other statistics objects.

Dean Rasheed, reviewed by Tomas Vondra.

Discussion: https://postgr.es/m/CAEZATCW=J65GUFm50RcPv-iASnS2mTXQbr=CfBvWRVhFLJ_fWA@mail.gmail.com

Doc: clarify that CREATE TABLE discards redundant unique constraints.

The SQL standard says that redundant unique constraints are disallowed,
but we long ago decided that throwing an error would be too
user-unfriendly, so we just drop redundant ones.  The docs weren't very
clear about that though, as this behavior was only explained for PRIMARY
KEY vs UNIQUE, not UNIQUE vs UNIQUE.

While here, I couldn't resist doing some copy-editing and markup-fixing
on the adjacent text about INCLUDE options.

Per bug #16767 from Matthias vd Meent.

Discussion: https://postgr.es/m/16767-1714a2056ca516d0@postgresql.org

Doc: explain that the string types can't store \0 (ASCII NUL).

This restriction was mentioned in connection with string literals,
but it wasn't made clear that it's a general restriction not just
a syntactic limitation in query strings.

Per unsigned documentation comment.

Discussion: https://postgr.es/m/160720552914.710.16625261471128631268@wrigleys.postgresql.org

Speed up rechecking if relation needs to be vacuumed or analyze in autovacuum.

After autovacuum collects the relations to vacuum or analyze, it rechecks
whether each relation still needs to be vacuumed or analyzed before actually
doing that. Previously this recheck could be a significant overhead
especially when there were a very large number of relations. This was
because each recheck forced the statistics to be refreshed, and the refresh
of the statistics for a very large number of relations could cause heavy
overhead. There was the report that this issue caused autovacuum workers
to have gotten “stuck” in a tight loop of table_recheck_autovac() that
rechecks whether a relation needs to be vacuumed or analyzed.

This commit speeds up the recheck by making autovacuum worker reuse
the previously-read statistics for the recheck if possible. Then if that
"stale" statistics says that a relation still needs to be vacuumed or analyzed,
autovacuum refreshes the statistics and does the recheck again.

The benchmark shows that the more relations exist and autovacuum workers
are running concurrently, the more this change reduces the autovacuum
execution time. For example, when there are 20,000 tables and 10 autovacuum
workers are running, the benchmark showed that the change improved
the performance of autovacuum more than three times. On the other hand,
even when there are only 1000 tables and only a single autovacuum worker
is running, the benchmark didn't show any big performance regression by
the change.

Firstly POC  was proposed by Jim Nasby. As the result of discussion,
we used Tatsuhito Kasahara's version of the  using the approach
suggested by Tom Lane.

Reported-by: Jim Nasby
Author: Tatsuhito Kasahara
Reviewed-by: Masahiko Sawada, Fujii Masao
Discussion: https://postgr.es/m/3FC6C2F2-8A47-44C0-B997-28830B5716D0@amazon.com

Bump catversion for pg_stat_wal changes.

Oversight in 01469241b2.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20201207185614[email protected]

pgcrypto: Detect errors with EVP calls from OpenSSL

The following routines are called within pgcrypto when handling digests
but there were no checks for failures:
- EVP_MD_CTX_size (can fail with -1 as of 3.0.0)
- EVP_MD_CTX_block_size (can fail with -1 as of 3.0.0)
- EVP_DigestInit_ex
- EVP_DigestUpdate
- EVP_DigestFinal_ex

A set of elog(ERROR) is added by this commit to detect such failures,
that should never happen except in the event of a processing failure
internal to OpenSSL.

Note that it would be possible to use ERR_reason_error_string() to get
more context about such errors, but these refer mainly to the internals
of OpenSSL, so it is not really obvious how useful that would be.  This
is left out for simplicity.

Per report from Coverity.  Thanks to Tom Lane for the discussion.

Back-through: 9.5

jit: Correct parameter type for generated expression evaluation functions.

clang only uses the 'i1' type for scalar booleans, not for pointers to
booleans (as the pointer might be pointing into a larger memory
allocation). Therefore a pointer-to-bool needs to the "storage" boolean.

There's no known case of wrong code generation due to this, but it seems quite
possible that it could cause problems (see e.g. 72559438f92).

Author: Andres Freund
Discussion: https://postgr.es/m/20201207212142[email protected]
Back: 11-, where jit support was added

jit: configure: Explicitly reference 'native' component.

Until recently 'native' was implicitly included via 'orcjit', but a change
included in LLVM 11 (not yet released) removed a number of such indirect
component references.

Reported-By: Fabien COELHO <[email protected]>
Reported-By: Andres Freund <[email protected]>
Reported-By: Thomas Munro <[email protected]>
Author: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/20201201064949[email protected]
Back: 11-, where jit support was added

Avoid using tuple from syscache for update of pg_database.datfrozenxid

pg_database.datfrozenxid gets updated using an in-place update at the
end of vacuum or autovacuum.  Since 96cdeae, as pg_database has a toast
relation, it is possible for a pg_database tuple to have toast values
if there is a large set of ACLs in place.  In such a case, the in-place
update would fail because of the flattening of the toast values done for
the catcache entry fetched.  Instead of using a copy from the catcache,
this changes the logic to fetch the copy of the tuple by directly
scanning pg_database.

Per the lack of complaints on the matter, no back is done.  Note
that before 96cdeae, attempting to insert such a tuple to pg_database
would cause a "row is too big" error, so the end-of-vacuum problem was
not reachable.

Author: Ashwin Agrawal, Junfeng Yang
Discussion: https://postgr.es/m/DM5PR0501MB38800D9E4605BCA72DD35557CCE10@DM5PR0501MB3880.namprd05.prod.outlook.com

Add a couple of regression test cases related to array subscripting.

Exercise some error cases that were never reached in the existing
regression tests.  This is partly for code-coverage reasons, and
partly to memorialize the current behavior in advance of planned
changes for generic subscripting.

Also, I noticed that type_sanity's check to verify that all standard
types have array types was never extended when we added arrays for
all system catalog rowtypes (f7f70d5e2), nor when we added arrays
over domain types (c12d570fa).  So do that.  Also, since the query's
expected output isn't empty, it seems like a good idea to add an
ORDER BY to make sure the result stays stable.

Fix more race conditions in the newly-added pg_rewind test.

pg_rewind looks at the control file to check what timeline a server is on.
But promotion doesn't immediately write a checkpoint, it merely writes
an end-of-recovery WAL record. If pg_rewind runs immediately after
promotion, before the checkpoint has completed, it will think think that
the server is still on the earlier timeline. We ran into this issue a long
time ago already, see commit 484a848a73f.

It's a bit bogus that pg_rewind doesn't determine the timeline correctly
until the end-of-recovery checkpoint has completed. We probably should
fix that. But for now work around it by waiting for the checkpoint
to complete before running pg_rewind, like we did in commit 484a848a73f.

In the passing, tidy up the new test a little bit. Rerder the INSERTs so
that the comments make more sense, remove a spurious CHECKPOINT call after
pg_rewind has already run, and add --debug option, so that if this fails
again, we'll have more data.

Per buildfarm failure at https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=rorqual&dt=2020-12-06%2018%3A32%3A19&stg=pg_rewind-check.
Back to all supported versions.

Discussion: https://www.postgresql.org/message-id/1713707e-e318-761c-d287-5b6a4aa807e8@iki.fi

pg_dump: Reorganize dumpBaseType()

Along the same lines as ed2c7f65b and daa9fe8a5, reduce code duplication
by having just one copy of the parts of the query that are the same
across all server versions; and make the conditionals control the
smallest possible amount of code.  This is in preparation for adding
another dumpable field to pg_type.

Fix fd  in pg_verifybackup

An error code path newly-introduced by 87ae969 forgot to close a file
descriptor when verifying a file's checksum.

Per report from Coverity, via Tom Lane.

Fix missed step in removal of useless RESULT RTEs in the planner.

Commit 4be058fe9 forgot that the append_rel_list would already be
populated at the time we remove useless result RTEs, and it might contain
PlaceHolderVars that need to be adjusted like the ones in the main parse
tree.  This could lead to "no relation entry for relid N" failures later
on, when the planner tries to do something with an unadjusted PHV.

Per report from Tom Ellis.  Back- to v12 where the bug came in.

Discussion: https://postgr.es/m/20201205173056.GF30712@cloudinit-builder

Fix race conditions in newly-added test.

Buildfarm has been failing sporadically on the new test.  I was able to
reproduce this by adding a random 0-10 s delay in the walreceiver, just
before it connects to the primary. There's a race condition where node_3
is promoted before it has fully caught up with node_1, leading to diverged
timelines. When node_1 is later reconfigured as standby following node_3,
it fails to catch up:

LOG:  primary server contains no more WAL on requested timeline 1
LOG:  new timeline 2 forked off current database system timeline 1 before current recovery point 0/30000A0

That's the situation where you'd need to use pg_rewind, but in this case
it happens already when we are just setting up the actual pg_rewind
scenario we want to test, so change the test so that it waits until
node_3 is connected and fully caught up before promoting it, so that you
get a clean, controlled failover.

Also rewrite some of the comments, for clarity. The existing comments
detailed what each step in the test did, but didn't give a good overview
of the situation the steps were trying to create.

For reasons I don't understand, the test setup had to be written slightly
differently in 9.6 and 9.5 than in later versions. The 9.5/9.6 version
needed node 1 to be reinitialized from backup, whereas in later versions
it could be shut down and reconfigured to be a standby. But even 9.5 should
support "clean switchover", where primary makes sure that pending WAL is
replicated to standby on shutdown. It would be nice to figure out what's
going on there, but that's independent of pg_rewind and the scenario that
this test tests.

Discussion: https://www.postgresql.org/message-id/b0a3b95b-82d2-6089-6892-40570f8c5e60%40iki.fi