Tidy up GetMultiXactIdMembers()'s behavior on error

One of the error paths left *members uninitialized. That's not a live
bug, because most callers don't look at *members when the function
returns -1, but let's be tidy. One caller, in heap_lock_tuple(), does
"if (members != NULL) pfree(members)", but AFAICS it never passes an
invalid 'multi' value so it should not reach that error case.

The callers are also a bit inconsistent in their expectations.
heap_lock_tuple() pfrees the 'members' array if it's not-NULL, others
pfree() it if "nmembers >= 0", and others if "nmembers > 0". That's
not a live bug either, because the function should never return 0, but
add an Assert for that to make it more clear. I left the callers alone
for now.

I also moved the line where we set *nmembers. It wasn't wrong before,
but I like to do that right next to the 'return' statement, to make it
clear that it's always set on return.

Also remove one unreachable return statement after ereport(ERROR), for
brevity and for consistency with the similar if-block right after it.

Author: Greg Nancarrow with the additional changes by me
Backpatch-through: 9.6, all supported versions

Document a few caveats in synchronous logical replication.

In a synchronous logical setup, locking [user] catalog tables can cause
deadlock. This is because logical decoding of transactions can lock
catalog tables to access them so exclusively locking those in transactions
can lead to deadlock. To avoid this users must refrain from having
exclusive locks on catalog tables.

Author: Takamichi Osumi
Reviewed-by: Vignesh C, Amit Kapila
Backpatch-through: 9.6
Discussion: https://www.postgresql.org/message-id/20210222222847.tpnb6eg3yiykzpky%40alap3.anarazel.de

Fix plancache refcount leak after error in ExecuteQuery.

When stuffing a plan from the plancache into a Portal, one is
not supposed to risk throwing an error between GetCachedPlan and
PortalDefineQuery; if that happens, the plan refcount incremented
by GetCachedPlan will be leaked.  I managed to break this rule
while refactoring code in 9dbf2b7d7.  There is no visible
consequence other than some memory leakage, and since nobody is
very likely to trigger the relevant error conditions many times
in a row, it's not surprising we haven't noticed.  Nonetheless,
it's a bug, so rearrange the order of operations to remove the
hazard.

Noted on the way to looking for a better fix for bug #17053.
This mistake is pretty old, so back-patch to all supported
branches.

Fix copying data into slots with FDW batching

Commit b676ac443b optimized handling of tuple slots with bulk inserts
into foreign tables, so that the slots are initialized only once and
reused for all batches. The data was however copied into the slots only
after the initialization, inserting duplicate values when the slot gets
reused. Fixed by moving the ExecCopySlot outside the init branch.

The existing postgres_fdw tests failed to catch this due to inserting
data into foreign tables without unique indexes, and then checking only
the number of inserted rows. This adds a new test with both a unique
index and a check of inserted values.

Reported-by: Alexander Pyhalov
Discussion: https://postgr.es/m/7a8cf8d56b3d18e5c0bccd6cd42d04ac%40postgrespro.ru

Improve SQLSTATE reporting in some replication-related code.

I started out with the goal of reporting ERRCODE_CONNECTION_FAILURE
when walrcv_connect() fails, but as I looked around I realized that
whoever wrote this code was of the opinion that errcodes are purely
optional.  That's not my understanding of our project policy.  Hence,
make sure that an errcode is provided in each ereport that (a) is
ERROR or higher level and (b) isn't arguably an internal logic error.
Also fix some very dubious existing errcode assignments.

While this is not per policy, it's also largely cosmetic, since few
of these cases could get reported to applications.  So I don't
feel a need to back-patch.

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

Fix outdated comment that talked about seek position of WAL file.

Since commit c24dcd0cfd, we have been using pg_pread() to read the WAL
file, which doesn't change the seek position (unless we fall back to
the implementation in src/port/pread.c). Update comment accordingly.

Backpatch-through: 12, where we started to use pg_pread()

Update another variant expected-result file.

This should have been updated in 533e9c6b0, but it was overlooked.
Given the lack of complaints, I won't bother back-patching.

Remove another orphan expected-result file.

aborted-keyrevoke_2.out was apparently needed when it was added (in
commit 0ac5ad513) to handle the case of serializable transaction mode.
However, the output in serializable mode actually matches the regular
aborted-keyrevoke.out file, and AFAICT has done so for a long time.
There's no need to keep dragging this variant along.

Further refinement of stuck_on_old_timeline recovery test

TestLib::perl2host can take a file argument as well as a directory
argument, so that code becomes substantially simpler. Also add comments
on why we're using forward slashes, and why we're setting
PERL_BADLANG=0.

Discussion: https://postgr.es/m/e9947bcd-20ee-027c-f0fe-01f736b7e345@dunslane.net

Revert 29854ee8d1 due to buildfarm failures

Reported-by: Tom Lane
Discussion: https://postgr.es/m/CAPpHfdvcnw3x7jdV3r52p4%3D5S4WUxBCzcQKB3JukQHoicv1LSQ%40mail.gmail.com

Remove unneeded field from VACUUM state.

Bugfix commit 5fc89376 effectively made the lock_waiter_detected field
from vacuumlazy.c's global state struct into private state owned by
lazy_truncate_heap().  Finish this off by replacing the struct field
with a local variable.

Add missing type name "multirange" in docs chapter title

Discussion: https://postgr.es/m/CAFj8pRDioOxiJgmgw9TqQqZ3CxnJC4P5B2Oospf2eMgAjJuewA%40mail.gmail.com
Author: Pavel Stehule, Alexander Korotkov
Reviewed-by: Justin Pryzby, Tom Lane

Support for unnest(multirange) and cast multirange as an array of ranges

It has been spotted that multiranges lack of ability to decompose them into
individual ranges.  Subscription and proper expanded object representation
require substantial work, and it's too late for v14.  This commit
provides the implementation of unnest(multirange) and cast multirange as
an array of ranges, which is quite trivial.

unnest(multirange) is defined as a polymorphic procedure.  The catalog
description of the cast underlying procedure is duplicated for each multirange
type because we don't have anyrangearray polymorphic type to use here.

Catversion is bumped.

Reported-by: Jonathan S. Katz
Discussion: https://postgr.es/m/flat/60258efe-bd7e-4886-82e1-196e0cac5433%40postgresql.org
Author: Alexander Korotkov
Reviewed-by: Justin Pryzby, Jonathan S. Katz, Zhihong Yu

Fix decoding of speculative aborts.

During decoding for speculative inserts, we were relying for cleaning
toast hash on confirmation records or next change records. But that
could lead to multiple problems (a) memory leak if there is neither a
confirmation record nor any other record after toast insertion for a
speculative insert in the transaction, (b) error and assertion failures
if the next operation is not an insert/update on the same table.

The fix is to start queuing spec abort change and clean up toast hash
and change record during its processing. Currently, we are queuing the
spec aborts for both toast and main table even though we perform cleanup
while processing the main table's spec abort record. Later, if we have a
way to distinguish between the spec abort record of toast and the main
table, we can avoid queuing the change for spec aborts of toast tables.

Reported-by: Ashutosh Bapat
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Backpatch-through: 9.6, where it was introduced
Discussion: https://postgr.es/m/CAExHW5sPKF-Oovx_qZe4p5oM6Dvof7_P+XgsNAViug15Fm99jA@mail.gmail.com

Update variant expected-result file.

This should have been updated in d2d8a229b, but it was overlooked.
According to 31a877f18 which added it, this file is meant to show the
results you get under default_transaction_isolation = serializable.
We've largely lost track of that goal in other isolation tests, but
as long as we've got this one, it should be right.

Noted while fooling about with the isolationtester.

Remove orphaned expected-result file.

This should have been removed in 43e084197, which removed the
corresponding spec file.  Noted while fooling about with the
isolationtester.

Remove pg_wait_for_backend_termination().

It was unable to wait on a backend that had already left the procarray.
Users tolerant of that limitation can poll pg_stat_activity.  Other
users can employ the "timeout" argument of pg_terminate_backend().

Reviewed by Bharath Rupireddy.

Discussion: https://postgr.es/m/20210605013236.GA208701@rfd.leadboat.com

Copy-edit text for the pg_terminate_backend() "timeout" parameter.

Revert the pg_description entry to its v13 form, since those messages
usually remain shorter and don't discuss individual parameters.  No
catversion bump, since pg_description content does not impair backend
compatibility or application compatibility.

Justin Pryzby

Discussion: https://postgr.es/m/20210612182743.GY16435@telsasoft.com

Fix logic bug in 1632ea43682f

I overlooked that one condition was logically inverted.  The fix is a
little bit more involved than simply negating the condition, to make
the code easier to read.

Fix some outdated comments left by the same commit, while at it.

Author: Masahiko Sawada <sawada.mshk@gmail.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/YMRlmB3/lZw8YBH+@paquier.xyz

doc:  PG 14 relnotes fixes

Items related to logical replication attribution and BRIN indexes

Reported-by: Tomas Vondra, John Naylor
Discussion: https://postgr.es/m/0db66294-a668-2caa-2b5e-a8db60b30662@enterprisedb.com, CAFBsxsH21KnteYdk33F1oZu2O726NSD6_XBq51Tn0jytsA1AnA@mail.gmail.com

doc:  PG 14 relnote updates

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20210612034551.GU16435@telsasoft.com

doc:  add PG 14 relnote item about array function references

User-defined objects that reference some built-in array functions will
need to be recreated in PG 14.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20210608225618.GR16435@telsasoft.com

Improve handling of dropped objects in pg_event_trigger_ddl_commands()

An object found as dropped when digging into the list of objects
returned by pg_event_trigger_ddl_commands() could cause a cache lookup
error, as the calls grabbing for the object address and the type name
would fail if the object was missing.

Those lookup errors could be seen with combinations of ALTER TABLE
sub-commands involving identity columns.  The lookup logic is changed in
this code path to get a behavior similar to any other SQL-callable
function by ignoring objects that are not found, taking advantage of
2a10fdc.  The back-branches are not changed, as they require this commit
that is too invasive for stable branches.

While on it, add test cases to exercise event triggers with identity
columns, and stress more cases with the event ddl_command_end for
relations.

Author: Sven Klemm, Aleksander Alekseev, Michael Paquier
Discussion: https://postgr.es/m/CAMCrgp2R1cEXU53iYKtW6yVEp2_yKUz+z=3-CTrYpPP+xryRtg@mail.gmail.com

Remove forced toast recompression in VACUUM FULL/CLUSTER

The extra checks added by the recompression of toast data introduced in
bbe0a81 is proving to have a performance impact on VACUUM or CLUSTER
even if no recompression is done.  This is more noticeable with more
toastable columns that contain non-NULL values.

Improvements could be done to make those extra checks less expensive,
but that's not material for 14 at this stage, and we are not sure either
if the code path of VACUUM FULL/CLUSTER is adapted for this job.

Per discussion with several people, including Andres Freund, Robert
Haas, Álvaro Herrera, Tom Lane and myself.

Discussion: https://postgr.es/m/20210527003144.xxqppojoiwurc2iz@alap3.anarazel.de

Work around portability issue with newer versions of mktime().

Recent glibc versions have made mktime() fail if tm_isdst is
inconsistent with the prevailing timezone; in particular it fails for
tm_isdst = 1 when the zone is UTC.  (This seems wildly inconsistent
with the POSIX-mandated treatment of "incorrect" values for the other
fields of struct tm, so if you ask me it's a bug, but I bet they'll
say it's intentional.)  This has been observed to cause cosmetic
problems when pg_restore'ing an archive created in a different
timezone.

To fix, do mktime() using the field values from the archive, and if
that fails try again with tm_isdst = -1.  This will give a result
that's off by the UTC-offset difference from the original zone, but
that was true before, too.  It's not terribly critical since we don't
do anything with the result except possibly print it.  (Someday we
should flush this entire bit of logic and record a standard-format
timestamp in the archive instead.  That's not okay for a back-patched
bug fix, though.)

Also, guard our only other use of mktime() by having initdb's
build_time_t() set tm_isdst = -1 not 0.  This case could only have
an issue in zones that are DST year-round; but I think some do exist,
or could in future.

Per report from Wells Oliver.  Back-patch to all supported
versions, since any of them might need to run with a newer glibc.

Discussion: https://postgr.es/m/CAOC+FBWDhDHO7G-i1_n_hjRzCnUeFO+H-Czi1y10mFhRWpBrew@mail.gmail.com

Further tweaks to stuck_on_old_timeline recovery test

Translate path slashes on target directory path. This was confusing old
branches, but is applied to all branches for the sake of uniformity.
Perl is perfectly able to understand paths with forward slashes.

Along the way, restore the previous archive_wait query, for the sake of
uniformity with other tests, per gripe from Tom Lane.

Ignore more environment variables in pg_regress.c

This is similar to the work done in 8279f68 for TestLib.pm, where
environment variables set may cause unwanted failures if using a
temporary installation with pg_regress.  The list of variables reset is
adjusted in each stable branch depending on what is supported.

Comments are added to remember that the lists in TestLib.pm and
pg_regress.c had better be kept in sync.

Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/YMNR9GYDn+fHlMta@paquier.xyz
Backpatch-through: 9.6

Restore robustness of TAP tests that wait for postmaster restart.

Several TAP tests use poll_query_until() to wait for the postmaster
to restart.  They were checking to see if a trivial query
(e.g. "SELECT 1") succeeds.  However, that's problematic in the wake
of commit 11e9caff8, because now that we feed said query to psql
via stdin, we risk IPC::Run whining about a SIGPIPE failure if psql
quits before reading the query.  Hence, we can't use a nonempty
query in cases where we need to wait for connection failures to
stop happening.

Per the precedent of commits c757a3da0 and 6d41dd045, we can pass
"undef" as the query in such cases to ensure that IPC::Run has
nothing to write.  However, then we have to say that the expected
output is empty, and this exposes a deficiency in poll_query_until:
if psql fails altogether and returns empty stdout, poll_query_until
will treat that as a success!  That's because, contrary to its
documentation, it makes no actual check for psql failure, looking
neither at the exit status nor at stderr.

To fix that, adjust poll_query_until to insist on empty stderr as
well as a stdout match.  (I experimented with checking exit status
instead, but it seems that psql often does exit(1) in cases that we
need to consider successes.  That might be something to fix someday,
but it would be a non-back-patchable behavior change.)

Back-patch to v10.  The test cases needing this exist only as far
back as v11, but it seems wise to keep poll_query_until's behavior
the same in v10, in case we back-patch another such test case in
future.  (9.6 does not currently need this change, because in that
branch poll_query_until can't be told to accept empty stdout as
a success case.)

Per assorted buildfarm failures, mostly on hoverfly.

Discussion: https://postgr.es/m/CAA4eK1+zM6L4QSA1XMvXY_qqWwdUmqkOS1+hWvL8QcYEBGA1Uw@mail.gmail.com

Ensure pg_filenode_relation(0, 0) returns NULL.

Previously, a zero value for the relfilenode resulted in
a confusing error message about "unexpected duplicate".
This function returns NULL for other invalid relfilenode
values, so zero should be treated likewise.

It's been like this all along, so back-patch to all supported
branches.

Justin Pryzby

Discussion: https://postgr.es/m/20210612023324.GT16435@telsasoft.com

Don't use Asserts to check for violations of replication protocol.

Using an Assert to check the validity of incoming messages is an
extremely poor decision.  In a debug build, it should not be that easy
for a broken or malicious remote client to crash the logrep worker.
The consequences could be even worse in non-debug builds, which will
fail to make such checks at all, leading to who-knows-what misbehavior.
Hence, promote every Assert that could possibly be triggered by wrong
or out-of-order replication messages to a full test-and-ereport.

To avoid bloating the set of messages the translation team has to cope
with, establish a policy that replication protocol violation error
reports don't need to be translated.  Hence, all the new messages here
use errmsg_internal().  A couple of old messages are changed likewise
for consistency.

Along the way, fix some non-idiomatic or outright wrong uses of
hash_search().

Most of these mistakes are new with the "streaming replication"
patch (commit 464824323), but a couple go back a long way.
Back-patch as appropriate.

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

Fix new recovery test for use under msys

Commit caba8f0d43 wasn't quite right for msys, as demonstrated by
several buildfarm animals, including jacana and fairywren. We need to
use the msys perl in the archive command, but call it in such a way that
Windows will understand the path. Furthermore, inside the copy script we
need to convert a Windows path to an msys path.

Simplify some code in getObjectTypeDescription()

This routine is designed to never return an empty description or NULL,
providing description fallbacks even if missing objects are accepted,
but it included a code path where this was considered possible.  All the
callers of this routine already consider NULL as not possible, so
change a bit the code to map with the assumptions of the callers, and
add more comments close to the callers of this routine to outline the
behavior expected.

This code is new as of 2a10fdc, so no backpatch is needed.

Discussion: https://postgr.es/m/YMNY6RGPBRCeLmFb@paquier.xyz

Improve log pattern detection in recently-added TAP tests

ab55d74 has introduced some tests with rows found as missing in logical
replication subscriptions for partitioned tables, relying on a logic
with a lookup of the logs generated, scanning the whole file.  This
commit makes the logic more precise, by scanning the logs only from the
position before the key queries are run to the position where we check
for the logs.  This will reduce the risk of issues with log patterns
overlapping with each other if those tests get more complicated in the
future.

Per discussion with Tom Lane.

Discussion: https://postgr.es/m/YMP+Gx2S8meYYHW4@paquier.xyz
Backpatch-through: 13

Improve and cleanup ProcArrayAdd(), ProcArrayRemove().

941697c3c1a changed ProcArrayAdd()/Remove() substantially. As reported by
zhanyi, I missed that due to the introduction of the PGPROC->pgxactoff
ProcArrayRemove() does not need to search for the position in
procArray->pgprocnos anymore - that's pgxactoff. Remove the search loop.

The removal of the search loop reduces assertion coverage a bit - but we can
easily do better than before by adding assertions to other loops over
pgprocnos elements.

Also do a bit of cleanup, mostly by reducing line lengths by introducing local
helper variables and adding newlines.

Author: zhanyi <w@hidva.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/tencent_5624AA3B116B3D1C31CA9744@qq.com

doc:  remove extra right angle bracket in PG 14 relnotes

Reported-by: Justin Pryzby

docs:  fix incorrect indenting in PG 14 relnotes

Report sort phase progress in parallel btree build

We were already reporting it, but only after the parallel workers were
finished, which is visibly much later than what happens in a serial
build.

With this change we report it when the leader starts its own sort phase
when participating in the build (the normal case).  Now this might
happen a little later than when the workers start their sorting phases,
but a) communicating the actual phase start from workers is likely to be
a hassle, and b) the sort phase start is pretty fuzzy anyway, since
sorting per se is actually initiated by tuplesort.c internally earlier
than tuplesort_performsort() is called.

Backpatch to pg12, where the progress reporting code for CREATE INDEX
went in.

Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
Author: Matthias van de Meent <boekewurm+postgres@gmail.com>
Reviewed-by: Greg Nancarrow <gregn4422@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/1128176d-1eee-55d4-37ca-e63644422adb

Fix multiple crasher bugs in partitioned-table replication logic.

apply_handle_tuple_routing(), having detected and reported that
the tuple it needed to update didn't exist, tried to update that
tuple anyway, leading to a null-pointer dereference.

logicalrep_partition_open() failed to ensure that the
LogicalRepPartMapEntry it built for a partition was fully
independent of that for the partition root, leading to
trouble if the root entry was later freed or rebuilt.

Meanwhile, on the publisher's side, pgoutput_change() sometimes
attempted to apply execute_attr_map_tuple() to a NULL tuple.

The first of these was reported by Sergey Bernikov in bug #17055;
I found the other two while developing some test cases for this
sadly under-tested code.

Diagnosis and patch for the first issue by Amit Langote; patches
for the others by me; new test cases by me.  Back-patch to v13
where this logic came in.

Discussion: https://postgr.es/m/17055-9ba800ec8522668b@postgresql.org

Add 'Portal Close' message to pipelined PQsendQuery()

Commit acb7e4eb6b1c added a new implementation for PQsendQuery so that
it works in pipeline mode (by using extended query protocol), but it
behaves differently from the 'Q' message (in simple query protocol) used
by regular implementation: the new one doesn't close the unnamed portal.
Change the new code to have identical behavior to the old.

Reported-by: Yura Sokolov <y.sokolov@postgrespro.ru>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/202106072107.d4i55hdscxqj@alvherre.pgsql

Return ReplicationSlotAcquire API to its original form

Per 96540f80f833; the awkward API introduced by c6550776394e is no
longer needed.

Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20210408020913.zzprrlvqyvlt5cyy@alap3.anarazel.de

Optimize creation of slots for FDW bulk inserts

Commit b663a41363 introduced bulk inserts for FDW, but the handling of
tuple slots turned out to be problematic for two reasons. Firstly, the
slots were re-created for each individual batch. Secondly, all slots
referenced the same tuple descriptor - with reasonably small batches
this is not an issue, but with large batches this triggers O(N^2)
behavior in the resource owner code.

These two issues work against each other - to reduce the number of times
a slot has to be created/dropped, larger batches are needed. However,
the larger the batch, the more expensive the resource owner gets. For
practical batch sizes (100 - 1000) this would not be a big problem, as
the benefits (latency savings) greatly exceed the resource owner costs.
But for extremely large batches it might be much worse, possibly even
losing with non-batching mode.

Fixed by initializing tuple slots only once (and reusing them across
batches) and by using a new tuple descriptor copy for each slot.

Discussion: https://postgr.es/m/ebbbcc7d-4286-8c28-0272-61b4753af761%40enterprisedb.com

Fix race condition in invalidating obsolete replication slots

The code added to mark replication slots invalid in commit c6550776394e
had the race condition that a slot can be dropped or advanced
concurrently with checkpointer trying to invalidate it.  Rewrite the
code to close those races.

The changes to ReplicationSlotAcquire's API added with c6550776394e are
not necessary anymore.  To avoid an ABI break in released branches, this
commit leaves that unchanged; it'll be changed in a master-only commit
separately.

Backpatch to 13, where this code first appeared.

Reported-by: Andres Freund <andres@anarazel.de>
Author: Andres Freund <andres@anarazel.de>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20210408001037.wfmk6jud36auhfqm@alap3.anarazel.de

Improve psql tab completion for options of subcriptions and publications

The list of options provided by the tab completion was outdated for the
following commands:
- ALTER SUBSCRIPTION
- CREATE SUBSCRIPTION
- ALTER PUBLICATION
- CREATE PUBLICATION

Author: Vignesh C
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/CALDaNm18oHDFu6SFCHE=ZbiO153Fx7E-L1MG0YyScbaDV--U+A@mail.gmail.com

Change position of field "transformed" in struct CreateStatsStmt.

Resolve the disagreement with nodes/*funcs.c field order in favor of the
latter, which is better-aligned with the IndexStmt field order.  This
field is new in v14.

Discussion: https://postgr.es/m/20210611045546.GA573364@rfd.leadboat.com

Rename PQtraceSetFlags() to PQsetTraceFlags().

We have a dozen PQset*() functions.  PQresultSetInstanceData() and this
were the libpq setter functions having a different word order.  Adopt
the majority word order.

Reviewed by Alvaro Herrera and Robert Haas, though this choice of name
was not unanimous.

Discussion: https://postgr.es/m/20210605060555.GA216695@rfd.leadboat.com

Use the correct article for abbreviations

We've accumulated quite a mix of instances of "an SQL" and "a SQL" in the
documents.  It would be good to be a bit more consistent with these.

The most recent version of the SQL standard I looked at seems to prefer
"an SQL".  That seems like a good lead to follow, so here we change all
instances of "a SQL" to become "an SQL".  Most instances correctly use
"an SQL" already, so it also makes sense to use the dominant variation in
order to minimise churn.

Additionally, there were some other abbreviations that needed to be
adjusted. FSM, SSPI, SRF and a few others.  Also fix some pronounceable,
abbreviations to use "a" instead of "an".  For example, "a SASL" instead
of "an SASL".

Here I've only adjusted the documents and error messages.  Many others
still exist in source code comments.  Translator hint comments seem to be
the biggest culprit.  It currently does not seem worth the churn to change
these.

Discussion: https://postgr.es/m/CAApHDvpML27UqFXnrYO1MJddsKVMQoiZisPvsAGhKE_tsKXquw%40mail.gmail.com

Reconsider the handling of procedure OUT parameters.

Commit 2453ea142 redefined pg_proc.proargtypes to include the types of
OUT parameters, for procedures only.  While that had some advantages
for implementing the SQL-spec behavior of DROP PROCEDURE, it was pretty
disastrous from a number of other perspectives.  Notably, since the
primary key of pg_proc is name + proargtypes, this made it possible to
have multiple procedures with identical names + input arguments and
differing output argument types.  That would make it impossible to call
any one of the procedures by writing just NULL (or "?", or any other
data-type-free notation) for the output argument(s).  The change also
seems likely to cause grave confusion for client applications that
examine pg_proc and expect the traditional definition of proargtypes.

Hence, revert the definition of proargtypes to what it was, and
undo a number of complications that had been added to support that.

To support the SQL-spec behavior of DROP PROCEDURE, when there are
no argmode markers in the command's parameter list, we perform the
lookup both ways (that is, matching against both proargtypes and
proallargtypes), succeeding if we get just one unique match.
In principle this could result in ambiguous-function failures
that would not happen when using only one of the two rules.
However, overloading of procedure names is thought to be a pretty
rare usage, so this shouldn't cause many problems in practice.
Postgres-specific code such as pg_dump can defend against any
possibility of such failures by being careful to specify argmodes
for all procedure arguments.

This also fixes a few other bugs in the area of CALL statements
with named parameters, and improves the documentation a little.

catversion bump forced because the representation of procedures
with OUT arguments changes.

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

Rearrange logrep worker's snapshot handling some more.

It turns out that worker.c's code path for TRUNCATE was also
careless about establishing a snapshot while executing user-defined
code, allowing the checks added by commit 84f5c2908 to fail when
a trigger is fired in that context.

We could just wrap Push/PopActiveSnapshot around the truncate call,
but it seems better to establish a policy of holding a snapshot
throughout execution of a replication step.  To help with that and
possible future requirements, replace the previous ensure_transaction
calls with pairs of begin/end_replication_step calls.

Per report from Mark Dilger.  Back-patch to v11, like the previous
changes.

Discussion: https://postgr.es/m/B4A3AF82-79ED-4F4C-A4E5-CD2622098972@enterprisedb.com

Shut down EvalPlanQual machinery when LockRows node reaches the end.

Previously, we left the EPQ sub-executor alone until ExecEndLockRows.
This caused any buffer pins or other resources that it might hold to
remain held until ExecutorEnd, which in some code paths means that
they are held till the Portal is closed.  That can cause user-visible
problems, such as blocking VACUUM; and it's unlike the behavior of
ordinary table-scanning nodes, which will have released all buffer
pins by the time they return an EOF indication.

We can make LockRows work more like other plan nodes by calling
EvalPlanQualEnd just before returning NULL.  We still need to call it
in ExecEndLockRows in case the node was not run to completion, but in
the normal case the second call does nothing and costs little.

Per report from Yura Sokolov.  In principle this is a longstanding
bug, but in view of the lack of other complaints and the low severity
of the consequences, I chose not to back-patch.

Discussion: https://postgr.es/m/4aa370cb91ecf2f9885d98b80ad1109c@postgrespro.ru

Avoid ECPG test failures in some GSS-capable environments.

Buildfarm member hamerkop has been reporting that two cases in
connect/test5.pgc show different error messages than the test expects,
because since commit ffa2e4670 libpq's connection failure messages
are exposing the fact that a GSS-encrypted connection was attempted
and failed.  That's pretty interesting information in itself, and
I certainly don't wish to shoot the messenger, but we need to do
something to stabilize the ECPG results.

For the second of these two failure cases, we can add the
gssencmode=disable option to prevent the discrepancy.  However,
that solution is problematic for the first failure, because the only
unique thing about that case is that it's testing a completely-omitted
connection target; there's noplace to add the option without defeating
the point of the test case.  After some thrashing around with
alternative fixes that turned out to have undesirable side-effects,
the most workable answer is just to give up and remove that test case.
Perhaps we can revert this later, if we figure out why the GSS code
is misbehaving in hamerkop's environment.

Thanks to Michael Paquier for exploration of alternatives.

Discussion: https://postgr.es/m/YLRZH6CWs9N6Pusy@paquier.xyz

Add some const decorations

One of these functions is new in PostgreSQL 14; might as well start it
out right.

Adjust new test case to set wal_keep_size.

Per buildfarm member conchuela and Kyotaro Horiguchi, it's possible
for the WAL segment that the cascading standby needs to be removed
too quickly. Hopefully this will prevent that.

Kyotaro Horiguchi

Discussion: http://postgr.es/m/20210610.101240.1270925505780628275.horikyota.ntt@gmail.com

Fix an asssortment of typos in brin_minmax_multi.c and mcv.c

Discussion: https://postgr.es/m/CAApHDvrbyJNOPBws4RUhXghZ7+TBjtdO-rznTsqZECuowNorXg@mail.gmail.com

Fix corner case failure of new standby to follow new primary.

This only happens if (1) the new standby has no WAL available locally,
(2) the new standby is starting from the old timeline, (3) the promotion
happened in the WAL segment from which the new standby is starting,
(4) the timeline history file for the new timeline is available from
the archive but the WAL files for are not (i.e. this is a race),
(5) the WAL files for the new timeline are available via streaming,
and (6) recovery_target_timeline='latest'.

Commit ee994272ca50f70b53074f0febaec97e28f83c4e introduced this
logic and was an improvement over the previous code, but it mishandled
this case. If recovery_target_timeline='latest' and restore_command is
set, validateRecoveryParameters() can change recoveryTargetTLI to be
different from receiveTLI. If streaming is then tried afterward,
expectedTLEs gets initialized with the history of the wrong timeline.
It's supposed to be a list of entries explaining how to get to the
target timeline, but in this case it ends up with a list of entries
explaining how to get to the new standby's original timeline, which
isn't right.

Dilip Kumar and Robert Haas, reviewed by Kyotaro Horiguchi.

Discussion: http://postgr.es/m/CAFiTN-sE-jr=LB8jQuxeqikd-Ux+jHiXyh4YDiZMPedgQKup0g@mail.gmail.com

Fix inconsistencies in psql --help=commands

The set of subcommands supported by \dAp, \do and \dy was described
incorrectly in psql's --help.  The documentation was already consistent
with the code.

Reported-by: inoas, from IRC
Author: Matthijs van der Vleuten
Reviewed-by: Neil Chen
Discussion: https://postgr.es/m/6a984e24-2171-4039-9050-92d55e7b23fe@www.fastmail.com
Backpatch-through: 9.6

Force NO SCROLL for plpgsql's implicit cursors.

Further thought about bug #17050 suggests that it's a good idea
to use CURSOR_OPT_NO_SCROLL for the implicit cursor opened by
a plpgsql FOR-over-query loop.  This ensures that, if somebody
commits inside the loop, PersistHoldablePortal won't try to
rewind and re-read the cursor.  While we'd have selected NO_SCROLL
anyway if FOR UPDATE/SHARE appears in the query, there are other
hazards with volatile functions; and in any case, it's silly to
expend effort storing rows that we know for certain won't be needed.

(While here, improve the comment in exec_run_select, which was a bit
confused about the rationale for when we can use parallel mode.
Cursor operations aren't a hazard for nameless portals.)

This wasn't an issue until v11, which introduced the possibility
of persisting such cursors.  Hence, back-patch to v11.

Per bug #17050 from Алексей Булгаков.

Discussion: https://postgr.es/m/17050-f77aa827dc85247c@postgresql.org

Avoid misbehavior when persisting a non-stable cursor.

PersistHoldablePortal has long assumed that it should store the
entire output of the query-to-be-persisted, which requires rewinding
and re-reading the output.  This is problematic if the query is not
stable: we might get different row contents, or even a different
number of rows, which'd confuse the cursor state mightily.

In the case where the cursor is NO SCROLL, this is very easy to
solve: just store the remaining query output, without any rewinding,
and tweak the portal's cursor state to match.  Aside from removing
the semantic problem, this could be significantly more efficient
than storing the whole output.

If the cursor is scrollable, there's not much we can do, but it
was already the case that scrolling a volatile query's result was
pretty unsafe.  We can just document more clearly that getting
correct results from that is not guaranteed.

There are already prohibitions in place on using SCROLL with
FOR UPDATE/SHARE, which is one way for a SELECT query to have
non-stable results.  We could imagine prohibiting SCROLL when
the query contains volatile functions, but that would be
expensive to enforce.  Moreover, it could break applications
that work just fine, if they have functions that are in fact
stable but the user neglected to mark them so.  So settle for
documenting the hazard.

While this problem has existed in some guise for a long time,
it got a lot worse in v11, which introduced the possibility
of persisting plpgsql cursors (perhaps implicit ones) even
when they violate the rules for what can be marked WITH HOLD.
Hence, I've chosen to back-patch to v11 but not further.

Per bug #17050 from Алексей Булгаков.

Discussion: https://postgr.es/m/17050-f77aa827dc85247c@postgresql.org

doc:  update release note item about the v2 wire protocol

Protocol v2 was last used in PG 7.3, not 7.2.

Reported-by: Tatsuo Ishii
Discussion: https://postgr.es/m/20210608.091329.906837606658882674.t-ishii@sraoss.co.jp

Adjust batch size in postgres_fdw to not use too many parameters

The FE/BE protocol identifies parameters with an Int16 index, which
limits the maximum number of parameters per query to 65535. With
batching added to postges_fdw this limit is much easier to hit, as
the whole batch is essentially a single query, making this error much
easier to hit.

The failures are a bit unpredictable, because it also depends on the
number of columns in the query. So instead of just failing, this patch
tweaks the batch_size to not exceed the maximum number of parameters.

Reported-by: Hou Zhijie <houzj.fnst@cn.fujitsu.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/OS0PR01MB571603973C0AC2874AD6BF2594299%40OS0PR01MB5716.jpnprd01.prod.outlook.com

Fix pg_visibility regression failure with CLOBBER_CACHE_ALWAYS

Commit 8e03eb92e9 reverted a bit too much code, reintroducing one of the
issues fixed by 39b66a91bd - a page might have been left partially empty
after relcache invalidation.

Reported-By: Tom Lane
Author: Masahiko Sawada
Discussion: https://postgr.es/m/822752.1623032114@sss.pgh.pa.us
Discussion: https://postgr.es/m/CAD21AoA%3D%3Df2VSw3c-Cp_y%3DWLKHMKc1D6s7g3YWsCOvgaYPpJcg%40mail.gmail.com

Don't crash on empty statements in SQL-standard function bodies.

gram.y should discard NULL pointers (empty statements) when
assembling a routine_body_stmt_list, as it does for other
sorts of statement lists.

Julien Rouhaud and Tom Lane, per report from Noah Misch.

Discussion: https://postgr.es/m/20210606044418.GA297923@rfd.leadboat.com

libpq: Fix SNI host handling

Fix handling of NULL host name (possibly by using hostaddr).  It
previously crashed.  Also, we should look at connhost, not pghost, to
handle multi-host specifications.

Also remove an unnecessary SSL_CTX_free().

Reported-by: Jacob Champion <pchampion@vmware.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/504c276ab6eee000bb23d571ea9b0ced4250774e.camel@vmware.com

Doc: Further update documentation for asynchronous execution.

Add a note about asynchronous execution by postgres_fdw when applied to
Append nodes that contain synchronous subplan(s) as well.  Follow-up for
commit 27e1f1456.

Andrey Lepikhov and Etsuro Fujita

Discussion: https://postgr.es/m/58fa2aa5-07f5-80b5-59a1-fec8a349fee7%40postgrespro.ru

Reorder superuser check in pg_log_backend_memory_contexts()

The use of this function is limited to superusers and the code includes
a hardcoded check for that.  However, the code would look for the PGPROC
entry to signal for the memory dump before checking if the user is a
superuser or not, which does not make sense if we know that an error
will be returned.  Note that the code would let one know if a process
was a PostgreSQL process or not even for non-authorized users, which is
not the case now, but this avoids taking ProcArrayLock that will most
likely finish by being unnecessary.

Thanks to Julien Rouhaud and Tom Lane for the discussion.

Discussion: https://postgr.es/m/YLxw1uVGIAP5uMPl@paquier.xyz

Add _outTidRangePath()

We have outNode() coverage for all path nodes, but this one was
missed when it was added.

Stabilize contrib/seg regression test.

If autovacuum comes along just after we fill table test_seg with
some data, it will update the stats to the point where we prefer
a plain indexscan over a bitmap scan, breaking the expected
output (as well as the point of the test case).  To fix, just
force a bitmap scan to be chosen here.

This has evidently been wrong since commit de1d042f5.  It's not
clear why we just recently saw any buildfarm failures due to it;
but prairiedog has failed twice on this test in the past week.
Hence, backpatch to v11 where this test case came in.

Fix incautious handling of possibly-miscoded strings in client code.

An incorrectly-encoded multibyte character near the end of a string
could cause various processing loops to run past the string's
terminating NUL, with results ranging from no detectable issue to
a program crash, depending on what happens to be in the following
memory.

This isn't an issue in the server, because we take care to verify
the encoding of strings before doing any interesting processing
on them.  However, that lack of care leaked into client-side code
which shouldn't assume that anyone has validated the encoding of
its input.

Although this is certainly a bug worth fixing, the PG security team
elected not to regard it as a security issue, primarily because
any untrusted text should be sanitized by PQescapeLiteral or
the like before being incorporated into a SQL or psql command.
(If an app fails to do so, the same technique can be used to
cause SQL injection, with probably much more dire consequences
than a mere client-program crash.)  Those functions were already
made proof against this class of problem, cf CVE-2006-2313.

To fix, invent PQmblenBounded() which is like PQmblen() except it
won't return more than the number of bytes remaining in the string.
In HEAD we can make this a new libpq function, as PQmblen() is.
It seems imprudent to change libpq's API in stable branches though,
so in the back branches define PQmblenBounded as a macro in the files
that need it.  (Note that just changing PQmblen's behavior would not
be a good idea; notably, it would completely break the escaping
functions' defense against this exact problem.  So we just want a
version for those callers that don't have any better way of handling
this issue.)

Per private report from houjingyi.  Back-patch to all supported branches.

Fix portability issue in test indirect_toast

When run on a server using default_toast_compression set to LZ4, this
test would fail because of a consistency issue with the order of the
tuples treated.  LZ4 causes one tuple to be stored inline instead of
getting externalized.  As the goal of this test is to check after data
stored externally, stick to pglz as the compression algorithm used, so
as all data of this test is stored the way it should.

Analyzed-by: Dilip Kumar
Discussion: https://postgr.es/m/YLrDWxJgM8WWMoCg@paquier.xyz

Remove two_phase variable from CreateReplicationSlotCmd struct.

Commit 19890a064e added the option to enable two_phase commits via
pg_create_logical_replication_slot but didn't extend the support of same
in replication protocol. However, by mistake, it added the two_phase
variable in CreateReplicationSlotCmd which is required only when we extend
the replication protocol.

Reported-by: Jeff Davis
Author: Ajin Cherian
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/64b9f783c6e125f18f88fbc0c0234e34e71d8639.camel@j-davis.com

Fix rescanning of async-aware Append nodes.

In cases where run-time pruning isn't required, the synchronous and
asynchronous subplans for an async-aware Append node determined using
classify_matching_subplans() should be re-used when rescanning the node,
but the previous code re-determined them using that function repeatedly
each time when rescanning the node, leading to incorrect results in a
normal build and an Assert failure in an Assert-enabled build as that
function doesn't assume that it's called repeatedly in such cases.  Fix
the code as mentioned above.

My oversight in commit 27e1f1456.

While at it, initialize async-related pointers/variables to NULL/zero
explicitly in ExecInitAppend() and ExecReScanAppend(), just to be sure.
(The variables would have been set to zero before we get to the latter
function, but let's do so.)

Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CAPmGK16Q4B2_KY%2BJH7rb7wQbw54AUprp7TMekGTd2T1B62yysQ%40mail.gmail.com

Fix inconsistent equalfuncs.c behavior for FuncCall.funcformat.

Other equalfuncs.c checks on CoercionForm fields use
COMPARE_COERCIONFORM_FIELD (which makes them no-ops),
but commit 40c24bfef neglected to make _equalFuncCall
do likewise.  Fix that.

This is only strictly correct if FuncCall.funcformat has
no semantic effect, instead just determining ruleutils.c
display formatting.  40c24bfef added a couple of checks
in parse analysis that could break that rule; but on closer
inspection, they're redundant, so just take them out again.

Per report from Noah Misch.

Discussion: https://postgr.es/m/20210606063331.GC297923@rfd.leadboat.com

Add transformed flag to nodes/*funcs.c for CREATE STATISTICS

Commit a4d75c86bf added a new flag, tracking if the statement was
processed by transformStatsStmt(), but failed to add this flag to
nodes/*funcs.c.

Catversion bump, due to adding a flag to copy/equal/out functions.

Reported-by: Noah Misch
Discussion: https://postgr.es/m/ad7891d2-e90c-b446-9fe2-7419143847d7%40enterprisedb.com

Standardize nodes/*funcs.c cosmetics for ForeignScan.resultRelation.

catversion bump due to readfuncs.c field order change.

doc: Simplify COMMENT and SECURITY LABEL documentation

Just say that objects that reside in schemas can be schema-qualified.
Don't list all possible such objects.  The existing lists weren't
complete anyway.

Discussion: https://www.postgresql.org/message-id/flat/b2ec2234-67fe-d861-5cea-f526cd18c086%40enterprisedb.com

doc: Make terminology in glossary consistent

Use "reside in" rather than "belong to" for objects in a schema.
Previous use was a mix of the two.

Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/202106021932.idmbjjaqk7ke@alvherre.pgsql

gitattributes: Add new entry to silence whitespace error

Fix subtransaction test for Python 3.10

Starting with Python 3.10, the stacktrace looks differently:
  -  PL/Python function "subtransaction_exit_subtransaction_in_with", line 3, in <module>
  -    s.__exit__(None, None, None)
  +  PL/Python function "subtransaction_exit_subtransaction_in_with", line 2, in <module>
  +    with plpy.subtransaction() as s:
Using try/except specifically makes the error look always the same.

(See https://github.com/python/cpython/pull/25719 for the discussion
of this change in Python.)

Author: Honza Horak <hhorak@redhat.com>
Discussion: https://www.postgresql.org/message-id/flat/853083.1620749597%40sss.pgh.pa.us
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1959080

Fix postgres_fdw failure with whole-row Vars of type RECORD.

Commit 86dc90056 expects that FDWs can cope with whole-row Vars for
their tables, even if the Vars are marked with vartype RECORDOID.
Previously, whole-row Vars generated by the planner had vartype equal
to the relevant table's rowtype OID.  (The point behind this change is
to enable sharing of resjunk columns across inheritance child tables.)

It turns out that postgres_fdw fails to cope with this, though through
bad fortune none of its test cases exposed that.  Things mostly work,
but when we try to read back a value of such a Var, the expected
rowtype is not available to record_in().  Fortunately, it's not
difficult to hack up the tupdesc that controls this process to
substitute the foreign table's rowtype for RECORDOID.  Thus we can
solve the runtime problem while still sharing the resjunk column
with other tables.

Per report from Alexander Pyhalov.

Discussion: https://postgr.es/m/7817fb9ebd6661cdf9b67dec6e129a78@postgrespro.ru

Doc: Fix some spelling mistakes

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/7838B8EE-CFD6-48D7-AE2D-520D69FD84BD@yesql.se

Clean up some questionable usages of DatumGet* macros

This tidies up some questionable coding which made use of
DatumGetPointer() for Datums being passed into functions where the
parameter is expected to be a cstring.  We saw no compiler warnings with
the old code as the Pointer type used in DatumGetPointer() happens to
be a char * rather than a void *.  However, that's no excuse and we should
be using the correct macro for the job.

Here we also make use of OutputFunctionCall() rather than using
FunctionCall1() directly to call the type's output function.
OutputFunctionCall() is the standard way to do this.  It casts the
returned value to a cstring for us.

In passing get rid of a duplicate call to strlen().  Most compilers will
likely optimize away the 2nd call, but there may be some that won't.  In
any case, this just aligns the code to some other nearby code that already
does this.

Discussion: https://postgr.es/m/CAApHDvq1D=ehZ8hey8Hz67N+_Zth0GHO5wiVCfv1YcGPMXJq0A@mail.gmail.com

Doc: fix bogus intarray index example.

The siglen parameter is provided by gist__intbig_ops not
gist__int_ops.

Simon Norris

Discussion: https://postgr.es/m/11BF2AA9-17AE-432A-AFE1-584FB9FB079D@hillcrestgeo.ca

doc: Add description for PGSSLCRLDIR

This was missing in the section dedicated to the supported environment
variables of libpq.  Oversight in f5465fa.

Reviewed-by: Daniel Gustafsson, Kyotaro Horiguchi
Discussion: https://postgr.es/m/YLhI0mLoRkY3u4Wj@paquier.xyz

doc: Fix link reference for PGSSLMAXPROTOCOLVERSION

The link was pointing to the minimum protocol version.  Incorrect as of
ff8ca5f.

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/F893F184-C645-4C21-A2BA-583441B7288F@yesql.se
Backpatch-through: 13

Adjust locations which have an incorrect copyright year

A few patches committed after ca3b37487 mistakenly forgot to make the
copyright year 2021.  Fix these.

Discussion: https://postgr.es/m/CAApHDvqyLmd9P2oBQYJ=DbrV8QwyPRdmXtCTFYPE08h+ip0UJw@mail.gmail.com

In PostgresNode.pm, don't pass SQL to psql on the command line

The Msys shell mangles certain patterns in its command line, so avoid
handing arbitrary SQL to psql on the command line and instead use
IPC::Run's redirection facility for stdin. This pattern is already
mostly whats used, but query_poll_until() was not doing the right thing.

Problem discovered on the buildfarm when a new TAP test failed on msys.

Fix incorrect permissions on pg_subscription.

The documented intent is for all columns except subconninfo to be
publicly readable.  However, this has been overlooked twice.
subsynccommit has never been readable since it was introduced,
nor has the oid column (which is important for joining).

Given the lack of previous complaints, it's not clear that it's
worth doing anything about this in the back branches.  But there's
still time to fix it inexpensively for v14.

Per report from Israel Barth (via Euler Taveira).

Patch by Euler Taveira, possibly-vain comment updates by me.

Discussion: https://postgr.es/m/b8f7c17c-0041-46b6-acfe-2d1f5a985ab4@www.fastmail.com

Reduce risks of conflicts in internal queries of REFRESH MATVIEW CONCURRENTLY

The internal SQL queries used by REFRESH MATERIALIZED VIEW CONCURRENTLY
include some aliases for its diff and temporary relations with
rather-generic names: diff, newdata, newdata2 and mv.  Depending on the
queries used for the materialized view, using CONCURRENTLY could lead to
some internal failures if the matview query and those internal aliases
conflict.

Those names have been chosen in 841c29c8.  This commit switches instead
to a naming pattern which is less likely going to cause conflicts, based
on an idea from Thomas Munro, by appending _$ to those aliases.  This is
not perfect as those new names could still conflict, but at least it has
the advantage to keep the code readable and simple while reducing the
likelihood of conflicts to be close to zero.

Reported-by: Mathis Rudolf
Author: Bharath Rupireddy
Reviewed-by: Bernd Helmle, Thomas Munro, Michael Paquier
Discussion: https://postgr.es/m/109c267a-10d2-3c53-b60e-720fcf44d9e8@credativ.de
Backpatch-through: 9.6

doc: Group options in pg_amcheck reference page

The previous arrangement was just one big list, and the internal order
was not all consistent either.  Now arrange the options by group and
sort them, the way it's already done in the --help output and one
other reference pages.  Also fix some ordering in the --help output.

Standardize usages of appendStringInfo and appendPQExpBuffer

Fix a few places that were using appendStringInfo() when they should have
been using appendStringInfoString().  Also some cases of
appendPQExpBuffer() that would have been better suited to use
appendPQExpBufferChar(), and finally, some places that used
appendPQExpBuffer() when appendPQExpBufferStr() would have suited better.

There are no bugs are being fixed here.  The aim is just to make the code
use the most optimal function for the job.

All the code being changed here is new to PG14.  It makes sense to fix
these before we branch for PG15.  There are a few other places that we
could fix, but those cases are older code so fixing those seems less
worthwhile as it may cause unnecessary back-patching pain in the future.

Author: Hou Zhijie
Discussion: https://postgr.es/m/OS0PR01MB5716732158B1C4142C6FE375943D9@OS0PR01MB5716.jpnprd01.prod.outlook.com

Ignore more environment variables in TAP tests

Various environment variables were not getting reset in the TAP tests,
which would cause failures depending on the tests or the environment
variables involved.  For example, PGSSL{MAX,MIN}PROTOCOLVERSION could
cause failures in the SSL tests.  Even worse, a junk value of
PGCLIENTENCODING makes a server startup fail.  The list of variables
reset is adjusted in each stable branch depending on what is supported.

While on it, simplify a bit the code per a suggestion from Andrew
Dunstan, using a list of variables instead of doing single deletions.

Reviewed-by: Andrew Dunstan, Daniel Gustafsson
Discussion: https://postgr.es/m/YLbjjRpucIeZ78VQ@paquier.xyz
Backpatch-through: 9.6

Re-allow custom GUC names that have more than two components.

Commit 3db826bd5 disallowed this case, but it turns out that some
people are depending on it.  Since the core grammar has allowed
it since 3dc37cd8d, it seems like this code should fall in line.

Per bug #17045 from Robert Sosinski.

Discussion: https://postgr.es/m/17045-6a4a9f0d1513f72b@postgresql.org

Revert most of 39b66a91bd

Reverts most of commit 39b66a91bd, which was found to cause significant
regression for REFRESH MATERIALIZED VIEW. This means only rows inserted
by heap_multi_insert will benefit from the optimization, implemented in
commit 7db0cd2145.

Reported-by: Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoA%3D%3Df2VSw3c-Cp_y%3DWLKHMKc1D6s7g3YWsCOvgaYPpJcg%40mail.gmail.com

Fix planner's row-mark code for inheritance from a foreign table.

Commit 428b260f8 broke planning of cases where row marks are needed
(SELECT FOR UPDATE, etc) and one of the query's tables is a foreign
table that has regular table(s) as inheritance children.  We got the
reverse case right, but apparently were thinking that foreign tables
couldn't be inheritance parents.  Not so; so we need to be able to
add a CTID junk column while adding a new child, not only a wholerow
junk column.

Back-patch to v12 where the faulty code came in.

Amit Langote

Discussion: https://postgr.es/m/CA+HiwqEmo3FV1LAQ4TVyS2h1WM=kMkZUmbNuZSCnfHvMcUcPeA@mail.gmail.com

Update plannodes.h's comments about PlanRowMark.

The reference here to different physical column numbers in inherited
UPDATE/DELETE plans is obsolete as of 86dc90056; remove it.  Also
rework the text about inheritance cases to make it clearer.

Teach tab-complete.c about recently-added CREATE TYPE options.

Commit c7aba7c14 missed adding SUBSCRIPT here,
and commit 6df7a9698 missed adding MULTIRANGE_TYPE_NAME.

Haiying Tang and Tom Lane

Discussion: https://postgr.es/m/OS0PR01MB6113F9EDA46FA53BAA5445BDFB3D9@OS0PR01MB6113.jpnprd01.prod.outlook.com

Remove unnecessary use of Time::HiRes from 013_crash_restart.pl.

The regression test 013_crash_restart.pl included "use Time::HiRes qw(usleep)",
but the usleep was not used there.

Author: Fujii Masao
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/63ad1368-18e2-8900-8443-524bdfb1bef5@oss.nttdata.com

Add regression test for recovery pause.

Previously there was no regression test for recovery pause feature.
This commit adds the test that checks

- recovery can be paused or resumed expectedly
- pg_get_wal_replay_pause_state() reports the correct pause state
- the paused state ends and promotion continues if a promotion
   is triggered while recovery is paused

Suggested-by: Michael Paquier
Author: Fujii Masao
Reviewed-by: Kyotaro Horiguchi, Dilip Kumar
Discussion: https://postgr.es/m/YKNirzqM1HYyk5h4@paquier.xyz

Add Windows file version information to libpq_pipeline.exe.

Fix missing gettimeofday() declaration.

This avoids a warning under MinGW versions having gettimeofday(), per
buildfarm member walleye.

Reject SELECT ... GROUP BY GROUPING SETS (()) FOR UPDATE.

This case should be disallowed, just as FOR UPDATE with a plain
GROUP BY is disallowed; FOR UPDATE only makes sense when each row
of the query result can be identified with a single table row.
However, we missed teaching CheckSelectLocking() to check
groupingSets as well as groupClause, so that it would allow
degenerate grouping sets.  That resulted in a bad plan and
a null-pointer dereference in the executor.

Looking around for other instances of the same bug, the only one
I found was in examine_simple_variable().  That'd just lead to
silly estimates, but it should be fixed too.

Per private report from Yaoguang Chen.
Back-patch to all supported branches.