Fix check for conflicting SSL min/max protocol settings

Commit 79dfa8a has introduced a check to catch when the minimum protocol
version was set higher than the maximum version, however an error was
getting generated when both bounds are set even if they are able to
work, causing a backend to not use a new SSL context but keep the old
one.

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/14BFD060-8C9D-43B4-897D-D5D9AA6FC92B@yesql.se

Fix checkpoint signalling

Checkpointer uses its MyLatch to wake up when a checkpoint request is
received.  But before commit c6550776394e the latch was not used for
anything else, so the code could just go to sleep after each loop
without rechecking the sleeping condition.  That commit added a separate
ResetLatch in its code path[1], which can cause a checkpoint to go
unnoticed for potentially a long time.

Fix by skipping sleep if any checkpoint flags are set.  Also add a test
to verify this; authored by Kyotaro Horiguchi.

[1] CreateCheckPoint -> InvalidateObsoleteReplicationSlots ->
ConditionVariableTimeSleep

Report and diagnosis by Kyotaro Horiguchi.
Co-authored-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20200408.141956.891237856186513376.horikyota.ntt@gmail.com

Fix typo

from 927474ce1a2

Check slot->restart_lsn validity in a few more places

Lack of these checks could cause visible misbehavior, including
assertion failures.  This was missed in commit c6550776394e, whereby
restart_lsn becomes invalid when the size limit is exceeded.

Also reword some existing error messages, and add errdetail(), so that
the reported errors all match in spirit.

Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20200408.093710.447591748588426656.horikyota.ntt@gmail.com

Add LP_DEAD deletion of a posting list tuple test.

Make sure that we have test coverage of the posting list tuple path
within _bt_xid_horizon().

Per off-list complaint from Andres Freund.

Add missing gettext triggers

Some translatable strings have been moved to scanner_yyerror(), so we
need to add that, too.

Fix definition of pg_statio_all_tables view

pg_statio_all_tables view appears to have a wrong grouping.  As the result
numbers of toast index blocks read and hit were multiplied to the number of
table indexes.  This commit fixes the view definition.

Backpatching this appears difficult.  We don't have a mechanism to patch a
system catalog of existing instances in minor upgrade.  We can write a
release notes instruction to do this manually.  But per discussion this is
probably not so critical bug for doing such an intrusive fix.

Reported-by: Andrei Zubkov
Discussion: https://postgr.es/m/CAPpHfdtMYkkNudLMG9G0dxX_B%3Dn5sfKzOyxxrvWYtSicaGW0Lw%40mail.gmail.com

Add more TAP coverage for archive status with crash recovery of standbys

This part of the test was included originally in 4e87c48, but got
reverted as of f9c1b8d because of timing issues in the test, where,
after more analysis, we found that the standbys may not have recovered
from the archives all the segments needed by the test.  This stabilizes
the test by making sure that standbys replay up to the position returned
by pg_switch_wal(), meaning that all segments are recovered before the
manual checkpoint that tests WAL segment recycling and its interactions
with archive status files.

Author: Jehan-Guillaume de Rorthais
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/20200424034248.GL33034@paquier.xyz

Fix bogus tar-file padding logic for standby.signal.

When pg_basebackup -R is used, we inject standby.signal into the
tar file for the main tablespace. The proper thing to do is to pad
each file injected into the tar file out to a 512-byte boundary
by appending nulls, but here the file is of length 0 and we add
511 zero bytes.  Since 0 is already a multiple of 512, we should
not add any zero bytes. Do that instead.

Patch by me, reviewed by Tom Lane.

Discussion: http://postgr.es/m/CA+TgmobWbfReO9-XFk8urR1K4wTNwqoHx_v56t7=T8KaiEoKNw@mail.gmail.com

Fix full text search to handle NOT above a phrase search correctly.

Queries such as '!(foo<->bar)' failed to find matching rows when
implemented as a GiST or GIN index search.  That's because of
failing to handle phrase searches as tri-valued when considering
a query without any position information for the target tsvector.
We can only say that the phrase operator might match, not that it
does match; and therefore its NOT also might match.  The previous
coding incorrectly inverted the approximate phrase result to
decide that there was certainly no match.

To fix, we need to make TS_phrase_execute return a real ternary result,
and then bubble that up accurately in TS_execute.  As long as we have
to do that anyway, we can simplify the baroque things TS_phrase_execute
was doing internally to manage tri-valued searching with only a bool
as explicit result.

For now, I left the externally-visible result of TS_execute as a plain
bool.  There do not appear to be any outside callers that need to
distinguish a three-way result, given that they passed in a flag
saying what to do in the absence of position data.  This might need
to change someday, but we wouldn't want to back-patch such a change.

Although tsginidx.c has its own TS_execute_ternary implementation for
use at upper index levels, that sadly managed to get this case wrong
as well :-(.  Fixing it is a lot easier fortunately.

Per bug #16388 from Charles Offenbacher.  Back-patch to 9.6 where
phrase search was introduced.

Discussion: https://postgr.es/m/16388-98cffba38d0b7e6e@postgresql.org

Doc: render &pi; more nicely in PDF output.

We need to select symbol font explicitly, or it comes out misaligned.

Alexander Lakhin, Tom Lane

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

pg_dump: Replace can't-happen error with assertion

Fix some typos

Author: Justin Pryzby
Discussion: https://postgr.es/m/20200408165653.GF2228@telsasoft.com

Doc: improve documentation of websearch_to_tqsuery().

It wasn't totally clear about punctuation other than what's
specified being ignored.

Pavel Borisov and Tom Lane

Discussion: https://postgr.es/m/CALT9ZEFsBdsogVjG40Z4KfM1Um=wj1FE9hJ00GK3oVfzz0sFNg@mail.gmail.com

Fix typo

from 303640199d0

Raise a timeout to 180s, in test 003_recovery_targets.pl.

Buildfarm member chipmunk has failed twice due to taking >30s, and
twenty-four runs of other members have used >5s.  The test is new in
v13, so no back-patch.

Fix another minor page deletion buffer lock issue.

Avoid accessing the leaf page's top parent tuple without a buffer lock
held during the second phase of nbtree page deletion.  The old approach
was safe, though only because VACUUM never drops its buffer pin (and
because only VACUUM itself can modify a half-dead page).  Even still, it
seems like a good idea to be strict here.  Tighten things up by copying
the top parent page's block number to a local variable before releasing
the buffer lock on the leaf page -- not after.

This is a follow-up to commit fa7ff642, which fixed a similar issue in
the first phase of nbtree page deletion.

Update some related comments in passing.

Discussion: https://postgr.es/m/CAH2-WzkLgyN3zBvRZ1pkNJThC=xi_0gpWRUb_45eexLH1+k2_Q@mail.gmail.com

Fix minor nbtree page deletion buffer lock issue.

Avoid accessing the deletion target page's special area during nbtree
page deletion at a point where there is no buffer lock held.  This issue
was detected by a patch that extends Valgrind's memcheck tool to mark
nbtree pages that are unsafe to access (due to not having a buffer lock
or buffer pin) as NOACCESS.

We do hold a buffer pin at this point, and only access the special area,
so the old approach was safe.  Even still, it seems like a good idea to
tighten up the rules in this area.  There is no reason to not simply
insist on always holding a buffer lock (not just a pin) when accessing
nbtree pages.

Update some related comments in passing.

Discussion: https://postgr.es/m/CAH2-WzkLgyN3zBvRZ1pkNJThC=xi_0gpWRUb_45eexLH1+k2_Q@mail.gmail.com

In caught-up logical walsender, sleep only in WalSndWaitForWal().

Before sleeping, WalSndWaitForWal() sends a keepalive if MyWalSnd->write
< sentPtr.  When the latest physical LSN yields no logical replication
messages (a common case), that keepalive elicits a reply.  Processing
the reply updates pg_stat_replication.replay_lsn.  WalSndLoop() lacks
that; when WalSndLoop() slept, replay_lsn advancement could stall until
wal_receiver_status_interval elapsed.  This sometimes stalled
src/test/subscription/t/001_rep_changes.pl for up to 10s.

Reviewed by Fujii Masao and Michael Paquier.

Discussion: https://postgr.es/m/20200418070142.GA1075445@rfd.leadboat.com

Revert "When WalSndCaughtUp, sleep only in WalSndWaitForWal()."

This reverts commit 421685812290406daea58b78dfab0346eb683bbb.  It caused
idle physical walsenders to busy-wait, as reported by Fujii Masao.

Discussion: https://postgr.es/m/20200417054146.GA1061007@rfd.leadboat.com

Fix error case for CREATE ROLE ... IN ROLE.

CreateRole() was passing a Value node, not a RoleSpec node, for the
newly-created role name when adding the role as a member of existing
roles for the IN ROLE syntax.

This mistake went unnoticed because the node in question is used only
for error messages and is not accessed on non-error paths.

In older pg versions (such as 9.5 where this was found), this results
in an "unexpected node type" error in place of the real error. That
node type check was removed at some point, after which the code would
accidentally fail to fail on 64-bit platforms (on which accessing the
Value node as if it were a RoleSpec would be mostly harmless) or give
an "unexpected role type" error on 32-bit platforms.

Fix the code to pass the correct node type, and add an lfirst_node
assertion just in case.

Per report on irc from user m1chelangelo.

Backpatch all the way, because this error has been around for a long
time.

Update Windows timezone name list to include currently-known zones.

Thanks to Juan José Santamaría Flecha.

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

Improve placement of "display name" comment in win32_tzmap[] entries.

Sticking this comment at the end of the last line was a bad idea: it's
not particularly readable, and it tempts pgindent to mess with line
breaks within the comment, which in turn reveals that win32tzlist.pl's
clean_displayname() does the wrong thing to clean up such line breaks.
While that's not hard to fix, there's basically no excuse for this
arrangement to begin with, especially since it makes the table layout
needlessly vary across back branches with different pgindent rules.
Let's just put the comment inside the braces, instead.

This commit just moves and reformats the comments, and updates
win32tzlist.pl to match; there's no actual data change.

Per odd-looking results from Juan José Santamaría Flecha.
Back-patch, since the point is to make win32_tzmap[] look the
same in all supported branches again.

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

Doc: update section 9.13 for new function table layout.

This includes the usual amount of editorial cleanup, such as
correcting wrong or less-helpful-than-they-could-be examples.

I moved the two tsvector-updating triggers into "9.28 Trigger
Functions", which seems like a better home for them.  (I believe
that section didn't exist when this text was originally written.)

git_changelog: use modern format for rel branch names in example

e.g., REL_12_STABLE

Try to avoid compiler warnings in optimized builds.

Per report from Andres Freund, who also says that this fix
works for him.

Discussion: http://postgr.es/m/20200405193118.alprgmozhxcfabkw@alap3.anarazel.de

Repair performance regression in information_schema.triggers view.

Commit 32ff26911 introduced use of rank() into the triggers view to
calculate the spec-mandated action_order column.  As written, this
prevents query constraints on the table-name column from being pushed
below the window aggregate step.  That's bad for performance of this
typical usage pattern, since the view now has to be evaluated for all
tables not just the one(s) the user wants to see.  It's also the cause
of some recent buildfarm failures, in which trying to evaluate the view
rows for triggers in process of being dropped resulted in "cache lookup
failed for function NNN" errors.  Those rows aren't of interest to the
test script doing the query, but the filter that would eliminate them
is being applied too late.  None of this happened before the rank()
call was there, so it's a regression compared to v10 and before.

We can improve matters by changing the rank() call so that instead of
partitioning by OIDs, it partitions by nspname and relname, casting
those to sql_identifier so that they match the respective view output
columns exactly.  The planner has enough intelligence to know that
constraints on partitioning columns are safe to push down, so this
eliminates the performance problem and the regression test failure
risk.  We could make the other partitioning columns match view outputs
as well, but it'd be more complicated and the performance benefits
are questionable.

Side note: as this stands, the planner will push down constraints on
event_object_table and trigger_schema, but not on event_object_schema,
because it checks for ressortgroupref matches not expression
equivalence.  That might be worth improving someday, but it's not
necessary to fix the immediate concern.

Back-patch to v11 where the rank() call was added.  Ordinarily we'd not
change information_schema in released branches, but the test failure has
been seen in v12 and presumably could happen in v11 as well, so we need
to do this to keep the buildfarm happy.  The change is harmless so far
as users are concerned.  Some might wish to apply it to existing
installations if performance of this type of query is of concern,
but those who don't are no worse off.

I bumped catversion in HEAD as a pro forma matter (there's no
catalog incompatibility that would really require a re-initdb).
Obviously that can't be done in the back branches.

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

Update time zone data files to tzdata release 2020a.

DST law changes in Morocco and the Canadian Yukon.
Historical corrections for Shanghai.

The America/Godthab zone is renamed to America/Nuuk to reflect
current English usage; however, the old name remains available as a
compatibility link.

Update Unicode data to Unicode 13.0.0 and CLDR 37

Remove some unstable parts from new TAP test for archive status check

The test is proving to have timing issues when looking at archive status
files on standbys after crash recovery, while other parts of the test
rely on pg_stat_archiver as a wait point to make sure that a given state
of the archiving is reached.  The coverage is not heavily impacted by
the removal those extra tests.

Per reports from several buildfarm animals, like crake, piculet,
culicidae and francolin.

Discussion: https://postgr.es/m/20200424005929.GK33034@paquier.xyz
Backpatch-through: 9.5

Fix handling of WAL segments ready to be archived during crash recovery

78ea8b5 has fixed an issue related to the recycling of WAL segments on
standbys depending on archive_mode.  However, it has introduced a
regression with the handling of WAL segments ready to be archived during
crash recovery, causing those files to be recycled without getting
archived.

This commit fixes the regression by tracking in shared memory if a live
cluster is either in crash recovery or archive recovery as the handling
of WAL segments ready to be archived is different in both cases (those
WAL segments should not be removed during crash recovery), and by using
this new shared memory state to decide if a segment can be recycled or
not.  Previously, it was not possible to know if a cluster was in crash
recovery or archive recovery as the shared state was able to track only
if recovery was happening or not, leading to the problem.

A set of TAP tests is added to close the gap here, making sure that WAL
segments ready to be archived are correctly handled when a cluster is in
archive or crash recovery with archive_mode set to "on" or "always", for
both standby and primary.

Reported-by: Benoît Lobréau
Author: Jehan-Guillaume de Rorthais
Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/20200331172229.40ee00dc@firost
Backpatch-through: 9.5

Remove ACLDEBUG #define and associated code.

In the footsteps of aaf069aa3, remove ACLDEBUG, which was the only
other remaining undocumented symbol in pg_config_manual.h.  The fact
that nobody had bothered to document it in seventeen years is a good
clue to its usefulness.  In practice, none of the tracing logic it
enabled would be of any value without additional effort.

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

Remove useless (and broken) logging logic in memory context functions.

Nobody really uses this stuff, especially not since we created
valgrind-based infrastructure that does the same thing better.
It is thus unsurprising that the generation.c and slab.c versions
were actually broken.  Rather than fix 'em, let's just remove 'em.

Alexander Lakhin

Discussion: https://postgr.es/m/8936216c-3492-3f6e-634b-d638fddc5f91@gmail.com

Doc: update section 9.12 for new function table layout.

Also rearrange that page a bit for more consistency and less
duplication.

In passing, fix erroneous examples of the results of abbrev(cidr)
in datatype.sgml, and do a bit of copy-editing there.

Also rename 'struct manifest_info'.

The previous commit renamed the struct's typedef, but not the struct
name itself.

Rename exposed identifiers to say "backup manifest".

Function names declared "extern" now use BackupManifest in the name
rather than just Manifest, and data types use backup_manifest
rather than just manifest.

Per note from Michael Paquier.

Discussion: http://postgr.es/m/20200418125713.GG350229@paquier.xyz

Fix transient memory leak for SRFs in FROM.

In a9c35cf85ca I changed ExecMakeTableFunctionResult() to dynamically
allocate the FunctionCallInfo used to call the SRF. Unfortunately I
did not account for the fact that the surrounding memory context has
query lifetime, leading to a leak till the end of the query.

In most cases the leak is fairly inconsequential, but if the
FunctionScan is done many times in the query, the leak can add
up. This happens e.g. if the function scan is on the inner side of a
nested loop, due to a lateral join.
EXPLAIN SELECT sum(f) FROM generate_series(1, 100000000) g(i), generate_series(i, i+1) f;
quickly shows the leak.

Instead of explicitly freeing the FunctionCallInfo it seems better to
make sure all the per-set temporary state in
ExecMakeTableFunctionResult() is cleaned up wholesale. Currently
that's probably just the FunctionCallInfo allocation, but since
there's some initialization work, and since there's already an
appropriate context, this seems like a more robust approach.

Bug: #16112
Reported-By: Ben Cornett
Author: Andres Freund
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/16112-4448bbf55a404189%40postgresql.org
Backpatch: 12, a9c35cf85ca

Fix option related issues in pg_verifybackup.

This commit does:

- get rid of the garbage code for unused --print-parse-wal option.
- add help message for --quiet option into usage().
- fix typo of option name in help message.

Author: Fujii Masao
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/ff4710f7-2331-4f6b-012e-d76da3275e91@oss.nttdata.com

Doc: improve description of geometric multiplication/division.

David Johnston reminded me that the per-point calculations being done
by these operators are equivalent to complex multiplication/division.
(Once I would've recognized that immediately, but it's been too long
since I did any of that sort of math.)

Also put in a footnote mentioning that "rotation" of a box doesn't do
what you might expect, as I'd griped about in the referenced thread.

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

nbtree: Rename BT_RESERVED_OFFSET_MASK.

The mask was added by commit 8224de4f, which introduced INCLUDE nbtree
indexes.  The status bits really were reserved initially.  We now use 2
out of 4 of the bits for additional tuple metadata, though.  Rename the
mask to BT_STATUS_OFFSET_MASK.

Also consolidate related nbtree.h code comments about the format of
pivot tuples and posting list tuples.

Fix cost_incremental_sort for expressions with varno 0

When estimating the number of pre-sorted groups in cost_incremental_sort
we must not pass Vars with varno 0 to estimate_num_groups, which would
cause failues in find_base_rel. This may happen when sorting output of
set operations, thanks to generate_append_tlist.

Unlike recurse_set_operations we can't easily access the original target
list, so if we find any Vars with varno 0, we fall back to the default
estimate DEFAULT_NUM_DISTINCT.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20200411214639.GK2228%40telsasoft.com

docs:  land height is "elevation", not "altitude"

See https://mapscaping.com/blogs/geo-candy/what-is-the-difference-between-elevation-relief-and-altitude
No patching of regression tests.

Reported-by: taf1@cornell.edu
Discussion: https://postgr.es/m/158506544539.679.2278386310645558048@wrigleys.postgresql.org

Backpatch-through: 9.5

Sync up some inconsistent comments in config/c-compiler.m4.

Make header/trailer comments agree with the actual names of some macros.
These seem like legit names in earlier iterations of respective patches
(commit b779168ff "Detect PG_PRINTF_ATTRIBUTE automatically." and
commit 6869b4f25 "Add C++ support to configure.") but the macro had
been renamed out of sync with the header / trailer comment in the final
committed patch.

Even more nitpickily, make the dashed underlines agree with the lengths
of the macro names everyplace.  There doesn't seem to have been any
meeting of the minds previously on whether those should match or not,
but at least some people have been trying to make 'em match.

Jesse Zhang, Tom Lane

Discussion: https://postgr.es/m/CAGf+fX7DDyq6WfCy6X_KtD28MkbNBE6NkRi26fSf25dfUwX0zw@mail.gmail.com

Doc: update section 9.11 for new function table layout.

This also makes an attempt to flesh out the docs for some of the more
severely underdocumented geometric operators and functions.

This effort exposed that the point <^ point (point_below) and
point >^ point (point_above) operators are misnamed; they should be
<<| and |>>, because they act like the other operators named that
way and not like the other operators named <^ and >^.  But I just
documented them that way; fixing it is matter for another patch.

The haphazard datatype coverage of many of the operators is also
now depressingly obvious.

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

Remove bogus Assert in foreign key cloning code

This Assert was trying to ensure that the number of columns in the foreign
key being cloned was the same number of attributes in the parentRel.  Of
course, it's perfectly valid to have columns in the table which are not
part of the foreign key constraint. It appears that this Assert was
misunderstanding that.

Reported-by: Rajkumar Raghuwanshi
Reviewed-by: amul sul
Discussion: https://postgr.es/m/CAKcux6=z1dtiWw5BOpqDx-U6KTiq+zD0Y2m810zUtWL+giVXWA@mail.gmail.com

Remove HEAPDEBUGALL

This has been broken since PostgreSQL 12 and was probably never really
used.  PostgreSQL 12 added an analogous HEAPAMSLOTDEBUGALL, which
still works right now, but it's also not very useful, so remove that
as well.

Discussion: https://www.postgresql.org/message-id/flat/645c0646-4218-d4c3-409a-a7003a0c108d%402ndquadrant.com

Fix single-record reads to use restore_command if available in pg_rewind

readOneRecord() is used now when looking for a checkpoint record to
check if the target server is an ancestor of the source across multiple
timelines, and using a restore_command if available improves the
stability of the operation.  This part was missed in a7e8ece.

Reported-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20200421.150830.1410714948345179794.horikyota.ntt@gmail.com

psql \d: Display table where trigger is defined, if inherited

It's important to know that a trigger is cloned from a parent table,
because of the behavior that the trigger is dropped on detach.  Make
psql's \d display it.

We'd like to backpatch, but lack of the pg_trigger.tgparentid column
makes it more difficult.  Punt for now.  If somebody wants to volunteer
an implementation that reads pg_depend on older versions, that can
probably be backpatched.

Authors: Justin Pryzby, Amit Langote, Álvaro Herrera
Discussion: https://postgr.es/m/20200419002206.GM26953@telsasoft.com

Fix memory leak in libpq when using sslmode=verify-full

Checking if Subject Alternative Names (SANs) from a certificate match
with the hostname connected to leaked memory after each lookup done.

This is broken since acd08d7 that added support for SANs in SSL
certificates, so backpatch down to 9.5.

Author: Roman Peshkurov
Reviewed-by: Hamid Akhtar, Michael Paquier, David Steele
Discussion: https://postgr.es/m/CALLDf-pZ-E3mjxd5=bnHsDu9zHEOnpgPgdnO84E2RuwMCjjyPw@mail.gmail.com
Backpatch-through: 9.5

Document partitiong tables ancillary object handling some more

Add a couple of lines to make it explicit that indexes, constraints,
triggers are added, removed, or left alone.

Backpatch to pg11.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20200421162038.GA18628@alvherre.pgsql

Fix possible crash during FATAL exit from reindexing.

index.c supposed that it could just use a PG_TRY block to clean up the
state associated with an active REINDEX operation.  However, that code
doesn't run if we do a FATAL exit --- for example, due to a SIGTERM
shutdown signal --- while the REINDEX is happening.  And that state does
get consulted during catalog accesses, which makes it problematic if we
do any catalog accesses during shutdown --- for example, to clean up any
temp tables created in the session.

If this combination of circumstances occurred, we could find ourselves
trying to access already-freed memory.  In debug builds that'd fairly
reliably cause an assertion failure.  In production we might often
get away with it, but with some bad luck it could cause a core dump.

Another possible bad outcome is an erroneous conclusion that an
index-to-be-accessed is being reindexed; but it looks like that would
be unlikely to have any consequences worse than failing to drop temp
tables right away.  (They'd still get dropped by the next session that
uses that temp schema.)

To fix, get rid of the use of PG_TRY here, and instead hook into
the transaction abort mechanisms to clean up reindex state.

Per bug #16378 from Alexander Lakhin.  This has been wrong for a
very long time, so back-patch to all supported branches.

Discussion: https://postgr.es/m/16378-7a70ca41b3ec2009@postgresql.org

Fix minor violations of FunctionCallInvoke usage protocol.

Working on commit 1c455078b led me to check through FunctionCallInvoke
call sites to see if every one was being honest about (a) making sure
that fcinfo.isnull is initially false, and (b) checking its state after
the call.  Sure enough, I found some violations.

The main one is that finalize_partialaggregate re-used serialfn_fcinfo
without resetting isnull, even though it clearly intends to cater for
serialfns that return NULL.  There would only be an issue with a
non-strict serialfn, since it's unlikely that a serialfn would return
NULL for non-null input.  We have no non-strict serialfns in core, and
there may be none in the wild either, which would account for the lack
of complaints.  Still, it's clearly wrong, so back-patch that fix to
9.6 where finalize_partialaggregate was introduced.

Also, arrayfuncs.c and rowtypes.c contained various callers that were
not bothering to check for result nulls.  While what's being called is
a comparison or hash function that probably *shouldn't* return null,
that's a lousy excuse for not having any check at all.  There are
existing places that just Assert(!fcinfo->isnull) in comparable
situations, so I added that to the places that were calling btree
comparison or hash support functions.  In the places calling
boolean-returning equality functions, it's quite cheap to have them
treat isnull as FALSE, so make those places do that.  Also remove some
"locfcinfo->isnull = false" assignments that are unnecessary given the
assumption that no previous call returned null.  These changes seem like
mostly neatnik-ism or debugging support, so I didn't back-patch.

Fix detaching partitions with cloned row triggers

When a partition is detached, any triggers that had been cloned from its
parent were not properly disentangled from its parent triggers.
This resulted in triggers that could not be dropped because they
depended on the trigger in the trigger in the no-longer-parent table:
  ALTER TABLE t DETACH PARTITION t1;
  DROP TRIGGER trig ON t1;
    ERROR:  cannot drop trigger trig on table t1 because trigger trig on table t requires it
    HINT:  You can drop trigger trig on table t instead.

Moreover the table can no longer be re-attached to its parent, because
the trigger name is already taken:
  ALTER TABLE t ATTACH PARTITION t1 FOR VALUES FROM (1)TO(2);
    ERROR:  trigger "trig" for relation "t1" already exists

The former is a bug introduced in commit 86f575948c77.  (The latter is
not necessarily a bug, but it makes the bug more uncomfortable.)

To avoid the complexity that would be needed to tell whether the trigger
has a local definition that has to be merged with the one coming from
the parent table, establish the behavior that the trigger is removed
when the table is detached.

Backpatch to pg11.

Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/20200408152412.GZ2228@telsasoft.com

Consider outliers in split interval calculation.

Commit 0d861bbb, which introduced deduplication to nbtree, added some
logic to take large posting list tuples into account when choosing a
split point.  We subtract firstright posting list overhead from the
projected new high key size when calculating leftfree/rightfree values
for an affected candidate split point.  Posting list tuples aren't
special to nbtsplitloc.c, but taking them into account like this makes a
huge difference in practice.  Posting list tuples are frequently tuple
size outliers.

However, commit 0d861bbb missed a closely related issue: split interval
itself is calculated based on the assumption that tuples on the page
being split are roughly equisized.  That assumption was acceptable back
when commit fab25024 taught the logic for choosing a split point about
suffix truncation, but it's pretty questionable now that very large
tuple sizes are common.  This oversight led to unbalanced page splits in
low cardinality multi-column indexes when deduplication was used: page
splits that don't give sufficient weight to how unbalanced the split is
when the interval happens to include some large posting list tuples (and
when most other tuples on the page are not so large).

Nail this down by calculating an initial split interval in a way that's
attuned to the actual cost that we want to keep under control (not a
fuzzy proxy for the cost): apply a leftfree + rightfree evenness test to
each candidate split point that actually gets included in the split
interval (for the default strategy).  This replaces logic that used a
percentage of all legal split points for the page as the basis of the
initial split interval.

Discussion: https://postgr.es/m/CAH2-WznJt5aT2uUB2Bs+JBLdwe0XTX67+xeLFcaNvCKxO=QBVQ@mail.gmail.com

Allow matchingsel() to be used with operators that might return NULL.

Although selfuncs.c will never call a target operator with null inputs,
some functions might return null anyway.  The existing coding will fail
if that happens (since FunctionCall2Coll will punt), which seems
undesirable given that matchingsel() has such a broad range of potential
applicability --- in fact, we already have a problem because we apply it
to jsonb_path_exists_opr, which can return null.  Hence, rejigger the
underlying functions mcv_selectivity and histogram_selectivity to cope,
treating a null result as false.

While we are at it, we can move the InitFunctionCallInfoData overhead
out of the inner loops, which isn't a huge number of cycles but might
save something considering we are likely calling functions as cheap
as int4eq().  Plus, the number of loop cycles to be expected is much
more than it was when this code was written, since typical settings
of default_statistics_target are higher.

In view of that consideration, let's apply the same change to
var_eq_const, eqjoinsel_inner, and eqjoinsel_semi.  We do not expect
equality functions to ever return null for non-null inputs (and
certainly that code has been that way a long time without complaints),
but the cycle savings seem attractive, especially in the eqjoinsel loops
where there's potentially an O(N^2) savings.

Similar code exists in ineq_histogram_selectivity and
get_variable_range, but I forebore from changing those for now.
The performance argument for changing ineq_histogram_selectivity
is really weak anyway, since that will only iterate log2(N) times.

Nikita Glukhov and Tom Lane

Discussion: https://postgr.es/m/9d3b0959-95d6-c37e-2c0b-287bcfe5c705@postgrespro.ru

Clean up cpluspluscheck violation.

"operator" is a reserved word in C++, so per project conventions,
don't use it as an identifier in header files.

My oversight in commit a80818605.

Fix duplicate typedef from commit 0d8c9c121.

Older gcc versions don't like duplicate typedefs, so get rid of
that in favor of doing it like we do it elsewhere, ie just use
a "struct" declaration when trying to avoid importing a whole
header file.

Also, there seems no reason to include stringinfo.h here at all,
so get rid of that addition too.

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

Mention pg_promote() as a method to trigger promotion in documentation.

Previously in the "Standby Server Operation" section, pg_ctl promote and
protmote_trigger_file were documented as a method to trigger standby
promotion, but pg_promote() function not.

This commit also adds parentheses into <function>pg_promote</function>
in some docs to make it clearer that a function is being referred to.

Author: Masahiro Ikeda
Reviewed-by: Michael Paquier, Laurenz Albe, Tom Lane, Fujii Masao
Discussion: https://postgr.es/m/de0068417a9f4046bac693cbcc00bdc9@oss.nttdata.com

doc:  change SGML markup "figure" to "example"

Reported-by: Jürgen Purtz
Discussion: https://postgr.es/m/709d7809-d7f4-8175-47f3-4d131341bba8@purtz.de

Author: Jürgen Purtz

Backpatch-through: 9.5

Doc: update sections 9.7 and 9.8 for new function table layout.

Also some mop-up in section 9.9.

Move the server's backup manifest code to a separate file.

basebackup.c is already a pretty big and complicated file, so it
makes more sense to keep the backup manifest support routines
in a separate file, for clarity and ease of maintenance.

Discussion: http://postgr.es/m/CA+TgmoavRak5OdP76P8eJExDYhPEKWjMb0sxW7dF01dWFgE=uA@mail.gmail.com

Add tab-completion for ALTER INDEX .. [NO] DEPENDS ON

... as added in the prior commit.

(We'd like to have tab-completion for the other object types too, but
they don't have sub-command completion yet.)

Author: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CALtqXTcogrFEVP9uou5vFtnGsn+vHZUu9+9a0inarfYVOHScYQ@mail.gmail.com

Add ALTER .. NO DEPENDS ON

Commit f2fcad27d59c (9.6 era) added the ability to mark objects as
dependent an extension, but forgot to add a way for such dependencies to
be removed.  This commit fixes that oversight.

Strictly speaking this should be backpatched to 9.6, but due to lack of
demand we're not doing so at this time.

Discussion: https://postgr.es/m/20200217225333.GA30974@alvherre.pgsql
Reviewed-by: ahsan hadi <ahsan.hadi@gmail.com>
Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>

Doc: update sections 9.5 and 9.6 for new function table layout.

Along the way, update the older examples for bytea to use "hex"
output format.  That lets us get rid of the lame disclaimer about
how the examples assume bytea_output = escape, which was only half
true anyway because none of the more-recently-added examples had
paid any attention to that.

Allow pg_read_all_stats to access all stats views again

The views pg_stat_progress_* had not gotten the memo that
pg_read_all_stats is supposed to be able to read all statistics. Also
make a pass over all text-returning pg_stat_xyz functions that could
return "insufficient privilege" and make sure they also respect
pg_read_all_status.

Reported-by: Andrey M. Borodin
Reviewed-by: Andrey M. Borodin, Kyotaro Horiguchi
Discussion: https://postgr.es/m/13145F2F-8458-4977-9D2D-7B2E862E5722@yandex-team.ru

Doc: update the rest of section 9.4 for new function table layout.

Notably, this replaces the previous handwaving about these functions'
behavior with "character"-type inputs with some actual facts.

Fix missing pfree() in logtape.c, missed by 24d85952.

Doc: update sections 9.1-9.3 for new function table layout.

I took the opportunity to do some copy-editing in this area as well,
and to add some new material such as a note about BETWEEN's syntactical
peculiarities.

Of note is that quite a few of the examples of transcendental functions
needed to be updated, because the displayed output no longer matched
what you get on a modern server.  I believe some of these cases are
side-effects of the new Ryu algorithm in float8out.  Others appear to be
because the examples predate the addition of type numeric, and were
expecting that float8 calculations would be done although the given
syntax would actually lead to calling the numeric function nowadays.

Fix update-unicode target

The normalization-check target needs to be run last, after moving the
newly generated files into place.  Also, we need an additional
dependency so that unicode_norm.o is rebuilt first.  Otherwise,
norm_test will still test the old files but against the new expected
results, which will probably fail.

Doc: sync functableentry markup choices with website style.

Jonathan Katz felt that slightly different indentation settings made
for a better-looking result, so sync stylesheet-fo.xsl (for PDF) and
stylesheet.css (for non-website-style HTML) with those choices.

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

Fix race conditions in synchronous standby management.

We have repeatedly seen the buildfarm reach the Assert(false) in
SyncRepGetSyncStandbysPriority.  This apparently is due to failing to
consider the possibility that the sync_standby_priority values in
shared memory might be inconsistent; but they will be whenever only
some of the walsenders have updated their values after a change in
the synchronous_standby_names setting.  That function is vastly too
complex for what it does, anyway, so rewriting it seems better than
trying to apply a band-aid fix.

Furthermore, the API of SyncRepGetSyncStandbys is broken by design:
it returns a list of WalSnd array indexes, but there is nothing
guaranteeing that the contents of the WalSnd array remain stable.
Thus, if some walsender exits and then a new walsender process
takes over that WalSnd array slot, a caller might make use of
WAL position data that it should not, potentially leading to
incorrect decisions about whether to release transactions that
are waiting for synchronous commit.

To fix, replace SyncRepGetSyncStandbys with a new function
SyncRepGetCandidateStandbys that copies all the required data
from shared memory while holding the relevant mutexes.  If the
associated walsender process then exits, this data is still safe to
make release decisions with, since we know that that much WAL *was*
sent to a valid standby server.  This incidentally means that we no
longer need to treat sync_standby_priority as protected by the
SyncRepLock rather than the per-walsender mutex.

SyncRepGetSyncStandbys is no longer used by the core code, so remove
it entirely in HEAD.  However, it seems possible that external code is
relying on that function, so do not remove it from the back branches.
Instead, just remove the known-incorrect Assert.  When the bug occurs,
the function will return a too-short list, which callers should treat
as meaning there are not enough sync standbys, which seems like a
reasonably safe fallback until the inconsistent state is resolved.
Moreover it's bug-compatible with what has been happening in non-assert
builds.  We cannot do anything about the walsender-replacement race
condition without an API/ABI break.

The bogus assertion exists back to 9.6, but 9.6 is sufficiently
different from the later branches that the patch doesn't apply at all.
I chose to just remove the bogus assertion in 9.6, feeling that the
probability of a bad outcome from the walsender-replacement race
condition is too low to justify rewriting the whole patch for 9.6.

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

Fix possible crash with GENERATED ALWAYS columns

In some corner cases, this could also lead to corrupted values being
included in the tuple.

Users who are concerned that they are affected by this should first
upgrade and then perform a base backup of their database and restore onto
an off-line server. They should then query each table with generated
columns to ensure there are no rows where the generated expression does
not match a newly calculated version of the GENERATED ALWAYS expression.
If no crashes occur and no rows are returned then you're not affected.

Fixes bug #16369.

Reported-by: Cameron Ezell
Discussion: https://postgr.es/m/16369-5845a6f1bef59884@postgresql.org
Backpatch-through: 12 (where GENERATED ALWAYS columns were added.)

Doc: revise formatting of function/operator tables.

The table layout ideas proposed in commit e894c6183 were not as widely
popular as I'd hoped.  After discussion, we've settled on a layout
that's effectively a single-column table with cell contents much like a
<varlistentry> description of the function or operator; though we're not
actually using <varlistentry>, because it'd add way too much vertical
space.  Instead the effect is accomplished using line-break processing
instructions to separate the description and example(s), plus CSS or FO
customizations to produce indentation of all but the first line in each
cell.  While technically this is a bit grotty, it does have the
advantage that we won't need to write nearly as much boilerplate markup.

This patch updates tables 9.30, 9.31, and 9.33 (which were touched by
the previous patch) to the revised style, and additionally converts
table 9.10.  A lot of work still remains to do, but hopefully it won't
be too controversial.

Thanks to Andrew Dunstan, Pierre Giraud, Robert Haas, Alvaro Herrera,
David Johnston, Jonathan Katz, Isaac Morland for valuable ideas.

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

Revert "Only provide new libpq sslpasskey hook for openssl-enabled builds"

This reverts commit 9e24109f1a4e4d8d1d372b004d6a0dd06e673fe7.

This caused build errors when building without openssl, and it's
simplest just to revert it.

Only provide openssl_tls_init_hook if building with openssl

This should have been protected by #ifdef USE_OPENSSL in commit
896fcdb230.

Per the real complaint this time from Daniel Gustafsson.

Use a slightly more liberal regex to detect Visual Studio version

Apparently in some language versions of Visual Studio nmake outputs some
material after the version number and before the end of the line. This
has been seen in Chinese versions. Therefore, we no longer demand that
the version string comes at the end of a line.

Per complaint from Cuiping Lin.

Backpatch to all live branches.

Only provide new libpq sslpasskey hook for openssl-enabled builds

In commit 4dc6355210 I neglected to put #ifdef USE_OPENSSL around the
declarations of the new items. This is remedied here.

Per complaint from Daniel Gustafsson.

Fix possible future cache reference leak in ALTER EXTENSION ADD/DROP.

recordExtObjInitPriv and removeExtObjInitPriv were sloppy about
calling ReleaseSysCache.  The cases cannot occur given current usage
in ALTER EXTENSION ADD/DROP, since we wouldn't get here for these
relkinds; but it seems wise to clean up better.

In passing, extend test logic in test_pg_dump to exercise the
dropped-column code paths here.

Since the case is unreachable at present, there seems no great
need to back-patch; hence fix HEAD only.

Kyotaro Horiguchi, with test case and comment adjustments by me

Discussion: https://postgr.es/m/20200417.151831.1153577605111650154.horikyota.ntt@gmail.com

Add index term for backup manifest in documentation.

Author: Fujii Masao
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/951743d0-fd7e-8e2b-d489-1368a58b7304@oss.nttdata.com

Fix minor memory leak in pg_basebackup and pg_receivewal

The result of the query used to retrieve the WAL segment size from the
backend was not getting freed in two code paths.  Both pg_basebackup and
pg_receivewal exit immediately if a failure happened on this query, so
this was not an actual problem, but it could be an issue if this code
gets used for other tools in different ways, be they future tools in
this code tree or external, existing, ones.

Oversight in commit fc49e24, so backpatch down to 11.

Author: Jie Zhang
Discussion: https://postgr.es/m/970ad9508461469b9450b64027842331@G08CNEXMBPEKD06.g08.fujitsu.local
Backpatch-through: 11

Remove unneeded constraint dependency tracking

It was previously thought that remove_useless_groupby_columns() needed to
keep track of which constraints the generated plan depended upon, however,
this is unnecessary. The confusion likely arose regarding this because of
check_functional_grouping(), which does need to track the dependency to
ensure VIEWs with columns which are functionally dependant on the GROUP BY
remain so. For remove_useless_groupby_columns(), cached plans will just
become invalidated when the primary key's underlying index is removed
through the normal relcache invalidation code.

Here we just remove the unneeded code which records the dependency and
updates the comments. The previous comments claimed that we could not use
UNIQUE constraints for the same optimization due to lack of a
pg_constraint record for NOT NULL constraints (which are required because
NULLs can be duplicated in a unique index). Since we don't actually need a
pg_constraint record to handle the invalidation, it looks like we could
add code to do this in the future. But not today.

We're not really fixing any bug in the code here, this fix is just to set
the record straight on UNIQUE constraints. This code was added back in
9.6, but due to lack of any bug, we'll not be backpatching this.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAApHDvrdYa=VhOoMe4ZZjZ-G4ALnD-xuAeUNCRTL+PYMVN8OnQ@mail.gmail.com

Fix cache reference leak in contrib/sepgsql.

fixup_whole_row_references() did the wrong thing with a dropped column,
resulting in a commit-time warning about a cache reference leak.

I (tgl) added a test case exercising this, but back-patched the test
only as far as v10; the patch didn't apply cleanly to 9.6 and it
didn't seem worth the trouble to adapt it.  The bug is pretty old
though, so apply the code change all the way back.

Michael Luo, with cosmetic improvements by me

Discussion: https://postgr.es/m/BYAPR08MB5606D1453D7F50E2AF4D2FD29AD80@BYAPR08MB5606.namprd08.prod.outlook.com

Fix the usage of parallel and full options of vacuum command.

Earlier we were inconsistent in allowing the usage of parallel and
full options.  Change it such that we disallow them only when they are
combined in a way that we don't support.

In passing, improve the comments in some of the existing tests of parallel
vacuum.

Reported-by: Tushar Ahuja
Author: Justin Pryzby, Amit Kapila
Reviewed-by: Sawada Masahiko, Michael Paquier, Mahendra Singh Thalor and
Amit Kapila
Discussion: https://postgr.es/m/58c8d171-e665-6fa3-a9d3-d9423b694dae%40enterprisedb.com

Disable silently generation of manifests with servers <= 12 in pg_basebackup

Since 0d8c9c1, pg_basebackup would generate an error if connected to a
backend version older than 12 where backup manifests are not supported.
Avoiding this error is possible by using the --no-manifest option.

This error handling could be confusing for some users, where patching a
backup script that interacts with multiple backend versions would cause
the addition of --no-manifest to potentially not generate a backup
manifest even for Postgres 13 and newer versions.  As we want to
encourage the use of backup manifests as much as possible, this commit
silently disables manifests where not supported, instead of generating
an error.

While on it, rework a bit the code to make it more consistent with the
surroundings when generating the BASE_BACKUP command.

Per discussion with Andres Freund, Stephen Frost, Robert Haas, Álvaro
Herrera, Kyotaro Horiguchi, Tom Lane, David Steele, and me.

Author: Michael Paquier
Discussion: https://postgr.es/m/20200410080910.GZ1606@paquier.xyz

Slightly simplify nbtree split point choice loop.

Spotted during post-commit review of the nbtree deduplication commit
(commit 0d861bbb).

Fix minor memory leak in pg_dump

A query used to read default ACL information from the catalogs did not
free a set of PQExpBuffer.

Oversight in commit e2090d9, so backpatch down to 9.6.

Author: Jie Zhang
Reviewed-by: Sawada Masahiko
Discussion: https://postgr.es/m/05bcbc5857f948efa0b451b85a48ae10@G08CNEXMBPEKD06.g08.fujitsu.local
Backpatch-through: 9.6

Code review for backup manifest.

This commit prevents pg_basebackup from receiving backup_manifest file
when --no-manifest is specified. Previously, when pg_basebackup was
writing a tarfile to stdout, it tried to receive backup_manifest file even
when --no-manifest was specified, and reported an error.

Also remove unused -m option from pg_basebackup.

Also fix typo in BASE_BACKUP command documentation.

Author: Fujii Masao
Reviewed-by: Michael Paquier, Robert Haas
Discussion: https://postgr.es/m/01e3ed3a-8729-5aaa-ca84-e60e3ca59db8@oss.nttdata.com

Remove obsolete "hole in center of page" comment.

A comment from the Berkeley days incorrectly claimed that the page
management code cares about the contents of the hole in the center of
the page (at least in the case of the left half of an nbtree page
split).  Commit 8fa30f906be added an addendum that stated that the
original comment was "probably obsolete".  It's definitely obsolete,
though, so remove the original comment plus the addendum.

Account for collation when coercing the output of a SQL function.

Commit 913bbd88d overlooked that the result of coerce_to_target_type
might need collation fixups.  Per report from Andreas Joseph Krogh.

Discussion: https://postgr.es/m/VisenaEmail.72.37d08ec2b8cb8fb5.17179940cd3@tc7-visena

Stop requiring an explicit return from perl subroutines

The consensus of the project appears to be that this provides little
benefit and is simply an annoyance.

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

Set Perl search path more idiomatically

Back in commits 1df92eeafe, f884a96819, and 592123efbb I used some
hackish code to set the script search path, unaware despite decades of
perl that there was a completely standard way to do this. This patch
changes those cases to use the standard perl FindBin package.

Document the backup manifest file format.

Patch by me, at the request of Andres Freund. Reviewed by
Justin Pryzby, Erik Rijkers, Álvaro Herrera, and Andrew
Dunstan.

Discussion: http://postgr.es/m/20200327203225.hcm6ag4grwsiruea@alap3.anarazel.de

Rearrange _bt_insertonpg() "update metapage" code.

Nest the "update metapage as part of insert into root-like page" branch
inside the broader "insert into internal page" branch.  This improves
readability.

Fix collection of typos and grammar mistakes in the tree, volume 2

This fixes some comments and documentation new as of Postgres 13, and is
a follow-up of the work done in dd0f37e.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20200408165653.GF2228@telsasoft.com

Add defensive "split_only_page" nbtree assertion.

Clearly it's not okay for nbtree to split a page that is the only page
on its level, and then find that it has to split the parent one level up
in turn.  There is simply no code to handle the split_only_page case in
the _bt_insertonpg() "newitem won't fit" branch (only the "newitem fits"
branch handles split_only_page).  Add a defensive assertion that will
fail if a split_only_page call to _bt_insertonpg() somehow ends up
splitting the target/parent page.

I (pgeoghegan) believe that we don't need split_only_page handling for
the "newitem won't fit" branch because anybody calling _bt_insertonpg()
like this would have to hold a lock on the same one and only child page.

Comments and doc fixes for commit 40d964ec99.

Reported-by: Justin Pryzby
Author: Justin Pryzby, with few changes by me
Reviewed-by: Amit Kapila and Sawada Masahiko
Discussion: https://postgr.es/m/20200322021801.GB2563@telsasoft.com

Make _bt_insertonpg() more like _bt_split().

It seems like a good idea for nbtree's retail insert code to be
absolutely consistent with nbtree's page split code for anything that
naturally requires equivalent handling.  Anything that concerns
inserting newitem (which is handled as part of the page split atomic
action when a page split is required) should work in exactly the same
way.  With that in mind, make _bt_insertonpg() handle 'cbuf' in a way
that matches _bt_split().

Add a wait_for_catchup() before immediate stop of a test 

Per buildfarm member hoverfly, a slow walsender could make the test
fail.  Back-patch to v10, where the test was introduced.

Discussion: https://postgr.es/m/20200414013849.GA886648@rfd.leadboat.com

Silence Perl warning

Now that warnings are enabled across the board, this code that tries to
print an undef variable emits one.  Silently printing the empty string
achieves the previous behavior.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Discussion: https://postgr.es/m/E1jO1VT-0008Qk-TM@gemulon.postgresql.org

Harmonize nbtree page split point code.

An nbtree split point can be thought of as a point between two adjoining
tuples from an imaginary version of the page being split that includes
the incoming/new item (in addition to the items that really are on the
page).  These adjoining tuples are called the lastleft and firstright
tuples.

The variables that represent split points contained a field called
firstright, which is an offset number of the first data item from the
original page that goes on the new right page.  The corresponding tuple
from origpage was usually the same thing as the actual firstright tuple,
but not always: the firstright tuple is sometimes the new/incoming item
instead.  This situation seems unnecessarily confusing.

Make things clearer by renaming the origpage offset returned by
_bt_findsplitloc() to "firstrightoff".  We now have a firstright tuple
and a firstrightoff offset number which are comparable to the
newitem/lastleft tuples and the newitemoff/lastleftoff offset numbers
respectively.  Also make sure that we are consistent about how we
describe nbtree page split point state.

Push the responsibility for dealing with pg_upgrade'd !heapkeyspace
indexes down to lower level code, relieving _bt_split() from dealing
with it directly.  This means that we always have a palloc'd left page
high key on the leaf level, no matter what.  This enables simplifying
some of the code (and code comments) within _bt_split().

Finally, restructure the page split code to make it clearer why suffix
truncation (which only takes place during leaf page splits) is
completely different to the first data item truncation that takes place
during internal page splits.  Tuples are marked as having fewer
attributes stored in both cases, and the firstright tuple is truncated
in both cases, so it's easy to imagine somebody missing the distinction.