Revert test added by commit d207038053.

This test was trying to test the mechanism to release kernel FDs as needed
to get us under the max_safe_fds limit in case of spill files.  To do that,
it needs to set max_files_per_process to a very low value which doesn't
even permit starting of the server in the case when there are a few already
opened files.  This test also won't work on platforms where we use one FD
per semaphore.

Backpatch-through: 10, till where this test was added
Discussion:
https://postgr.es/m/CAA4eK1LHhERi06Q+MmP9qBXBBboi+7WV3910J0aUgz71LcnKAw@mail.gmail.com
https://postgr.es/m/6485.1578583522@sss.pgh.pa.us

Fix base backup with database OIDs larger than INT32_MAX

The use of pg_atoi() for parsing a string into an Oid fails for values
larger than INT32_MAX, since OIDs are unsigned.  Instead, use
atooid().  While this has less error checking, the contents of the
data directory are expected to be trustworthy, so we don't need to go
out of our way to do full error checking.

Discussion: https://www.postgresql.org/message-id/flat/dea47fc8-6c89-a2b1-07e3-754ff1ab094b%402ndquadrant.com

Fix typo.

Reported-by: Antonin Houska
Author: Antonin Houska
Backpatch-through: 11, where it was introduced
Discussion: https://postgr.es/m/2246.1578900133@antos

Fix edge-case crashes and misestimation in range containment selectivity.

When estimating the selectivity of "range_var <@ range_constant" or
"range_var @> range_constant", if the upper (or respectively lower)
bound of the range_constant was above the last bin of the range_var's
histogram, the code would access uninitialized memory and potentially
crash (though it seems the probability of a crash is quite low).
Handle the endpoint cases explicitly to fix that.

While at it, be more paranoid about the possibility of getting NaN
or other silly results from the range type's subdiff function.
And improve some comments.

Ordinarily we'd probably add a regression test case demonstrating
the bug in unpatched code.  But it's too hard to get it to crash
reliably because of the uninitialized-memory dependence, so skip that.

Per bug #16122 from Adam Scott.  It's been broken from the beginning,
apparently, so backpatch to all supported branches.

Diagnosis by Michael Paquier, patch by Andrey Borodin and Tom Lane.

Discussion: https://postgr.es/m/16122-eb35bc248c806c15@postgresql.org

Remove incorrect assertion for INSERT in logical replication's publisher

On the publisher, it was assumed that an INSERT change cannot happen for
a relation with no replica identity.  However this is true only for a
change that needs references to old rows, aka UPDATE or DELETE, so
trying to use logical replication with a relation that has no replica
identity led to an assertion failure in the publisher when issuing an
INSERT.  This commit removes the incorrect assertion, and adds more
regression tests to provide coverage for relations without replica
identity.

Reported-by: Neha Sharma
Author: Dilip Kumar, Michael Paquier
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CANiYTQsL1Hb8_Km08qd32svrqNumXLJeoGo014O7VZymgOhZEA@mail.gmail.com
Backpatch-through: 10

Extensive code review for GSSAPI encryption mechanism.

Fix assorted bugs in handling of non-blocking I/O when using GSSAPI
encryption.  The encryption layer could return the wrong status
information to its caller, resulting in effectively dropping some data
(or possibly in aborting a not-broken connection), or in a "livelock"
situation where data remains to be sent but the upper layers think
transmission is done and just go to sleep.  There were multiple small
thinkos contributing to that, as well as one big one (failure to think
through what to do when a send fails after having already transmitted
data).  Note that these errors could cause failures whether the client
application asked for non-blocking I/O or not, since both libpq and
the backend always run things in non-block mode at this level.

Also get rid of use of static variables for GSSAPI inside libpq;
that's entirely not okay given that multiple connections could be
open at once inside a single client process.

Also adjust a bunch of random small discrepancies between the frontend
and backend versions of the send/receive functions -- except for error
handling, they should be identical, and now they are.

Also extend the Kerberos TAP tests to exercise cases where nontrivial
amounts of data need to be pushed through encryption.  Before, those
tests didn't provide any useful coverage at all for the cases of
interest here.  (They still might not, depending on timing, but at
least there's a chance.)

Per complaint from pmc@citylink and subsequent investigation.
Back-patch to v12 where this code was introduced.

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

Maintain valid md.c state when FileClose() fails.

FileClose() failure ordinarily causes a PANIC.  Suppose the user
disables that PANIC via data_sync_retry=on.  After mdclose() issued a
FileClose() that failed, calls into md.c raised SIGSEGV.  This fix adds
repalloc() calls during mdclose(); update a comment about ignoring
repalloc() cost.  The rate of relation segment count change is a minor
factor; more relevant to overall performance is the rate of mdclose()
and subsequent re-opening of segments.  Back-patch to v10, where commit
45e191e3aa62d47a8bc1a33f784286b2051f45cb introduced the bug.

Reviewed by Kyotaro Horiguchi.

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

doc: Fix naming of SELinux

Reported-by: Tham Nguyen
Discussion: https://postgr.es/m/157851402876.29175.12977878383183540468@wrigleys.postgresql.org
Backpatch-through: 9.4

doc: Add link to upgrading chapter to release notes

Author: Vik Fearing 
Discussion: https://www.postgresql.org/message-id/flat/54c208b9-7e2c-6211-0ba0-ffb0429cf20b@2ndquadrant.com

Reimplement nullification of walsender timestamp

Make the value null only at pg_stat_activity-output time, as suggested
by Tom Lane, instead of messing with the internal state.  This should
appease buildfarm members with force_parallel_mode=regress, which are
running parallel queries on logical replication walsenders.

The fact that walsenders can run parallel queries should perhaps be
studied more carefully, but for the moment let's get rid of the red
blots in buildfarm.

Backpatch to pg10, like the previous commit.

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

Fix handling of generated columns in ALTER TABLE.

ALTER TABLE failed if a column referenced in a GENERATED expression
had been added or changed in type earlier in the ALTER command.
That's because the GENERATED expression needs to be evaluated
against the table's updated tuples, but it was being evaluated
against the original tuples.  (Fortunately the executor has adequate
cross-checks to notice the mismatch, so we just got an obscure error
message and not anything more dangerous.)

Per report from Andreas Joseph Krogh.  Back-patch to v12 where
GENERATED was added.

Discussion: https://postgr.es/m/VisenaEmail.200.231b0a41523275d0.16ea7f800c7@tc7-visena

Revert "Forbid DROP SCHEMA on temporary namespaces"

This reverts commit a052f6c, following complains from Robert Haas and
Tom Lane.  Backpatch down to 9.4, like the previous commit.

Discussion: https://postgr.es/m/CA+TgmobL4npEX5=E5h=5Jm_9mZun3MT39Kq2suJFVeamc9skSQ@mail.gmail.com
Backpatch-through: 9.4

pg_stat_activity: show NULL stmt start time for walsenders

Returning a non-NULL time is pointless, sinc a walsender is not a
process that would be running normal transactions anyway, but the code
was unintentionally exposing the process start time intermittently,
which was not only bogus but it also confused monitoring systems looking
for idle transactions.  Fix by avoiding all updates in walsenders.

Backpatch to 11, where walsenders started appearing in pg_stat_activity.

Reported-by: Tomas Vondra
Discussion: https://postgr.es/m/20191209234409.exe7osmyalwkt5j4@development

Reduce the number of GetFlushRecPtr() calls done by walsenders.

Since the WAL flush position only moves forward, it's safe to cache
its previous value within each walsender process, and update from
shared memory only once we've caught up to the previously-seen value.
When there are many active walsenders, this makes for a very significant
reduction in the amount of contention on the XLogCtl->info_lck spinlock.

This patch also adjusts the logic so that we update our idea of the
flush position after processing a WAL record, rather than beforehand.
This may cause us to realize we're not caught up when the preceding
coding would've thought that we were, but that seems all to the good;
it may avoid a useless sleep-and-wakeup cycle.

Back-patch to v12.  The contention problem exists in prior branches,
but it's much less severe (due to inefficiencies elsewhere) so there
seems no need to take any risk of back-patching further.

Pierre Ducroquet, reviewed by Julien Rouhaud

Discussion: https://postgr.es/m/2931018.Vxl9zapr77@pierred-pdoc

Have logical replication subscriber fire column triggers

The logical replication apply worker did not fire per-column update
triggers because the updatedCols bitmap in the RTE was not populated.
This fixes that.

Reviewed-by: Euler Taveira
Discussion: https://www.postgresql.org/message-id/flat/21673e2d-597c-6afe-637e-e8b10425b240%402ndquadrant.com

Docs: use more standard terminology "round-to-nearest-even" instead of "round-to-even".

Per suggestion from Tom Lane.
Discussion: https://postgr.es/m/flat/20191230.093451.1762483750956466101.t-ishii%40sraoss.co.jp

Fix typos in parallel query docs.

Reported-by: Jon Jensen
Author: Jon Jensen
Reviewed-by: Amit Kapila and Robert Haas
Backpatch-through: 10
Discussion: https://postgr.es/m/nycvar.YSQ.7.76.1912301807510.9899@ybpnyubfg

Fix cloning of row triggers to sub-partitions

When row triggers exist in partitioned partitions that are not either
part of FKs or deferred unique constraints, they are not correctly
cloned to their partitions.  That's because they are marked "internal",
and those are purposefully skipped when doing the clone triggers dance.
Fix by relaxing the condition on which internal triggers are skipped.

Amit Langote initially diagnosed the problem and proposed a fix, but I
used a different approach.

Reported-by: Petr Fedorov
Discussion: https://postgr.es/m/6b3f0646-ba8c-b3a9-c62d-1c6651a1920f@phystech.edu

Fix comment in test

The comment was apparently copy-and-pasted and did not reflect the
actual test outcome.

Fix running out of file descriptors for spill files.

Currently while decoding changes, if the number of changes exceeds a
certain threshold, we spill those to disk.  And this happens for each
(sub)transaction.  Now, while reading all these files, we don't close them
until we read all the files.  While reading these files, if the number of
such files exceeds the maximum number of file descriptors, the operation
errors out.

Use PathNameOpenFile interface to open these files as that internally has
the mechanism to release kernel FDs as needed to get us under the
max_safe_fds limit.

Reported-by: Amit Khandekar
Author: Amit Khandekar
Reviewed-by: Amit Kapila
Backpatch-through: 9.4
Discussion: https://postgr.es/m/CAJ3gD9c-sECEn79zXw4yBnBdOttacoE-6gAyP0oy60nfs_sabQ@mail.gmail.com

Update copyrights for 2020

Backpatch-through: update all files in master, backpatch legal files through 9.4

Add pg_dump test for triggers on partitioned tables

This currently works, but add this test to ensure it continues to work.
Lack of this test became evident after a recent bugfix submission that
would have inadvertently broken it, in
https://postgr.es/m/CA+HiwqFM2=i+uHB9o4OkLbE2S3sjPHoVe2wXuAD1GLJ4+Pk9eg@mail.gmail.com

doc:  add examples of creative use of unique expression indexes

Unique expression indexes can constrain data in creative ways, so show
two examples.

Reported-by: Tuomas Leikola
Discussion: https://postgr.es/m/156760275564.1127.12321702656456074572@wrigleys.postgresql.org

Backpatch-through: 9.4

docs:  clarify infinite range values from data-type infinities

The previous docs referenced these distinct ideas confusingly.

Reported-by: Eugen Konkov
Discussion: https://postgr.es/m/376945611.20191026161529@yandex.ru

Backpatch-through: 9.4

Forbid DROP SCHEMA on temporary namespaces

This operation was possible for the owner of the schema or a superuser.
Down to 9.4, doing this operation would cause inconsistencies in a
session whose temporary schema was dropped, particularly if trying to
create new temporary objects after the drop.  A more annoying
consequence is a crash of autovacuum on an assertion failure when
logging information about an orphaned temp table dropped.  Note that
because of 246a6c8 (present in v11~), which has made the removal of
orphaned temporary tables more aggressive, the failure could be
triggered more easily, but it is possible to reproduce down to 9.4.

Reported-by: Mahendra Singh, Prabhat Sahu
Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, Mahendra Singh
Discussion: https://postgr.es/m/CAKYtNAr9Zq=1-ww4etHo-VCC-k120YxZy5OS01VkaLPaDbv2tg@mail.gmail.com
Backpatch-through: 9.4

Fix possible loss of sync between rectypeid and underlying PLpgSQL_type.

When revalidate_rectypeid() acts to update a stale record type OID
in plpgsql's data structures, it fixes the active PLpgSQL_rec struct
as well as the PLpgSQL_type struct it references.  However, the latter
is shared across function executions while the former is not.  In a
later function execution, the PLpgSQL_rec struct would be reinitialized
by copy_plpgsql_datums and would then contain a stale type OID,
typically leading to "could not open relation with OID NNNN" errors.
revalidate_rectypeid() can easily fix this, fortunately, just by
treating typ->typoid as authoritative.

Per report and diagnosis from Ashutosh Sharma, though this is not his
suggested fix.  Back-patch to v11 where this code came in.

Discussion: https://postgr.es/m/CAE9k0Pkd4dZwt9J5pS9xhJFWpUtqs05C9xk_GEwPzYdV=GxwWg@mail.gmail.com

Fix some comments related to logical repslot advancing

confirmed_flush is part of a replication slot's information, but not
confirmed_lsn.

Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20191226.175919.17237335658671970[email protected]
Backpatch-through: 11

Rotate instead of shifting hash join batch number.

Our algorithm for choosing batch numbers turned out not to work
effectively for multi-billion key inner relations.  We would use
more hash bits than we have, and effectively concentrate all tuples
into a smaller number of batches than we intended.  While ideally
we should switch to wider hashes, for now, change the algorithm to
one that effectively gives up bits from the bucket number when we
don't have enough bits.  That means we'll finish up with longer
bucket chains than would be ideal, but that's better than having
batches that don't fit in work_mem and can't be divided.

Batch-patch to all supported releases.

Author: Thomas Munro
Reviewed-by: Tom Lane, thanks also to Tomas Vondra, Alvaro Herrera, Andres Freund for testing and discussion
Reported-by: James Coleman
Discussion: https://postgr.es/m/16104-dc11ed911f1ab9df%40postgresql.org

Disallow null category in crosstab_hash

While building a hash map of categories in load_categories_hash,
resulting category names have not thus far been checked to ensure
they are not null. Prior to pg12 null category names worked to the
extent that they did not crash on some platforms. This is because
those system libraries have an snprintf which can deal with being
passed a null pointer argument for a string. But even in those cases
null categories did nothing useful. And on some platforms it crashed.
As of pg12, our own version of snprintf gets called, and it does
not deal with null pointer arguments at all, and crashes consistently.

Fix that by disallowing null categories. They never worked usefully,
and no one has ever asked for them to work previously. Back-patch to
all supported branches.

Reported-By: Ireneusz Pluta
Discussion: https://postgr.es/m/16176-7489719b05e4303c@postgresql.org

Disallow partition key expressions that return pseudo-types.

This wasn't checked originally, but it should have been, because
in general pseudo-types can't be stored to and retrieved from disk.
Notably, partition bound values of type "record" would not be
interpretable by another session.

In v12 and HEAD, add another flag to CheckAttributeType's repertoire
so that it can produce a specific error message for this case.  That's
infeasible in older branches without an ABI break, so fall back to
a slightly-less-nicely-worded error message in v10 and v11.

Problem noted by Amit Langote, though this patch is not his initial
solution.  Back-patch to v10 where partitioning was introduced.

Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com

Prevent a rowtype from being included in itself via a range.

We probably should have thought of this case when ranges were added,
but we didn't.  (It's not the fault of commit eb51af71f, because
ranges didn't exist then.)

It's an old bug, so back-patch to all supported branches.

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

Avoid low-probability regression test failures in timestamp[tz] tests.

If the first transaction block in these tests were entered exactly
at midnight (California time), they'd report a bogus failure due
to 'now' and 'midnight' having the same values.  Commit 8c2ac75c5
had dismissed this as being of negligible probability, but we've
now seen it happen in the buildfarm, so let's prevent it.  We can
get pretty much the same test coverage without an it's-not-midnight
assumption by moving the does-'now'-work cases into their own test step.

While here, apply commit 47169c255's s/DELETE/TRUNCATE/ change to
timestamptz as well as timestamp (not sure why that didn't
occur to me at the time; the risk of failure is the same).

Back-patch to all supported branches, since the main point is
to get rid of potential buildfarm failures.

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

In pgwin32_open, loop after ERROR_ACCESS_DENIED only if we can't stat.

This fixes a performance problem introduced by commit 6d7547c21.
ERROR_ACCESS_DENIED is returned in some other cases besides the
delete-pending case considered by that commit; notably, if the
given path names a directory instead of a plain file.  In that
case we'll uselessly loop for 1 second before returning the
failure condition.  That slows down some usage scenarios enough
to cause test timeout failures on our Windows buildfarm critters.

To fix, try to stat() the file, and sleep/loop only if that fails.
It will fail in the delete-pending case, and also in the case where
the deletion completed before we could stat(), so we have the cases
where we want to loop covered.  In the directory case, the stat()
should succeed, letting us exit without a wait.

One case where we'll still wait uselessly is if the access-denied
problem pertains to a directory in the given pathname.  But we don't
expect that to happen in any performance-critical code path.

There might be room to refine this further, but I'll push it now
in hopes of making the buildfarm green again.

Back-patch, like the preceding commit.

Alexander Lakhin and Tom Lane

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

docs:  clarify handling of column lists in COPY TO/FROM

Previously it was unclear how COPY FROM handled cases where not all
columns were specified, or if the order didn't match.

Reported-by: [email protected]
Discussion: https://postgr.es/m/157487729344.7213.14245726713444755296@wrigleys.postgresql.org

Backpatch-through: 9.4

libpq should expose GSS-related parameters even when not implemented.

We realized years ago that it's better for libpq to accept all
connection parameters syntactically, even if some are ignored or
restricted due to lack of the feature in a particular build.
However, that lesson from the SSL support was for some reason never
applied to the GSSAPI support.  This is causing various buildfarm
members to have problems with a test case added by commit 6136e94dc,
and it's just a bad idea from a user-experience standpoint anyway,
so fix it.

While at it, fix some places where parameter-related infrastructure
was added with the aid of a dartboard, or perhaps with the aid of
the anti-pattern "add new stuff at the end".  It should be safe
to rearrange the contents of struct pg_conn even in released
branches, since that's private to libpq (and we'd have to move
some fields in some builds to fix this, anyway).

Back-patch to all supported branches.

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

Doc: add a short summary of available authentication methods.

The "auth-methods"  used to include descriptions of all our
authentication methods.  Commit 56811e573 promoted its child 's
to 's, which has advantages but also created some issues:
* The auth-methods page itself is essentially empty/useless.
* Links that pointed to "auth-methods" as a placeholder for all
auth methods were rendered a bit nonsensical.
* DocBook no longer provides a subsection table-of-contents here,
which formerly was a useful if terse summary of available auth methods.

To improve matters, add a handwritten list of all the auth methods.

Per gripe from Dave Cramer.  Back-patch to v11 where the previous
commit came in.

Discussion: https://postgr.es/m/CADK3HH+xQLhcPgg=kWqfogtXGGZr-JdSo=x=WQC0PkAVyxUWyQ@mail.gmail.com

Update neglected comment.

Commit d986d4e87f61c68f52c68ebc274960dc664b7b4e renamed a variable
but neglected to update the corresponding comment.

Amit Langote

Fix subscriber invalid memory access on DDL.

This patch allows building the local relmap cache for a subscribed
relation after processing pending invalidation messages and potential
relcache updates.  Without this, the attributes in the local cache don't
tally with the updated relcache entry leading to invalid memory access.

Reported-by Jehan-Guillaume de Rorthais
Author: Jehan-Guillaume de Rorthais and Vignesh C
Reviewed-by: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/20191025175929.7e90dbf5@firost

Remove shadow variables linked to RedoRecPtr in xlog.c

This changes the routines in charge of recycling WAL segments past the
last redo LSN to not use anymore "RedoRecPtr" as a local variable, which
is also available in the context of the session as a static declaration,
replacing it with "lastredoptr".  This confusion has been introduced by
d9fadbf, so backpatch down to v11 like the other commit.

Thanks to Tom Lane, Robert Haas, Alvaro Herrera, Mark Dilger and Kyotaro
Horiguchi for the input provided.

Author: Ranier Vilela
Discussion: https://postgr.es/m/MN2PR18MB2927F7B5F690065E1194B258E35D0@MN2PR18MB2927.namprd18.prod.outlook.com
Backpatch-through: 11

Fix error reporting for index expressions of prohibited types.

If CheckAttributeType() threw an error about the datatype of an
index expression column, it would report an empty column name,
which is pretty unhelpful and certainly not the intended behavior.
I (tgl) evidently broke this in commit cfc5008a5, by not noticing
that the column's attname was used above where I'd placed the
assignment of it.

In HEAD and v12, this is trivially fixable by moving up the
assignment of attname.  Before v12 the code is a bit more messy;
to avoid doing substantial refactoring, I took the lazy way out
and just put in two copies of the assignment code.

Report and patch by Amit Langote.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/CA+HiwqFA+BGyBFimjiYXXMa2Hc3fcL0+OJOyzUNjhU4NCa_XXw@mail.gmail.com

Change overly strict Assert in TransactionGroupUpdateXidStatus.

This Assert thought that an overflowed transaction can never get registered
for the group update.  But that is not true, because even when the number
of children for a transaction got reduced, the overflow flag is not
changed.  And, for group update, we only care about the current number of
children for a transaction that is being committed.

Based on comments by Andres Freund, remove a redundant Assert in
TransactionIdSetPageStatus as we already had a static Assert for the same
condition a few lines earlier.

Reported-by: Vignesh C
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Backpatch-through: 11
Discussion: https://postgr.es/m/CAFiTN-s5=uJw-Z6JC9gcqtBSjXsrHnU63PXBrA=pnBjqnkm5UA@mail.gmail.com

On Windows, wait a little to see if ERROR_ACCESS_DENIED goes away.

Attempting to open a file fails with ERROR_ACCESS_DENIED if the file
is flagged for deletion but not yet actually gone (another in a long
list of reasons why Windows is broken, if you ask me).  This seems
likely to explain a lot of irreproducible failures we see in the
buildfarm.  This state generally persists for only a millisecond or so,
so just wait a bit and retry.  If it's a real permissions problem,
we'll eventually give up and report it as such.  If it's the pending
deletion case, we'll see file-not-found and report that after the
deletion completes, and the caller will treat that in an appropriate
way.

In passing, rejigger the existing retry logic for some other error
cases so that we don't uselessly wait an extra time when we're
not going to retry anymore.

Alexander Lakhin (with cosmetic tweaks by me).  Back-patch to all
supported branches, since this seems like a pretty safe change and
the problem is definitely real.

Discussion: https://postgr.es/m/16161-7a985d2f1bbe8f71@postgresql.org

Fix yet another crash in page split during GiST index creation.

Commit a7ee7c8513 fixed a bug in GiST page split during index creation,
where we failed to re-find the position of a downlink after the page
containing it was split. However, that fix was incomplete; the other call
to gistinserttuples() in the same function needs to also clear
'downlinkoffnum'.

Fixes bug #16134 reported by Alexander Lakhin, for real this time. The
previous fix was enough to fix the crash with the reproducer script for
bug #16162, but the original script for #16134 was still crashing.

Backpatch to v12, like the previous incomplete fix.

Discussion: https://www.postgresql.org/message-id/d869f537-abe4-d2ea-0510-38cd053f5152%40gmail.com

Clean up some misplaced comments in partition_join.sql regression test.

Also, add a comment explaining a test case.

Back-patch to 11 where the regression test was added.

Discussion: https://postgr.es/m/CAPmGK15adZPh2B%2BmGUjSOMH%2BH39ogDRWfCfm4G6jncZCAs9V_Q%40mail.gmail.com

Prevent overly-aggressive collapsing of joins to RTE_RESULT relations.

The RTE_RESULT simplification logic added by commit 4be058fe9 had a
flaw: it would collapse out a RTE_RESULT that is due to compute a
PlaceHolderVar, and reassign the PHV to the parent join level, even if
another input relation of the join contained a lateral reference to
the PHV.  That can't work because the PHV would be computed too late.
In practice it led to failures of internal sanity checks later in
planning (either assertion failures or errors such as "failed to
construct the join relation").

To fix, add code to check for the presence of such PHVs in relevant
portions of the query tree.  Notably, this required refactoring
range_table_walker so that a caller could ask to walk individual RTEs
not the whole list.  (It might be a good idea to refactor
range_table_mutator in the same way, if only to keep those functions
looking similar; but I didn't do so here as it wasn't necessary for
the bug fix.)

This exercise also taught me that find_dependent_phvs(), as it stood,
could only safely be used on the entire Query, not on subtrees.
Adjust its API to reflect that; which in passing allows it to have
a fast path for the common case of no PHVs anywhere.

Per report from Will Leinweber.  Back-patch to v12 where the bug
was introduced.

Discussion: https://postgr.es/m/CALLb-4xJMd4GZt2YCecMC95H-PafuWNKcmps4HLRx2NHNBfB4g@mail.gmail.com

Fix mdsyncfiletag(), take II.

The previous commit failed to consider that FileGetRawDesc() might
not return a valid fd, as discovered on the build farm.  Switch to
using the File interface only.

Back-patch to 12, like the previous commit.

Don't use _mdfd_getseg() in mdsyncfiletag().

_mdfd_getseg() opens all segments up to the requested one.  That
causes problems for mdsyncfiletag(), if mdunlinkfork() has
already unlinked other segment files.  Open the file we want
directly by name instead, if it's not already open.

The consequence of this bug was a rare panic in the checkpointer,
made more likely if you saturated the sync request queue so that
the SYNC_FORGET_REQUEST messages for a given relation were more
likely to be absorbed in separate cycles by the checkpointer.

Back-patch to 12.  Defect in commit 3eb77eba.

Author: Thomas Munro
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20191119115759.GI30362%40telsasoft.com

Fix crash when a page was split during GiST index creation.

The bug was similar to the one that was fixed in commit 22251686f0. When
we split page X and insert the downlink for the new page, the parent page
might also need to be split. When that happens, the downlink offset number
we remembered for X is no longer valid. We correctly called
gistFindCorrectParent() to re-find it, but gistFindCorrectParent() doesn't
do anything if the LSN of the page hasn't changed, and we stopped updating
LSNs during index build in commit 9155580fd5. The buggy codepath was taken
if the page was split into three or more pages, and inserting the downlink
caused the parent page to split. To fix, explicitly mark the downlink
offset number as invalid, to force gistFindCorrectParent() to re-find it.

Fixes bug #16134 reported by Alexander Lakhin, reported again as #16162 by
Andreas Kunert. Thanks to Jeff Janes, Tom Lane and Tomas Vondra for
debugging. Backpatch to v12, where we stopped WAL-logging during index
build.

Discussion: https://www.postgresql.org/message-id/16134-0423f729671dec64%40postgresql.org
Discussion: https://www.postgresql.org/message-id/16162-45d21b7b6c1a3105%40postgresql.org

Fix EXTRACT(ISOYEAR FROM timestamp) for years BC.

The test cases added by commit 26ae3aa80 exposed an old oversight in
timestamp[tz]_part: they didn't correct the result of date2isoyear()
for BC years, so that we produced an off-by-one answer for such years.
Fix that, and back-patch to all supported branches.

Discussion: https://postgr.es/m/SG2PR06MB37762CAE45DB0F6CA7001EA9B6550@SG2PR06MB3776.apcprd06.prod.outlook.com

Remove redundant function calls in timestamp[tz]_part().

The DTK_DOW/DTK_ISODOW and DTK_DOY switch cases in timestamp_part() and
timestamptz_part() contained calls of timestamp2tm() that were fully
redundant with the ones done just above the switch.  This evidently crept
in during commit 258ee1b63, which relocated that code from another place
where the calls were indeed needed.  Just delete the redundant calls.

I (tgl) noted that our test coverage of these functions left quite a
bit to be desired, so extend timestamp.sql and timestamptz.sql to
cover all the branches.

Back-patch to all supported branches, as the previous commit was.
There's no real issue here other than some wasted cycles in some
not-too-heavily-used code paths, but the test coverage seems valuable.

Report and patch by Li Japin; test case adjustments by me.

Discussion: https://postgr.es/m/SG2PR06MB37762CAE45DB0F6CA7001EA9B6550@SG2PR06MB3776.apcprd06.prod.outlook.com

Remove extra parenthesis from comment.

In pg_ctl, work around ERROR_SHARING_VIOLATION on the postmaster log file.

On Windows, we use CMD.EXE to redirect the postmaster's stdout/stderr
into a log file.  CMD.EXE will open that file with non-sharing-friendly
parameters, and the file will remain open for a short time after the
postmaster has removed postmaster.pid.  This can result in an
ERROR_SHARING_VIOLATION failure if we attempt to start a new postmaster
immediately with the same log file (e.g. during "pg_ctl restart").
This seems to explain intermittent buildfarm failures we've been seeing
on Windows machines.

To fix, just open and close the log file using our own pgwin32_open(),
which will wait if necessary to avoid the failure.  (Perhaps someday
we should stop using CMD.EXE, but that would be a far more complex
patch, and it doesn't seem worth the trouble ... yet.)

Back-patch to v12.  This only solves the problem when frontend fopen()
is redirected to pgwin32_fopen(), which has only been true since commit
0ba06e0bf.  Hence, no point in back-patching further, unless we care
to back-patch that change too.

Diagnosis and patch by Alexander Lakhin (bug #16154).

Discussion: https://postgr.es/m/16154-1ccf0b537b24d5e0@postgresql.org

Fix handling of multiple AFTER ROW triggers on a foreign table.

AfterTriggerExecute() retrieves a fresh tuple or pair of tuples from a
tuplestore and then stores the tuple(s) in the passed-in slot(s) if
AFTER_TRIGGER_FDW_FETCH, while it uses the most-recently-retrieved
tuple(s) stored in the slot(s) if AFTER_TRIGGER_FDW_REUSE.  This was
done correctly before 12, but commit ff11e7f4b broke it by mistakenly
clearing the tuple(s) stored in the slot(s) in that function, leading to
an assertion failure as reported in bug #16139 from Alexander Lakhin.

Also, fix some other issues with the aforementioned commit in passing:

* For tg_newslot, which is a slot added to the TriggerData struct by the
  commit to store new updated tuples, it didn't ensure the slot was NULL
  if there was no such tuple.
* The commit failed to update the documentation about the trigger
  interface.

Author: Etsuro Fujita
Backpatch-through: 12
Discussion: https://postgr.es/m/16139-94f9ccf0db6119ec%40postgresql.org

Fix race condition in our Windows signal emulation.

pg_signal_dispatch_thread() responded to the client (signal sender)
and disconnected the pipe before actually setting the shared variables
that make the signal visible to the backend process's main thread.
In the worst case, it seems, effective delivery of the signal could be
postponed for as long as the machine has any other work to do.

To fix, just move the pg_queue_signal() call so that we do it before
responding to the client.  This essentially makes pgkill() synchronous,
which is a stronger guarantee than we have on Unix.  That may be
overkill, but on the other hand we have not seen comparable timing bugs
on any Unix platform.

While at it, add some comments to this sadly underdocumented code.

Problem diagnosis and fix by Amit Kapila; I just added the comments.

Back-patch to all supported versions, as it appears that this can cause
visible NOTIFY timing oddities on all of them, and there might be
other misbehavior due to slow delivery of other signals.

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

Improve isolationtester's timeout management.

isolationtester.c had a hard-wired limit of 3 minutes per test step.
It now emerges that this isn't quite enough for some of the slowest
buildfarm animals.  This isn't the first time we've had to raise
this limit (cf. 1db439ad4), so let's make it configurable.  This
patch raises the default to 5 minutes, and introduces an environment
variable PGISOLATIONTIMEOUT that can be set if more time is needed,
following the precedent of PGCTLTIMEOUT.

Also, modify isolationtester so that when the timeout is hit,
it explicitly reports having sent a cancel.  This makes the regression
failure log considerably more intelligible.  (In the worst case, a
timed-out test might actually be reported as "passing" without this
extra output, so arguably this is a bug fix in itself.)

In passing, update the README file, which had apparently not gotten
touched when we added "make check" support here.

Back-patch to 9.6; older versions don't have comparable timeout logic.

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

Fix typos in miscinit.c.

Commit f13ea95f9e moved the description of postmaster.pid file contents
from miscadmin.h to pidfile.h, but missed to update the comments in
miscinit.c.

Author: Hadi Moshayedi
Reviewed-by: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/CAK=1=WpYEM9x3LGkaxgXaxeYQjnkdW8XLsxrYRTE2Gq-H83FMw@mail.gmail.com

Document search_path security with untrusted dbowner or CREATEROLE.

Commit 5770172cb0c9df9e6ce27c507b449557e5b45124 wrote, incorrectly, that
certain schema usage patterns are secure against CREATEROLE users and
database owners.  When an untrusted user is the database owner or holds
CREATEROLE privilege, a query is secure only if its session started with
SELECT pg_catalog.set_config('search_path', '', false) or equivalent.
Back-patch to 9.4 (all supported versions).

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

Doc: improve documentation about run-time pruning's effects on EXPLAIN.

Tatsuo Ishii complained that this para wasn't very intelligible.
Try to make it better.

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

Fix handling of OpenSSL's SSL_clear_options

This function is supported down to OpenSSL 0.9.8, which is the oldest
version supported since 593d4e4 (from Postgres 10 onwards), and is used
since e3bdb2d (from 11 onwards).  It is defined as a macro from OpenSSL
0.9.8 to 1.0.2, and as a function in 1.1.0 and newer versions.  However,
the configure check present is only adapted for functions.  So, even if
the code would be able to compile, configure fails to detect the macro,
causing it to be ignored when compiling the code with OpenSSL from 0.9.8
to 1.0.2.

The code needs a configure check as per a364dfa, which has fixed a
compilation issue with a past version of LibreSSL in NetBSD 5.1.  On
HEAD, just remove the configure check as the last release of NetBSD 5 is
from 2014 (and we have no more buildfarm members for it).  In 11 and 12,
improve the configure logic so as both macros and functions are
correctly detected.  This makes NetBSD 5 still work on already-released
branches, but not for 13 onwards.

The patch for HEAD is from me, and Daniel has written the version to use
for the back-branches.

Author: Michael Paquier, Daniel Gustaffson
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/20191205083252[email protected]
Discussion: https://postgr.es/m/98F7F99E-1129-41D8-B86B-FE3B1E286881@yesql.se
Backpatch-through: 11

Fix whitespace.

Ensure maxlen is at leat 1 in dict_int

The dict_int text search dictionary template accepts maxlen parameter,
which is then used to cap the length of input strings. The value was
not properly checked, and the code simply does

    txt[d->maxlen] = '\0';

to insert a terminator, leading to segfaults with negative values.

This commit simply rejects values less than 1. The issue was there since
dct_int was introduced in 9.3, so backpatch all the way back to 9.4
which is the oldest supported version.

Reported-by: cili
Discussion: https://postgr.es/m/16144-a36a5bef7657047d@postgresql.org
Backpatch-through: 9.4

Fix failures with TAP tests of pg_ctl on Windows

On Windows, all the hosts spawned by the TAP tests bind to 127.0.0.1.
Hence, if there is a port conflict, starting a cluster would immediately
fail.  One of the test scripts of pg_ctl initializes a node without
PostgresNode.pm, using the default port 5432.  This could cause
unexpected startup failures in the tests if an independent server was up
and running on the same host (the reverse is also possible, though more
unlikely).  Fix this issue by assigning properly a free port to the node
configured, in the same range used as for the other nodes part of the
tests.

Author: Michael Paquier
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/20191202031444[email protected]
Backpatch-through: 11

Fix misbehavior with expression indexes on ON COMMIT DELETE ROWS tables.

We implement ON COMMIT DELETE ROWS by truncating tables marked that
way, which requires also truncating/rebuilding their indexes.  But
RelationTruncateIndexes asks the relcache for up-to-date copies of any
index expressions, which may cause execution of eval_const_expressions
on them, which can result in actual execution of subexpressions.
This is a bad thing to have happening during ON COMMIT.  Manuel Rigger
reported that use of a SQL function resulted in crashes due to
expectations that ActiveSnapshot would be set, which it isn't.
The most obvious fix perhaps would be to push a snapshot during
PreCommit_on_commit_actions, but I think that would just open the door
to more problems: CommitTransaction explicitly expects that no
user-defined code can be running at this point.

Fortunately, since we know that no tuples exist to be indexed, there
seems no need to use the real index expressions or predicates during
RelationTruncateIndexes.  We can set up dummy index expressions
instead (we do need something that will expose the right data type,
as there are places that build index tupdescs based on this), and
just ignore predicates and exclusion constraints.

In a green field it'd likely be better to reimplement ON COMMIT DELETE
ROWS using the same "init fork" infrastructure used for unlogged
relations.  That seems impractical without catalog changes though,
and even without that it'd be too big a change to back-patch.
So for now do it like this.

Per private report from Manuel Rigger.  This has been broken forever,
so back-patch to all supported branches.

Fix off-by-one error in PGTYPEStimestamp_fmt_asc

When using %b or %B patterns to format a date, the code was simply using
tm_mon as an index into array of month names. But that is wrong, because
tm_mon is 1-based, while array indexes are 0-based. The result is we
either use name of the next month, or a segfault (for December).

Fix by subtracting 1 from tm_mon for both patterns, and add a regression
test triggering the issue. Backpatch to all supported versions (the bug
is there far longer, since at least 2003).

Reported-by: Paul Spencer
Backpatch-through: 9.4
Discussion: https://postgr.es/m/16143-0d861eb8688d3fef%40postgresql.org

Remove unnecessary clauses_attnums variable

Commit c676e659b2 reworked how choose_best_statistics() picks the best
extended statistics, but failed to remove clauses_attnums which is now
unnecessary. So get rid of it and backpatch to 12, same as c676e659b2.

Author: Tomas Vondra
Discussion: https://postgr.es/m/CA+u7OA7H5rcE2=8f263w4NZD6ipO_XOrYB816nuLXbmSTH9pQQ@mail.gmail.com
Backpatch-through: 12

Fix choose_best_statistics to check clauses individually

When picking the best extended statistics object for a list of clauses,
it's not enough to look at attnums extracted from the clause list as a
whole. Consider for example this query with OR clauses:

   SELECT * FROM t WHERE (t.a = 1) OR (t.b = 1) OR (t.c = 1)

with a statistics defined on columns (a,b). Relying on attnums extracted
from the whole OR clause, we'd consider the statistics usable. That does
not work, as we see the conditions as a single OR-clause, referencing an
attribute not covered by the statistic, leading to empty list of clauses
to be estimated using the statistics and an assert failure.

This changes choose_best_statistics to check which clauses are actually
covered, and only using attributes from the fully covered ones. For the
previous example this means the statistics object will not be considered
as compatible with the OR-clause.

Backpatch to 12, where MCVs were introduced. The issue does not affect
older versions because functional dependencies don't handle OR clauses.

Author: Tomas Vondra
Reviewed-by: Dean Rasheed
Reported-By: Manuel Rigger
Discussion: https://postgr.es/m/CA+u7OA7H5rcE2=8f263w4NZD6ipO_XOrYB816nuLXbmSTH9pQQ@mail.gmail.com
Backpatch-through: 12

Fix typo in comment.

Allow access to child table statistics if user can read parent table.

The fix for CVE-2017-7484 disallowed use of pg_statistic data for
planning purposes if the user would not be able to select the associated
column and a non-leakproof function is to be applied to the statistics
values.  That turns out to disable use of pg_statistic data in some
common cases involving inheritance/partitioning, where the user does
have permission to select from the parent table that was actually named
in the query, but not from a child table whose stats are needed.  Since,
in non-corner cases, the user *can* select the child table's data via
the parent, this restriction is not actually useful from a security
standpoint.  Improve the logic so that we also check the permissions of
the originally-named table, and allow access if select permission exists
for that.

When checking access to stats for a simple child column, we can map
the child column number back to the parent, and perform this test
exactly (including not allowing access if the child column isn't
exposed by the parent).  For expression indexes, the current logic
just insists on whole-table select access, and this patch allows
access if the user can select the whole parent table.  In principle,
if the child table has extra columns, this might allow access to
stats on columns the user can't read.  In practice, it's unlikely
that the planner is going to do any stats calculations involving
expressions that are not visible to the query, so we'll ignore that
fine point for now.  Perhaps someday we'll improve that logic to
detect exactly which columns are used by an expression index ...
but today is not that day.

Back-patch to v11.  The issue was created in 9.2 and up by the
CVE-2017-7484 fix, but this patch depends on the append_rel_array[]
planner data structure which only exists in v11 and up.  In
practice the issue is most urgent with partitioned tables, so
fixing v11 and later should satisfy much of the practical need.

Dilip Kumar and Amit Langote, with some kibitzing by me

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

Don't shut down Gather[Merge] early under Limit.

Revert part of commit 19df1702f5.

Early shutdown was added by that commit so that we could collect
statistics from workers, but unfortunately, it interacted badly with
rescans.  The problem is that we ended up destroying the parallel context
which is required for rescans.  This leads to rescans of a Limit node over
a Gather node to produce unpredictable results as it tries to access
destroyed parallel context.  By reverting the early shutdown code, we
might lose statistics in some cases of Limit over Gather [Merge], but that
will require further study to fix.

Reported-by: Jerry Sievers
Diagnosed-by: Thomas Munro
Author: Amit Kapila, testcase by Vignesh C
Backpatch-through: 9.6
Discussion: https://postgr.es/m/[email protected]

Avoid assertion failure with LISTEN in a serializable transaction.

If LISTEN is the only action in a serializable-mode transaction,
and the session was not previously listening, and the notify queue
is not empty, predicate.c reported an assertion failure.  That
happened because we'd acquire the transaction's initial snapshot
during PreCommit_Notify, which was called *after* predicate.c
expects any such snapshot to have been established.

To fix, just swap the order of the PreCommit_Notify and
PreCommit_CheckForSerializationFailure calls during CommitTransaction.
This will imply holding the notify-insertion lock slightly longer,
but the difference could only be meaningful in serializable mode,
which is an expensive option anyway.

It appears that this is just an assertion failure, with no
consequences in non-assert builds.  A snapshot used only to scan
the notify queue could not have been involved in any serialization
conflicts, so there would be nothing for
PreCommit_CheckForSerializationFailure to do except assign it a
prepareSeqNo and set the SXACT_FLAG_PREPARED flag.  And given no
conflicts, neither of those omissions affect the behavior of
ReleasePredicateLocks.  This admittedly once-over-lightly analysis
is backed up by the lack of field reports of trouble.

Per report from Mark Dilger.  The bug is old, so back-patch to all
supported branches; but the new test case only goes back to 9.6,
for lack of adequate isolationtester infrastructure before that.

Discussion: https://postgr.es/m/3ac7f397-4d5f-be8e-f354-440020675694@gmail.com
Discussion: https://postgr.es/m/13881.1574557302@sss.pgh.pa.us

doc: Fix whitespace in syntax.

Back-patch to 10.

Author: Andreas Karlsson
Discussion: https://postgr.es/m/043acae2-a369-b7fa-be48-1933aa2e82d1%40proxel.se

Stabilize NOTIFY behavior by transmitting notifies before ReadyForQuery.

This patch ensures that, if any notify messages were received during
a just-finished transaction, they get sent to the frontend just before
not just after the ReadyForQuery message.  With libpq and other client
libraries that act similarly, this guarantees that the client will see
the notify messages as available as soon as it thinks the transaction
is done.

This probably makes no difference in practice, since in realistic
use-cases the application would have to cope with asynchronous
arrival of notify events anyhow.  However, it makes it a lot easier
to build cross-session-notify test cases with stable behavior.
I'm a bit surprised now that we've not seen any buildfarm instability
with the test cases added by commit b10f40bf0.  Tests that I intend
to add in an upcoming bug fix are definitely unstable without this.

Back-patch to 9.6, which is as far back as we can do NOTIFY testing
with the isolationtester infrastructure.

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

Improve test coverage for LISTEN/NOTIFY.

Back-patch commit b10f40bf0 into older branches.  This adds reporting
of NOTIFY messages to isolationtester.c, and extends the async-notify
test to include direct tests of basic NOTIFY functionality.

This provides useful infrastructure for testing a bug fix I'm about
to back-patch, and there seems no good reason not to have better tests
of LISTEN/NOTIFY in the back branches.  The commit's survived long
enough in HEAD to make it unlikely that it will cause problems.

Back-patch as far as 9.6.  isolationtester.c changed too much in 9.6
to make it sane to try to fix older branches this way, and I don't
really want to back-patch those changes too.

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

Add test coverage for "unchanged toast column" replication code path.

It seems pretty unacceptable to have no regression test coverage
for this aspect of the logical replication protocol, especially
given the bugs we've found in related code.

Discussion: https://postgr.es/m/16129-a0c0f48e71741e5f@postgresql.org

Fix bogus tuple-slot management in logical replication UPDATE handling.

slot_modify_cstrings seriously abused the TupleTableSlot API by relying
on a slot's underlying data to stay valid across ExecClearTuple.  Since
this abuse was also quite undocumented, it's little surprise that the
case got broken during the v12 slot rewrites.  As reported in bug #16129
from Ondřej Jirman, this could lead to crashes or data corruption when
a logical replication subscriber processes a row update.  Problems would
only arise if the subscriber's table contained columns of pass-by-ref
types that were not being copied from the publisher.

Fix by explicitly copying the datum/isnull arrays from the source slot
that the old row was in already.  This ends up being about the same
thing that happened pre-v12, but hopefully in a less opaque and
fragile way.

We might've caught the problem sooner if there were any test cases
dealing with updates involving non-replicated or dropped columns.
Now there are.

Back-patch to v10 where this code came in.  Even though the failure
does not manifest before v12, IMO this code is too fragile to leave
as-is.  In any case we certainly want the additional test coverage.

Patch by me; thanks to Tomas Vondra for initial investigation.

Discussion: https://postgr.es/m/16129-a0c0f48e71741e5f@postgresql.org

Defend against self-referential views in relation_is_updatable().

While a self-referential view doesn't actually work, it's possible
to create one, and it turns out that this breaks some of the
information_schema views.  Those views call relation_is_updatable(),
which neglected to consider the hazards of being recursive.  In
older PG versions you get a "stack depth limit exceeded" error,
but since v10 it'd recurse to the point of stack overrun and crash,
because commit a4c35ea1c took out the expression_returns_set() call
that was incidentally checking the stack depth.

Since this function is only used by information_schema views, it
seems like it'd be better to return "not updatable" than suffer
an error.  Hence, add tracking of what views we're examining,
in just the same way that the nearby fireRIRrules() code detects
self-referential views.  I added a check_stack_depth() call too,
just to be defensive.

Per private report from Manuel Rigger.  Back-patch to all
supported versions.

Provide statistics for hypothetical BRIN indexes

Trying to use hypothetical indexes with BRIN currently fails when trying
to access a relation that does not exist when looking for the
statistics.  With the current API, it is not possible to easily pass
a value for pages_per_range down to the hypothetical index, so this
makes use of the default value of BRIN_DEFAULT_PAGES_PER_RANGE, which
should be fine enough in most cases.

Being able to refine or enforce the hypothetical costs in more
optimistic ways would require more refactoring by filling in the
statistics when building IndexOptInfo in plancat.c.  This would involve
ABI breakages around the costing routines, something not fit for stable
branches.

This is broken since 7e534ad, so backpatch down to v10.

Author: Julien Rouhaud, Heikki Linnakangas
Reviewed-by: Álvaro Herrera, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAOBaU_ZH0LKEA8VFCocr6Lpte1ab0b6FpvgS0y4way+RPSXfYg@mail.gmail.com
Backpatch-through: 10

Remove incorrect markup

Author: Daniel Gustafsson 

Handle ReadFile() EOF correctly on Windows.

When ReadFile() encounters the end of a file while reading from
a synchronous handle with an offset provided via OVERLAPPED, it
reports an error instead of returning 0.  By not handling that
(undocumented) result correctly, we caused some noisy LOG
messages about an unknown error code.  Repair.

Back-patch to 12, where we started using pread()/ReadFile() with
an offset.

Reported-by: ZhenHua Cai, Amit Kapila
Diagnosed-by: Juan Jose Santamaria Flecha
Tested-by: Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1LK3%2BWRtpz68TiRdpHwxxWm%3D%2Bt1BMf-G68hhQsAQ41PZg%40mail.gmail.com

Doc: fix minor typo in func.sgml.

Discussion: https://postgr.es/m/20191119.222048.49467220816510881.t-ishii%40sraoss.co.jp

Fix corner-case failure in match_pattern_prefix().

The planner's optimization code for LIKE and regex operators could
error out with a complaint like "no = operator for opfamily NNN"
if someone created a binary-compatible index (for example, a
bpchar_ops index on a text column) on the LIKE's left argument.

This is a consequence of careless refactoring in commit 74dfe58a5.
The old code in match_special_index_operator only accepted specific
combinations of the pattern operator and the index opclass, thereby
indirectly guaranteeing that the opclass would have a comparison
operator with the same LHS input type as the pattern operator.
While moving the logic out to a planner support function, I simplified
that test in a way that no longer guarantees that.  Really though we'd
like an altogether weaker dependency on the opclass, so rather than
put back exactly the old code, just allow lookup failure.  I have in
mind now to rewrite this logic completely, but this is the minimum
change needed to fix the bug in v12.

Per report from Manuel Rigger.  Back-patch to v12 where the mistake
came in.

Discussion: https://postgr.es/m/CA+u7OA7nnGYy8rY0vdTe811NuA+Frr9nbcBO9u2Z+JxqNaud+g@mail.gmail.com

Fix page modification outside of critical section in GIN

By oversight 52ac6cd2d0 makes ginDeletePage() sets pd_prune_xid of page to be
deleted before entering the critical section.  It appears that only versions 11
and later were affected by this oversight.

Backpatch-through: 11

Revise GIN README

We find GIN concurrency bugs from time to time.  One of the problems here is
that concurrency of GIN isn't well-documented in README.  So, it might be even
hard to distinguish design bugs from implementation bugs.

This commit revised concurrency section in GIN README providing more details.
Some examples are illustrated in ASCII art.

Also, this commit add the explanation of how is tuple layout in internal GIN
B-tree page different in comparison with nbtree.

Discussion: https://postgr.es/m/CAPpHfduXR_ywyaVN4%2BOYEGaw%3DcPLzWX6RxYLBncKw8de9vOkqw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Peter Geoghegan
Backpatch-through: 9.4

Fix traversing to the deleted GIN page via downlink

Current GIN code appears to don't handle traversing to the deleted page via
downlink.  This commit fixes that by stepping right from the delete page like
we do in nbtree.

This commit also fixes setting 'deleted' flag to the GIN pages.  Now other page
flags are not erased once page is deleted.  That helps to keep our assertions
true if we arrive deleted page via downlink.

Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Peter Geoghegan
Backpatch-through: 9.4

Fix deadlock between ginDeletePage() and ginStepRight()

When ginDeletePage() is about to delete page it locks its left sibling to revise
the rightlink.  So, it locks pages in right to left manner.  Int he same time
ginStepRight() locks pages in left to right manner, and that could cause a
deadlock.

This commit makes ginScanToDelete() keep exclusive lock on left siblings of
currently investigated path.  That elimites need to relock left sibling in
ginDeletePage().  Thus, deadlock with ginStepRight() can't happen anymore.

Reported-by: Chen Huajun
Discussion: https://postgr.es/m/5c332bd1.87b6.16d7c17aa98.Coremail.chjischj%40163.com
Author: Alexander Korotkov
Reviewed-by: Peter Geoghegan
Backpatch-through: 10

Doc: clarify use of RECURSIVE in WITH.

Apparently some people misinterpreted the syntax as being that
RECURSIVE is a prefix of individual WITH queries.  It's a modifier
for the WITH clause as a whole, so state that more clearly.

Discussion: https://postgr.es/m/ca53c6ce-a0c6-b14a-a8e3-162f0b2cc119@a-kretschmer.de

Doc: clarify behavior of ALTER DEFAULT PRIVILEGES ... IN SCHEMA.

The existing text stated that "Default privileges that are specified
per-schema are added to whatever the global default privileges are for
the particular object type".  However, that bare-bones observation is
not quite clear enough, as demonstrated by the complaint in bug #16124.
Flesh it out by stating explicitly that you can't revoke built-in
default privileges this way, and by providing an example to drive
the point home.

Back-patch to all supported branches, since it's been like this
from the beginning.

Discussion: https://postgr.es/m/16124-423d8ee4358421bc@postgresql.org

Further fix dumping of views that contain just VALUES(...).

It turns out that commit e9f1c01b7 missed a case: we must print a
VALUES clause in long format if get_query_def is given a resultDesc
that would require the query's output column name(s) to be different
from what the bare VALUES clause would produce.

This applies in case an ALTER ... RENAME COLUMN has been done to
a view that formerly could be printed in simple format, as shown
in the added regression test case.  It also explains bug #16119
from Dmitry Telpt, because it turns out that (unlike CREATE VIEW)
CREATE MATERIALIZED VIEW fails to apply any column aliases it's
given to the stored ON SELECT rule.  So to get them to be printed,
we have to account for the resultDesc renaming.  It might be worth
changing the matview code so that it creates the ON SELECT rule
with the correct aliases; but we'd still need these messy checks in
get_simple_values_rte to handle the case of a subsequent column
rename, so any such change would be just neatnik-ism not a bug fix.

Like the previous patch, back-patch to all supported branches.

Discussion: https://postgr.es/m/16119-e64823f30a45a754@postgresql.org

Improve stability of tests for VACUUM (SKIP_LOCKED)

Concurrent autovacuums running with the main regression test suite
could cause the tests with VACUUM (SKIP_LOCKED) to generate randomly
WARNING messages.  For these tests, set client_min_messages to ERROR to
get rid of those random failures, as disabling autovacuum for the
relations operated would not completely close the failure window.

For isolation tests, disable autovacuum for the relations vacuumed with
SKIP_LOCKED.  The tests are designed so as LOCK commands are taken
in a first session before running a concurrent VACUUM (SKIP_LOCKED) in a
second to generate WARNING messages, but a concurrent autovacuum could
cause the tests to be slower.

Reported-by: Tom Lane
Author: Michael Paquier
Reviewed-by: Andres Freund, Tom Lane
Discussion: https://postgr.es/m/25294.1573077278@sss.pgh.pa.us
Backpatch-through: 12

Skip system attributes when applying mvdistinct stats

When estimating number of distinct groups, we failed to ignore system
attributes when matching the group expressions to mvdistinct stats,
causing failures like

  ERROR: negative bitmapset member not allowed

Fix that by simply skipping anything that is not a regular attribute.
Backpatch to PostgreSQL 10, where the extended stats were introduced.

Bug: #16111
Reported-by: Tuomas Leikola
Author: Tomas Vondra
Backpatch-through: 10
Discussion: https://postgr.es/m/16111-687799584c3a7e73@postgresql.org

Always call ExecShutdownNode() if appropriate.

Call ExecShutdownNode() after ExecutePlan()'s loop, rather than at each
break.  We had forgotten to do that in one case.  The omission caused
intermittent "temporary file leak" warnings from multi-batch parallel
hash joins with a LIMIT clause.

Back-patch to 11.  Though the problem exists in theory in earlier
parallel query releases, nothing really depended on it.

Author: Kyotaro Horiguchi
Reviewed-by: Thomas Munro, Amit Kapila
Discussion: https://postgr.es/m/20191111.212418.2222262873417235945.horikyota.ntt%40gmail.com

Doc: in v12 release notes, explain how to replace uses of consrc and adsrc.

While you can find that info if you drill down far enough, it seems more
helpful to put something right in the compatibility notes.  Per a question
from Ivan Sergio Borgonovo.

Discussion: https://postgr.es/m/a6359855-2a5e-a56c-ebba-4ea46a1f0ebe@webthatworks.it

Add missing check_collation_set call to bpcharne().

We should throw an error for indeterminate collation, but bpcharne()
was missing that logic, resulting in a much less user-friendly error
(either an assertion failure or "cache lookup failed for collation 0").

Per report from Manuel Rigger.  Back-patch to v12 where the mistake
came in, evidently in commit 5e1963fb7.  (Before non-deterministic
collations, this function wasn't collation sensitive.)

Discussion: https://postgr.es/m/CA+u7OA4HOjtymxAbuGNh4-X_2R0Lw5n01tzvP8E5-i-2gQXYWA@mail.gmail.com

Fix silly initializations (cosmetic only).

Initializing a pointer to "false" isn't per project style,
and reportedly some compilers warn about it (though I've
not seen any such warnings in the buildfarm).

Seems to have come in with commit ff11e7f4b, so back-patch
to v12 where that was added.

Didier Gautheron

Discussion: https://postgr.es/m/CAJRYxu+XQuM0qnSqt1Ujztu6fBPzMMAT3VEn6W32rgKG6A2Fsw@mail.gmail.com

Avoid downcasing/truncation of RADIUS authentication parameters.

Commit 6b76f1bb5 changed all the RADIUS auth parameters to be lists
rather than single values.  But its use of SplitIdentifierString
to parse the list format was not very carefully thought through,
because that function thinks it's parsing SQL identifiers, which
means it will (a) downcase the strings and (b) truncate them to
be shorter than NAMEDATALEN.  While downcasing should be harmless
for the server names and ports, it's just wrong for the shared
secrets, and probably for the NAS Identifier strings as well.
The truncation aspect is at least potentially a problem too,
though typical values for these parameters would fit in 63 bytes.

Fortunately, we now have a function SplitGUCList that is exactly
the same except for not doing the two unwanted things, so fixing
this is a trivial matter of calling that function instead.

While here, improve the documentation to show how to double-quote
the parameter values.  I failed to resist the temptation to do
some copy-editing as well.

Report and patch from Marcos David (bug #16106); doc changes by me.
Back-patch to v10 where the aforesaid commit came in, since this is
arguably a regression from our previous behavior with RADIUS auth.

Discussion: https://postgr.es/m/16106-7d319e4295d08e70@postgresql.org

Include TableFunc references when computing expression dependencies.

The TableFunc node (i.e., XMLTABLE) includes type and collation OIDs
that might not be referenced anywhere else in the expression tree,
so they need to be accounted for when extracting dependencies.

Fortunately, the practical effects of this are limited, since
(a) it's somewhat unlikely that people would be extracting
columns of non-builtin types from an XML document, and (b)
in many scenarios, the query would contain other references
to such types, or functions depending on them.  However, it's
not hard to construct examples wherein the existing code lets
one drop a type used in XMLTABLE and thereby break a view.

This is evidently an original oversight in the XMLTABLE patch,
so back-patch to v10 where that came in.

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

Handle arrays and ranges in pg_upgrade's test for non-upgradable types.

pg_upgrade needs to check whether certain non-upgradable data types
appear anywhere on-disk in the source cluster.  It knew that it has
to check for these types being contained inside domains and composite
types; but it somehow overlooked that they could be contained in
arrays and ranges, too.  Extend the existing recursive-containment
query to handle those cases.

We probably should have noticed this oversight while working on
commit 0ccfc2822 and follow-ups, but we failed to :-(.  The whole
thing's possibly a bit overdesigned, since we don't really expect
that any of these types will appear on disk; but if we're going to
the effort of doing a recursive search then it's silly not to cover
all the possibilities.

While at it, refactor so that we have only one copy of the search
logic, not three-and-counting.  Also, to keep the branches looking
more alike, back-patch the output wording change of commit 1634d3615.

Back-patch to all supported branches.

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

docs:  clarify that only INSERT and UPDATE triggers can mod. NEW

The point is that DELETE triggers cannot modify any values.

Reported-by: Eugen Konkov, Liudmila Mantrova
Discussion: https://postgr.es/m/919823407.20191029175436@yandex.ru

Backpatch-through: 12 only, where commit as missing

Stamp 12.1.

Doc: fix ancient mistake, or at least obsolete info, in rules example.

The example of expansion of multiple views claimed that the resulting
subquery nest would not get fully flattened because of an aggregate
function.  There's no aggregate in the example, though, only a user
defined function confusingly named MIN().  In a modern server, the
reason for the non-flattening is that MIN() is volatile, but I'm
unsure whether that was true back when this text was written.

Let's reduce the confusion level by using LEAST() instead (which
we didn't have at the time this example was created).  And then
we can just say that the planner will flatten the sub-queries, so
the rewrite system doesn't have to.

Noted by Paul Jungwirth.  This text is old enough to vote, so
back-patch to all supported branches.

Discussion: https://postgr.es/m/CA+renyXZFnmp9PcvX1EVR2dR=XG5e6E-AELr8AHCNZ8RYrpnPw@mail.gmail.com