configure: Update python search order

Some systems don't ship with "python" by default anymore, only
"python3" or "python2" or some combination, so include those in the
configure search.

Discussion: https://www.postgresql.org/message-id/flat/1457.1543184081%40sss.pgh.pa.us#c9cc1199338fd6a257589c6dcea6cf8d

Fix up confusion over how to use EXTRA_INSTALL.

Some makefiles were trying to do this:

temp-install: EXTRA_INSTALL=contrib/test_decoding

but that no longer works as of commit aa019da52: the macro is now
consulted by the checkprep target, one level down, and apparently
gmake doesn't propagate such macro settings recursively.

The problem is masked since 42e61c774 because pgxs.mk also sets up
EXTRA_INSTALL, and correctly applies it to the checkprep target.

Unfortunately I'd not risked back-patching that to before v11.
Since aa019da52 was pushed back to v10, it broke test_decoding
there (the only module for which this actually makes a difference
at present).

Hence, back-patch 42e61c774 to v10.  Also, remove some demonstrably
useless settings of EXTRA_INSTALL in v10 and v11 (they'd already
been cleaned up in HEAD).

Per buildfarm.

Discussion: https://postgr.es/m/CAEepm=1pEJdwv6DSGmOfpX0EaX7L7sT28c1nXpqvQvmLfEWb1g@mail.gmail.com

Avoid sharing PARAM_EXEC slots between different levels of NestLoop.

Up to now, createplan.c attempted to share PARAM_EXEC slots for
NestLoopParams across different plan levels, if the same underlying Var
was being fed down to different righthand-side subplan trees by different
NestLoops.  This was, I think, more of an artifact of using subselect.c's
PlannerParamItem infrastructure than an explicit design goal, but anyway
that was the end result.

This works well enough as long as the plan tree is executing synchronously,
but the feature whereby Gather can execute the parallelized subplan locally
breaks it.  An upper NestLoop node might execute for a row retrieved from
a parallel worker, and assign a value for a PARAM_EXEC slot from that row,
while the leader's copy of the parallelized subplan is suspended with a
different active value of the row the Var comes from.  When control
eventually returns to the leader's subplan, it gets the wrong answers if
the same PARAM_EXEC slot is being used within the subplan, as reported
in bug #15577 from Bartosz Polnik.

This is pretty reminiscent of the problem fixed in commit 46c508fbc, and
the proper fix seems to be the same: don't try to share PARAM_EXEC slots
across different levels of controlling NestLoop nodes.

This requires decoupling NestLoopParam handling from PlannerParamItem
handling, although the logic remains somewhat similar.  To avoid bizarre
division of labor between subselect.c and createplan.c, I decided to move
all the param-slot-assignment logic for both cases out of those files
and put it into a new file paramassign.c.  Hopefully it's a bit better
documented now, too.

A regression test case for this might be nice, but we don't know a
test case that triggers the problem with a suitably small amount
of data.

Back-patch to 9.6 where we added Gather nodes.  It's conceivable that
related problems exist in older branches; but without some evidence
for that, I'll leave the older branches alone.

Discussion: https://postgr.es/m/15577-ca61ab18904af852@postgresql.org

doc: Correct documentation of install-time environment variables

Since approximately PostgreSQL 10, it is no longer required that
environment variables at installation time such as PERL, PYTHON, TCLSH
be "full path names", so change that phrasing in the installation
instructions.  (The exact time of change appears to differ for PERL
and the others, but it works consistently in PostgreSQL 10.)

Also while we're here document the defaults for PERL and PYTHON, but
since the search list for TCLSH is so long, let's leave that out so we
don't need to maintain a copy of that list in the installation
instructions.

Doc: update our docs about kernel IPC parameters on *BSD.

runtime.sgml said that you couldn't change SysV IPC parameters on OpenBSD
except by rebuilding the kernel.  That's definitely wrong in OpenBSD 6.x,
and excavation in their man pages says it changed in OpenBSD 3.3.

Update NetBSD and OpenBSD sections to recommend adjustment of the SEMMNI
and SEMMNS settings, which are painfully small by default on those
platforms.  (The discussion thread contemplated recommending that
people select POSIX semaphores instead, but the performance consequences
of that aren't really clear, so I'll refrain.)

Remove pointless discussion of SEMMNU and SEMMAP from the FreeBSD
section.  Minor other wordsmithing.

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

doc: document that INFO messages always go to client.

In passing add a couple of links to the message severity table.

Backpatch because it's always been this way.

Author: Karl O. Pinc <kop@meme.com>

Improve ANALYZE's handling of concurrent-update scenarios.

This patch changes the rule for whether or not a tuple seen by ANALYZE
should be included in its sample.

When we last touched this logic, in commit 51e1445f1, we weren't
thinking very hard about tuples being UPDATEd by a long-running
concurrent transaction.  In such a case, we might see the pre-image as
either LIVE or DELETE_IN_PROGRESS depending on timing; and we might see
the post-image not at all, or as INSERT_IN_PROGRESS.  Since the existing
code will not sample either DELETE_IN_PROGRESS or INSERT_IN_PROGRESS
tuples, this leads to concurrently-updated rows being omitted from the
sample entirely.  That's not very helpful, and it's especially the wrong
thing if the concurrent transaction ends up rolling back.

The right thing seems to be to sample DELETE_IN_PROGRESS rows just as if
they were live.  This makes the "sample it" and "count it" decisions the
same, which seems good for consistency.  It's clearly the right thing
if the concurrent transaction ends up rolling back; in effect, we are
sampling as though IN_PROGRESS transactions haven't happened yet.
Also, this combination of choices ensures maximum robustness against
the different combinations of whether and in which state we might see the
pre- and post-images of an update.

It's slightly annoying that we end up recording immediately-out-of-date
stats in the case where the transaction does commit, but on the other
hand the stats are fine for columns that didn't change in the update.
And the alternative of sampling INSERT_IN_PROGRESS rows instead seems
like a bad idea, because then the sampling would be inconsistent with
the way rows are counted for the stats report.

Per report from Mark Chambers; thanks to Jeff Janes for diagnosing
what was happening.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/CAFh58O_Myr6G3tcH3gcGrF-=OExB08PJdWZcSBcEcovaiPsrHA@mail.gmail.com

Update ssl test certificates and keys

Debian testing and newer now require that RSA and DHE keys are at
least 2048 bit long and no longer allow SHA-1 for signatures in
certificates.  This is currently causing the ssl tests to fail there
because the test certificates and keys have been created in violation
of those conditions.

Update the parameters to create the test files and create a new set of
test files.

Author: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Reported-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/20180917131340.GE31460%40paquier.xyz

Don't believe MinMaxExpr is leakproof without checking.

MinMaxExpr invokes the btree comparison function for its input datatype,
so it's only leakproof if that function is.  Many such functions are
indeed leakproof, but others are not, and we should not just assume that
they are.  Hence, adjust contain_leaked_vars to verify the leakproofness
of the referenced function explicitly.

I didn't add a regression test because it would need to depend on
some particular comparison function being leaky, and that's a moving
target, per discussion.

This has been wrong all along, so back-patch to supported branches.

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

Update copyright for 2019

Backpatch-through: certain files through 9.4

Fix generation of padding message before encrypting Elgamal in pgcrypto

fe0a0b5, which has added a stronger random source in Postgres, has
introduced a thinko when creating a padding message which gets encrypted
for Elgamal.  The padding message cannot have zeros, which are replaced
by random bytes.  However if pg_strong_random() failed, the message
would finish by being considered in correct shape for encryption with
zeros.

Author: Tom Lane
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/20186.1546188423@sss.pgh.pa.us
Backpatch-through: 10

Process EXTRA_INSTALL serially, during the first temp-install.

This closes a race condition in "make -j check-world"; the symptom was
EEXIST errors.  Back-patch to v10, before which parallel check-world had
worse problems.

Discussion: https://postgr.es/m/20181224221601.GA3227827@rfd.leadboat.com

Send EXTRA_INSTALL errors to install.log, not stderr.

We already redirected other temp-install stderr and all temp-install
stdout in this way.  Back-patch to v10, like the next commit.

Discussion: https://postgr.es/m/20181224221601.GA3227827@rfd.leadboat.com

pg_regress: Promptly detect failed postmaster startup.

Detect it the way pg_ctl's wait_for_postmaster() does.  When pg_regress
spawned a postmaster that failed startup, we were detecting that only
with "pg_regress: postmaster did not respond within 60 seconds".
Back-patch to 9.4 (all supported versions).

Reviewed by Tom Lane.

Discussion: https://postgr.es/m/20181231172922.GA199150@gust.leadboat.com

pg_rewind: Add missing newline to error message

Fix latent problem with pg_jrand48().

POSIX specifies that jrand48() returns a signed 32-bit value (in the
range [-2^31, 2^31)), but our code was returning an unsigned 32-bit
value (in the range [0, 2^32)).  This doesn't actually matter to any
existing call site, because they all cast the "long" result to int32
or uint32; but it will doubtless bite somebody in the future.
To fix, cast the arithmetic result to int32 explicitly before the
compiler widens it to long (if widening is needed).

While at it, upgrade this file's far-short-of-project-style comments.
Had there been some peer pressure to document pg_jrand48() properly,
maybe this thinko wouldn't have gotten committed to begin with.

Backpatch to v10 where pg_jrand48() was added, just in case somebody
back-patches a fix that uses it and depends on the standard behavior.

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

Have DISCARD ALL/TEMP remove leftover temp tables

Previously, it would only remove temp tables created in the same
session; but if the session uses the BackendId of a previously crashed
backend that left temp tables around, those would not get removed.
Since autovacuum would not drop them either (because it sees that the
BackendId is in use by the current session) these can cause annoying
xid-wraparound warnings.

Apply to branches 9.4 to 10.  This is not a problem since version 11,
because commit 943576bddcb5 added state tracking that makes autovacuum
realize that those temp tables are not ours, so it removes them.

This is useful to handle in DISCARD, because even though it does not
handle all situations, it does handle the common one where a connection
pooler keeps the same session open for an indefinitely long time.

Discussion: https://postgr.es/m/20181226190834.wsk2wzott5yzrjiq@alvherre.pgsql
Reviewed-by: Takayuki Tsunakawa, Michaël Paquier

Make autovacuum more selective about temp tables to keep

When temp tables are in danger of XID wraparound, autovacuum drops them;
however, it preserves those that are owned by a working session.  This
is desirable, except when the session is connected to a different
database (because the temp tables cannot be from that session), so make
it only keep the temp tables only if the backend is in the same database
as the temp tables.

This is not bulletproof: it fails to detect temp tables left by a
session whose backend ID is reused in the same database but the new
session does not use temp tables.  Commit 943576bddcb5 fixes that case
too, for branches 11 and up (which is why we don't apply this fix to
those branches), but back-patching that one is not universally agreed
on.

Discussion: https://postgr.es/m/20181214162843.37g6h3txto43akrb@alvherre.pgsql
Reviewed-by: Takayuki Tsunakawa, Michaël Paquier

Ignore inherited temp relations from other sessions when truncating

Inheritance trees can include temporary tables if the parent is
permanent, which makes possible the presence of multiple temporary
children from different sessions.  Trying to issue a TRUNCATE on the
parent in this scenario causes a failure, so similarly to any other
queries just ignore such cases, which makes TRUNCATE work
transparently.

This makes truncation behave similarly to any other DML query working on
the parent table with queries which need to be issues on children.  A
set of isolation tests is added to cover basic cases.

Reported-by: Zhou Digoal
Author: Amit Langote, Michael Paquier
Discussion: https://postgr.es/m/15565-ce67a48d0244436a@postgresql.org
Backpatch-through: 9.4

Fix portability failure introduced in commits d2b0b60e7 et al.

I made a frontend fprintf() format use %m, forgetting that that's only
safe in HEAD not the back branches; prior to 96bf88d52 and d6c55de1f,
it would work on glibc platforms but not elsewhere.  Revert to using
%s ... strerror(errno) as the code did before.

We could have left HEAD as-is, but for code consistency across branches,
I chose to apply this patch there too.

Per Coverity and a few buildfarm members.

Prioritize history files when archiving

At the end of recovery for the post-promotion process, a new history
file is created followed by the last partial segment of the previous
timeline.  Based on the timing, the archiver would first try to archive
the last partial segment and then the history file.  This can delay the
detection of a new timeline taken, particularly depending on the time it
takes to transfer the last partial segment as it delays the moment the
history file of the new timeline gets archived.  This can cause promoted
standbys to use the same timeline as one already taken depending on the
circumstances if multiple instances look at archives at the same
location.

This commit changes the order of archiving so as history files are
archived in priority over other file types, which reduces the likelihood
of the same timeline being taken (still not reducing the window to
zero), and it makes the archiver behave more consistently with the
startup process doing its post-promotion business.

Author: David Steele
Reviewed-by: Michael Paquier, Kyotaro Horiguchi
Discussion: https://postgr.es/m/929068cf-69e1-bba2-9dc0-e05986aed471@pgmasters.net
Backpatch-through: 9.5

Disable WAL-skipping optimization for COPY on views

COPY can skip writing WAL when loading data on a table which has been
created in the same transaction as the one loading the data, however
this cannot work on views as this would result in trying to flush
relation files which do not exist.  So disable the optimization so as
commands are able to work the same way with any configuration of
wal_level.

A test is added to cover this case, which needs to have wal_level set to
minimal to allow the problem to show up, and that is not the default
configuration.

Reported-by: Etsuro Fujita
Author: Michael Paquier
Reviewed-by: Etsuro Fujita
Discussion: https://postgr.es/m/15552-c64aa14c5c22f63c@postgresql.org
Backpatch-through: 10, where support for COPY on views has been added,
while v11 has added support for COPY on foreign tables.

Fix ancient compiler warnings and typos in !HAVE_SYMLINK code

This has never been correct since this code was introduced.

Check for conflicting queries during replay of gistvacuumpage()

013ebc0a7b implements so-called GiST microvacuum.  That is gistgettuple() marks
index tuples as dead when kill_prior_tuple is set.  Later, when new tuple
insertion claims page space, those dead index tuples are physically deleted
from page.  When this deletion is replayed on standby, it might conflict with
read-only queries.  But 013ebc0a7b doesn't handle this.  That may lead to
disappearance of some tuples from read-only snapshots on standby.

This commit implements resolving of conflicts between replay of GiST microvacuum
and standby queries.  On the master we implement new WAL record type
XLOG_GIST_DELETE, which comprises necessary information.  On stable releases
we've to be tricky to keep WAL compatibility.  Information required for conflict
processing is just appended to data of XLOG_GIST_PAGE_UPDATE record.  So,
PostgreSQL version, which doesn't know about conflict processing, will just
ignore that.

Reported-by: Andres Freund
Diagnosed-by: Andres Freund
Discussion: https://postgr.es/m/20181212224524.scafnlyjindmrbe6%40alap3.anarazel.de
Author: Alexander Korotkov
Backpatch-through: 9.6

Fix lock level used for partition when detaching it

For probably bogus reasons, we acquire only AccessShareLock on the
partition when we try to detach it from its parent partitioned table.
This can cause ugly things to happen if another transaction is doing
any sort of DDL to the partition concurrently.

Upgrade that lock to ShareUpdateExclusiveLock, which per discussion
seems to be the minimum needed.

Reported by Robert Haas.

Discussion: https://postgr.es/m/CA+TgmoYruJQ+2qnFLtF1xQtr71pdwgfxy3Ziy-TxV28M6pEmyA@mail.gmail.com

Doc: fix ancient mistake in search_path documentation.

"$user" in a search_path string is replaced by CURRENT_USER not
SESSION_USER.  (It actually was SESSION_USER in the initial implementation,
but we changed it shortly later, and evidently forgot to fix the docs to
match.)

Noted by antonov@stdpr.ru

Discussion: https://postgr.es/m/159151fb45d490c8d31ea9707e9ba99d@stdpr.ru

Fix ADD IF NOT EXISTS used in conjunction with ALTER TABLE ONLY

The flag for IF NOT EXISTS was only being passed down in the normal
recursing case. It's been this way since originally added in 9.6 in
commit 2cd40adb85 so backpatch back to 9.6.

Doc: fix incorrect example of collecting arguments with fmgr macros.

Thinko in commit f66912b0a.  Back-patch to v10, as that was.

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

Fix ancient thinko in mergejoin cost estimation.

"rescanratio" was computed as 1 + rescanned-tuples / total-inner-tuples,
which is sensible if it's to be multiplied by total-inner-tuples or a cost
value corresponding to scanning all the inner tuples.  But in reality it
was (mostly) multiplied by inner_rows or a related cost, numbers that take
into account the possibility of stopping short of scanning the whole inner
relation thanks to a limited key range in the outer relation.  This'd
still make sense if we could expect that stopping short would result in a
proportional decrease in the number of tuples that have to be rescanned.
It does not, however.  The argument that establishes the validity of our
estimate for that number is independent of whether we scan all of the inner
relation or stop short, and experimentation also shows that stopping short
doesn't reduce the number of rescanned tuples.  So the correct calculation
is 1 + rescanned-tuples / inner_rows, and we should be sure to multiply
that by inner_rows or a corresponding cost value.

Most of the time this doesn't make much difference, but if we have
both a high rescan rate (due to lots of duplicate values) and an outer
key range much smaller than the inner key range, then the error can
be significant, leading to a large underestimate of the cost associated
with rescanning.

Per report from Vijaykumar Jain.  This thinko appears to go all the way
back to the introduction of the rescan estimation logic in commit
70fba7043, so back-patch to all supported branches.

Discussion: https://postgr.es/m/CAE7uO5hMb_TZYJcZmLAgO6iD68AkEK6qCe7i=vZUkCpoKns+EQ@mail.gmail.com

Update project link of pgBadger in documentation

The project has moved to a new place.

Reported-by: Peter Neave
Discussion: https://postgr.es/m/154474118231.5066.16352227860913505754@wrigleys.postgresql.org

Remove extra semicolons.

Reported-by: David Rowley
Author: David Rowley
Reviewed-by: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/CAKJS1f8EneeYyzzvdjahVZ6gbAHFkHbSFB5m_C0Y6TUJs9Dgdg@mail.gmail.com

Fix use-after-free bug when renaming constraints

This is an oversight from recent commit b13fd344.  While on it, tweak
the previous test with a better name for the renamed primary key.

Detected by buildfarm member prion which forces relation cache release
with -DRELCACHE_FORCE_RELEASE.  Back-patch down to 9.4 as the previous
commit.

Make constraint rename issue relcache invalidation on target relation

When a constraint gets renamed, it may have associated with it a target
relation (for example domain constraints don't have one).  Not
invalidating the target relation cache when issuing the renaming can
result in issues with subsequent commands that refer to the old
constraint name using the relation cache, causing various failures.  One
pattern spotted was using CREATE TABLE LIKE after a constraint
renaming.

Reported-by: Stuart <sfbarbee@gmail.com>
Author: Amit Langote
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/2047094.V130LYfLq4@station53.ousa.org

Make error handling in parallel pg_upgrade less bogus.

reap_child() basically ignored the possibility of either an error in
waitpid() itself or a child process failure on signal.  We don't really
need to do more than report and crash hard, but proceeding as though
nothing is wrong is definitely Not Acceptable.  The error report for
nonzero child exit status was pretty off-point, as well.

Noted while fooling around with child-process failure detection
logic elsewhere.  It's been like this a long time, so back-patch to
all supported branches.

Improve detection of child-process SIGPIPE failures.

Commit ffa4cbd62 added logic to detect SIGPIPE failure of a COPY child
process, but it only worked correctly if the SIGPIPE occurred in the
immediate child process.  Depending on the shell in use and the
complexity of the shell command string, we might instead get back
an exit code of 128 + SIGPIPE, representing a shell error exit
reporting SIGPIPE in the child process.

We could just hack up ClosePipeToProgram() to add the extra case,
but it seems like this is a fairly general issue deserving a more
general and better-documented solution.  I chose to add a couple
of functions in src/common/wait_error.c, which is a natural place
to know about wait-result encodings, that will test for either a
specific child-process signal type or any child-process signal failure.
Then, adjust other places that were doing ad-hoc tests of this type
to use the common functions.

In RestoreArchivedFile, this fixes a race condition affecting whether
the process will report an error or just silently proc_exit(1): before,
that depended on whether the intermediate shell got SIGTERM'd itself
or reported a child process failing on SIGTERM.

Like the previous patch, back-patch to v10; we could go further
but there seems no real need to.

Per report from Erik Rijkers.

Discussion: https://postgr.es/m/f3683f87ab1701bea5d86a7742b22432@xs4all.nl

Fix bogus logic for skipping unnecessary partcollation dependencies.

The idea here is to not call recordDependencyOn for the default collation,
since we know that's pinned.  But what the code actually did was to record
the partition key's dependency on the opclass twice, instead.

Evidently introduced by sloppy coding in commit 2186b608b.  Back-patch
to v10 where that came in.

Prevent GIN deleted pages from being reclaimed too early

When GIN vacuum deletes a posting tree page, it assumes that no concurrent
searchers can access it, thanks to ginStepRight() locking two pages at once.
However, since 9.4 searches can skip parts of posting trees descending from the
root.  That leads to the risk that page is deleted and reclaimed before
concurrent search can access it.

This commit prevents the risk of above by waiting for every transaction, which
might wait to reference this page, to finish.  Due to binary compatibility
we can't change GinPageOpaqueData to store corresponding transaction id.
Instead we reuse page header pd_prune_xid field, which is unused in index pages.

Discussion: https://postgr.es/m/31a702a.14dd.166c1366ac1.Coremail.chjischj%40163.com
Author: Andrey Borodin, Alexander Korotkov
Reviewed-by: Alexander Korotkov
Backpatch-through: 9.4

Prevent deadlock in ginRedoDeletePage()

On standby ginRedoDeletePage() can work concurrently with read-only queries.
Those queries can traverse posting tree in two ways.
1) Using rightlinks by ginStepRight(), which locks the next page before
   unlocking its left sibling.
2) Using downlinks by ginFindLeafPage(), which locks at most one page at time.

Original lock order was: page, parent, left sibling.  That lock order can
deadlock with ginStepRight().  In order to prevent deadlock this commit changes
lock order to: left sibling, page, parent.  Note, that position of parent in
locking order seems insignificant, because we only lock one page at time while
traversing downlinks.

Reported-by: Chen Huajun
Diagnosed-by: Chen Huajun, Peter Geoghegan, Andrey Borodin
Discussion: https://postgr.es/m/31a702a.14dd.166c1366ac1.Coremail.chjischj%40163.com
Author: Alexander Korotkov
Backpatch-through: 9.4

Fix deadlock in GIN vacuum introduced by 218f51584d5

Before 218f51584d5 if posting tree page is about to be deleted, then the whole
posting tree is locked by LockBufferForCleanup() on root preventing all the
concurrent inserts.  218f51584d5 reduced locking to the subtree containing
page to be deleted.  However, due to concurrent parent split, inserter doesn't
always holds pins on all the pages constituting path from root to the target
leaf page.  That could cause a deadlock between GIN vacuum process and GIN
inserter.  And we didn't find non-invasive way to fix this.

This commit reverts VACUUM behavior to lock the whole posting tree before
delete any page.  However, we keep another useful change by 218f51584d5: the
tree is locked only if there are pages to be deleted.

Reported-by: Chen Huajun
Diagnosed-by: Chen Huajun, Andrey Borodin, Peter Geoghegan
Discussion: https://postgr.es/m/31a702a.14dd.166c1366ac1.Coremail.chjischj%40163.com
Author: Alexander Korotkov, based on ideas from Andrey Borodin and Peter Geoghegan
Reviewed-by: Andrey Borodin
Backpatch-through: 10

Repair bogus EPQ plans generated for postgres_fdw foreign joins.

postgres_fdw's postgresGetForeignPlan() assumes without checking that the
outer_plan it's given for a join relation must have a NestLoop, MergeJoin,
or HashJoin node at the top.  That's been wrong at least since commit
4bbf6edfb (which could cause insertion of a Sort node on top) and it seems
like a pretty unsafe thing to Just Assume even without that.

Through blind good fortune, this doesn't seem to have any worse
consequences today than strange EXPLAIN output, but it's clearly trouble
waiting to happen.

To fix, test the node type explicitly before touching Join-specific
fields, and avoid jamming the new tlist into a node type that can't
do projection.  Export a new support function from createplan.c
to avoid building low-level knowledge about the latter into FDWs.

Back-patch to 9.6 where the faulty coding was added.  Note that the
associated regression test cases don't show any changes before v11,
apparently because the tests back-patched with 4bbf6edfb don't actually
exercise the problem case before then (there's no top-level Sort
in those plans).

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

Repair bogus handling of multi-assignment Params in upper plan levels.

Our support for multiple-set-clauses in UPDATE assumes that the Params
referencing a MULTIEXPR_SUBLINK SubPlan will appear before that SubPlan
in the targetlist of the plan node that calculates the updated row.
(Yeah, it's a hack...)  In some PG branches it's possible that a Result
node gets inserted between the primary calculation of the update tlist
and the ModifyTable node.  setrefs.c did the wrong thing in this case
and left the upper-level Params as Params, causing a crash at runtime.
What it should do is replace them with "outer" Vars referencing the child
plan node's output.  That's a result of careless ordering of operations
in fix_upper_expr_mutator, so we can fix it just by reordering the code.

Fix fix_join_expr_mutator similarly for consistency, even though join
nodes could never appear in such a context.  (In general, it seems
likely to be a bit cheaper to use Vars than Params in such situations
anyway, so this patch might offer a tiny performance improvement.)

The hazard extends back to 9.5 where the MULTIEXPR_SUBLINK stuff
was introduced, so back-patch that far.  However, this may be a live
bug only in 9.6.x and 10.x, as the other branches don't seem to want
to calculate the final tlist below the Result node.  (That plan shape
change between branches might be a mini-bug in itself, but I'm not
really interested in digging into the reasons for that right now.
Still, add a regression test memorializing what we expect there,
so we'll notice if it changes again.)

Per bug report from Eduards Bezverhijs.

Discussion: https://postgr.es/m/b6cd572a-3e44-8785-75e9-c512a5a17a73@tieto.com

Fix test_rls_hooks to assign expression collations properly.

This module overlooked this necessary fixup step on the results of
transformWhereClause().  It accidentally worked anyway, because the
constructed expression involved type "name" which is not collatable,
but it fell over while I was experimenting with changing "name" to
be collatable.

Back-patch, not because there's any live bug here in back branches,
but because somebody might use this code as a model for some real
application and then not understand why it doesn't work.

Doc: improve documentation about ALTER LARGE OBJECT requirements.

Unlike other ALTER ref pages, this one neglected to mention that
ALTER OWNER requires being a member of the new owning role.
Per bug #15546 from Stefan Kadow.

Discussion: https://postgr.es/m/15546-0558c75fd2025e7c@postgresql.org

Raise some timeouts to 180s, in test code.

Slow runs of buildfarm members chipmunk, hornet and mandrill saw the
shorter timeouts expire.  The 180s timeout in poll_query_until has been
trouble-free since 2a0f89cd717ce6d49cdc47850577823682167e87 introduced
it two years ago, so use 180s more widely.  Back-patch to 9.6, where the
first of these timeouts was introduced.

Reviewed by Michael Paquier.

Discussion: https://postgr.es/m/20181209001601.GC2973271@rfd.leadboat.com

Add stack depth checks to key recursive functions in backend/nodes/*.c.

Although copyfuncs.c has a check_stack_depth call in its recursion,
equalfuncs.c, outfuncs.c, and readfuncs.c lacked one.  This seems
unwise.

Likewise fix planstate_tree_walker(), in branches where that exists.

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

Make TupleDescInitBuiltinEntry throw error for unsupported types.

Previously, it would just pass back a partially-uninitialized tupdesc,
which doesn't seem like a safe or useful behavior.

Backpatch to v10 where this code came in.

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

Fix misapplication of pgstat_count_truncate to wrong relation.

The stanza of ExecuteTruncate[Guts] that truncates a target table's toast
relation re-used the loop local variable "rel" to reference the toast rel.
This was safe enough when written, but commit d42358efb added code below
that that supposed "rel" still pointed to the parent table.  Therefore,
the stats counter update was applied to the wrong relcache entry (the
toast rel not the user rel); and if we were unlucky and that relcache
entry had been flushed during reindex_relation, very bad things could
ensue.

(I'm surprised that CLOBBER_CACHE_ALWAYS testing hasn't found this.
I'm even more surprised that the problem wasn't detected during the
development of d42358efb; it must not have been tested in any case
with a toast table, as the incorrect stats counts are very obvious.)

To fix, replace use of "rel" in that code branch with a more local
variable.  Adjust test cases added by d42358efb so that some of them
use tables with toast tables.

Per bug #15540 from Pan Bian.  Back-patch to 9.5 where d42358efb came in.

Discussion: https://postgr.es/m/15540-01078812338195c0@postgresql.org

Clean up sloppy coding in publicationcmds.c's OpenTableList().

Remove dead code (which would be incorrect if it weren't dead),
per report from Pan Bian.  Add a CHECK_FOR_INTERRUPTS in the
inner loop over child relations, because there's little point
in having one in the outer loop if there's not one here too.
Minor stylistic adjustments and comment improvements.

Seems to be aboriginal to this code (cf commit 665d1fad9).
Back-patch to v10 where that came in, not because any of this
is significant, but just to keep the branches looking similar.

Discussion: https://postgr.es/m/15539-06d00ef6b1e2e1bb@postgresql.org

Improve our response to invalid format strings, and detect more cases.

Places that are testing for *printf failure ought to include the format
string in their error reports, since bad-format-string is one of the
more likely causes of such failure.  This both makes it easier to find
and repair the mistake, and provides at least some useful info to the
user who stumbles across such a problem.

Also, tighten snprintf.c to report EINVAL for an invalid flag or
final character in a format %-spec (including the case where the
%-spec is missing a final character altogether).  This seems like
better project policy, and it also allows removing an instruction
or two from the hot code path.

Back-patch the error reporting change in pvsnprintf, since it should be
harmless and may be helpful; but not the snprintf.c change.

Per discussion of bug #15511 from Ertuğrul Kahveci, which reported an
invalid translated format string.  These changes don't fix that error,
but they should improve matters next time we make such a mistake.

Discussion: https://postgr.es/m/15511-1d8b6a0bc874112f@postgresql.org

Improve planner stats documentation

It was pointed out that in the planner stats documentation under
Extended Statistics, one of the sentences was a bit awkward.  Improve
that by rewording it slightly.

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

Fix invalid value of synchronous_commit in description of flush_lag

"remote_flush" has never been a valid user-facing value, but "on" is.

Author: Maksim Milyutin
Discussion: https://postgr.es/m/27b3b80c-3615-2d76-02c5-44566b53136c@gmail.com

Document handling of invalid/ambiguous timestamp input near DST boundaries.

The source code comments documented this, but the user-facing docs, not
so much.  Add a section to Appendix B that discusses it.

In passing, improve a couple other things in Appendix B --- notably,
a long-obsolete claim that time zone abbreviations are looked up in
a fixed table.

Per bug #15527 from Michael Davidson.

Discussion: https://postgr.es/m/15527-f1be0b4dc99ebbe7@postgresql.org

Ensure static libraries have correct mod time even if ranlib messes it up.

In at least Apple's version of ranlib, the output file is updated to have
a mod time equal to the max of the timestamps of its components, and that
data only has seconds precision.  On a filesystem with sub-second file
timestamp precision --- say, APFS --- this can result in the finished
static library appearing older than its input files, which causes useless
rebuilds and possible outright failures in parallel makes.

We've only seen this reported in the field from people using Apple's
ranlib with a non-Apple make, because Apple's make doesn't know about
sub-second timestamps either so it doesn't decide rebuilds are needed.
But Apple's ranlib presumably shares code with at least some BSDen,
so it's not that unlikely that the same problem could arise elsewhere.

To fix, just "touch" the output file after ranlib finishes.

We seem to need this in only one place.  There are other calls of
ranlib in our makefiles, but they are working on intermediate files
whose timestamps are not actually important, or else on an installed
static library for which sub-second timestamp precision is unlikely
to matter either.  (Also, so far as I can tell, Apple's ranlib doesn't
mess up the file timestamp in the latter usage anyhow.)

In passing, change "ranlib" to "$(RANLIB)" in one place that was
bypassing the make macro for no good reason.

Per bug #15525 from Jack Kelly (via Alyssa Ross).
Back-patch to all supported branches.

Discussion: https://postgr.es/m/15525-a30da084f17a1faa@postgresql.org

Fix minor typo in dsa.c.

Author: Takeshi Ideriha
Discussion: https://postgr.es/m/4E72940DA2BF16479384A86D54D0988A6F3BF22D%40G01JPEXMBKW04

Fix handling of synchronous replication for stopping WAL senders

This fixes an oversight from c6c3334 which forgot that if a subset of
WAL senders are stopping and in a sync state, other WAL senders could
still be waiting for a WAL position to be synced while committing a
transaction.  However the subset of stopping senders would not release
waiters, potentially breaking synchronous replication guarantees.  This
commit makes sure that even WAL senders stopping are able to release
waiters and are tracked properly.

On 9.4, this can also trigger an assertion failure when setting for
example max_wal_senders to 1 where a WAL sender is not able to find
itself as in synchronous state when the instance stops.

Reported-by: Paul Guo
Author: Paul Guo, Michael Paquier
Discussion: https://postgr.es/m/CAEET0ZEv8VFqT3C-cQm6byOB4r4VYWcef1J21dOX-gcVhCSpmA@mail.gmail.com
Backpatch-through: 9.4

C comment:  remove extra '*'

Reported-by: Etsuro Fujita
Discussion: https://postgr.es/m/5BFE34DE.1080404@lab.ntt.co.jp

Author: Etsuro Fujita

Backpatch-through: 10

Don't set PAM_RHOST for Unix sockets.

Since commit 2f1d2b7a we have set PAM_RHOST to "[local]" for Unix
sockets.  This caused Linux PAM's libaudit integration to make DNS
requests for that name.  It's not exactly clear what value PAM_RHOST
should have in that case, but it seems clear that we shouldn't set it
to an unresolvable name, so don't do that.

Back-patch to 9.6.  Bug #15520.

Author: Thomas Munro
Reviewed-by: Peter Eisentraut
Reported-by: Albert Schabhuetl
Discussion: https://postgr.es/m/15520-4c266f986998e1c5%40postgresql.org

Do not decode TOAST data for table rewrites

During table rewrites (VACUUM FULL and CLUSTER), the main heap is logged
using XLOG / FPI records, and thus (correctly) ignored in decoding.
But the associated TOAST table is WAL-logged as plain INSERT records,
and so was logically decoded and passed to reorder buffer.

That has severe consequences with TOAST tables of non-trivial size.
Firstly, reorder buffer has to keep all those changes, possibly spilling
them to a file, incurring I/O costs and disk space.

Secondly, ReoderBufferCommit() was stashing all those TOAST chunks into
a hash table, which got discarded only after processing the row from the
main heap.  But as the main heap is not decoded for rewrites, this never
happened, so all the TOAST data accumulated in memory, resulting either
in excessive memory consumption or OOM.

The fix is simple, as commit e9edc1ba already introduced infrastructure
(namely HEAP_INSERT_NO_LOGICAL flag) to skip logical decoding of TOAST
tables, but it only applied it to system tables.  So simply use it for
all TOAST data in raw_heap_insert().

That would however solve only the memory consumption issue - the TOAST
changes would still be decoded and added to the reorder buffer, and
spilled to disk (although without TOAST tuple data, so much smaller).
But we can solve that by tweaking DecodeInsert() to just ignore such
INSERT records altogether, using XLH_INSERT_CONTAINS_NEW_TUPLE flag,
instead of skipping them later in ReorderBufferCommit().

Review: Masahiko Sawada
Discussion: https://www.postgresql.org/message-id/flat/1a17c643-e9af-3dba-486b-fbe31bc1823a%402ndquadrant.com
Backpatch: 9.4-, where logical decoding was introduced

Fix ac218aa4f6 to work on versions before 9.5.

Unfortunately ac218aa4f6 missed the fact that a reference to
'pg_catalog.regnamespace'::regclass wouldn't work before that type is
known. Fix that, by replacing the regtype usage with a join to
pg_type.

Reported-By: Tom Lane
Author: Andres Freund
Discussion: https://postgr.es/m/8863.1543297423@sss.pgh.pa.us
Backpatch: 9.5-, like ac218aa4f6

Update pg_upgrade test for reg* to include regrole and regnamespace.

When the regrole (0c90f6769) and regnamespace (cb9fa802b) types were
added in 9.5, pg_upgrade's check for reg* types wasn't updated. While
regrole currently is safe, regnamespace is not.

It seems unlikely that anybody uses regnamespace inside catalog tables
across a pg_upgrade, but the tests should be correct nevertheless.

While at it, reorder the types checked in the query to be
alphabetical. Otherwise it's annoying to compare existing and tested
for types.

Author: Andres Freund
Discussion: https://postgr.es/m/037e152a-cb25-3bcb-4f35-bdc9988f8204@2ndQuadrant.com
Backpatch: 9.5-, as regrole/regnamespace

doc:  fix wording for plpgsql, add "and"

Reported-by: Anthony Greene
Discussion: https://postgr.es/m/CAPRNmnsSZ4QL75FUjcS8ND_oV+WjgyPbZ4ch2RUwmW6PWzF38w@mail.gmail.com

Backpatch-through: 9.4

Fix translation of special characters in psql's LaTeX output modes.

latex_escaped_print() mistranslated \ and failed to provide any translation
for # ^ and ~, all of which would typically lead to LaTeX document syntax
errors.  In addition it didn't translate < > and |, which would typically
render as unexpected characters.

To some extent this represents shortcomings in ancient versions of LaTeX,
which if memory serves had no easy way to render these control characters
as ASCII text.  But that's been fixed for, um, decades.  In any case there
is no value in emitting guaranteed-to-fail output for these characters.

Noted while fooling with test cases added by commit 9a98984f4.  Back-patch
the code change to all supported versions.

Fix sample output for hash_metapage_info query

One output column was duplicated.  Couldn't resist fixing the version
number while at it.

Reported-by: Gianni Ciolli

Revert "Fix typo in documentation of toast storage"

This reverts commit 058ef3a, per complains from Magnus Hagander and Vik
Fearing.

Fix typo in documentation of toast storage

Author: Nawaz Ahmed
Discussion: https://postgr.es/m/154319327168.1315.1846953598601966513@wrigleys.postgresql.org

Fix hstore hash function for empty hstores upgraded from 8.4.

Hstore data generated on pg 8.4 and pg_upgraded to current versions
remains in its original on-disk format unless modified. The same goes
for values generated by the addon hstore-new module on pre-9.0
versions. (The hstoreUpgrade function converts old values on the fly
when read in, but the on-disk value is not modified by this.)

Since old-format empty hstores (and hstore-new hstores) have
representations compatible with the new format, hstoreUpgrade thought
it could get away without modifying such values; but this breaks
hstore_hash (and the new hstore_hash_extended) which assumes
bit-perfect matching between semantically identical hstore values.
Only one bit actually differs (the "new version" flag in the count
field) but that of course is enough to break the hash.

Fix by making hstoreUpgrade unconditionally convert all old values to
new format.

Backpatch all the way, even though this changes a hash value in some
cases, because in those cases the hash value is already failing - for
example, a hash join between old- and new-format empty hstores will be
failing to match, or a hash index on an hstore column containing an
old-format empty value will be failing to find the value since it will
be searching for a hash derived from a new-format datum. (There are no
known field reports of this happening, probably because hashing of
hstores has only been useful in limited circumstances and there
probably isn't much upgraded data being used this way.)

Per concerns arising from discussion of commit eb6f29141be. Original
bug is my fault.

Discussion: https://postgr.es/m/60b1fd3b-7332-40f0-7e7f-f2f04f777747%402ndquadrant.com

Update additional float4/8 expected-output files.

I forgot that the back branches have more variant files than HEAD :-(.
Per buildfarm.

Discussion: https://postgr.es/m/15519-4fc785b483201ff1@postgresql.org

Fix float-to-integer coercions to handle edge cases correctly.

ftoi4 and its sibling coercion functions did their overflow checks in
a way that looked superficially plausible, but actually depended on an
assumption that the MIN and MAX comparison constants can be represented
exactly in the float4 or float8 domain.  That fails in ftoi4, ftoi8,
and dtoi8, resulting in a possibility that values near the MAX limit will
be wrongly converted (to negative values) when they need to be rejected.

Also, because we compared before rounding off the fractional part,
the other three functions threw errors for values that really ought
to get rounded to the min or max integer value.

Fix by doing rint() first (requiring an assumption that it handles
NaN and Inf correctly; but dtoi8 and ftoi8 were assuming that already),
and by comparing to values that should coerce to float exactly, namely
INTxx_MIN and -INTxx_MIN.  Also remove some random cosmetic discrepancies
between these six functions.

This back-patches commits cbdb8b4c0 and 452b637d4.  In the 9.4 branch,
also back-patch the portion of 62e2a8dc2 that added PG_INTnn_MIN and
related constants to c.h, so that these functions can rely on them.

Per bug #15519 from Victor Petrovykh.

Patch by me; thanks to Andrew Gierth for analysis and discussion.

Discussion: https://postgr.es/m/15519-4fc785b483201ff1@postgresql.org

Avoid crashes in contrib/intarray gist__int_ops (bug #15518)

1. Integer overflow in internal_size could result in memory corruption
in decompression since a zero-length array would be allocated and then
written to. This leads to crashes or corruption when traversing an
index which has been populated with sufficiently sparse values. Fix by
using int64 for computations and checking for overflow.

2. Integer overflow in g_int_compress could cause pessimal merge
choices, resulting in unnecessarily large ranges (which would in turn
trigger issue 1 above). Fix by using int64 again.

3. Even without overflow, array sizes could become large enough to
cause unexplained memory allocation errors. Fix by capping the sizes
to a safe limit and report actual errors pointing at gist__intbig_ops
as needed.

4. Large inputs to the compression function always consist of large
runs of consecutive integers, and the compression loop was processing
these one at a time in an O(N^2) manner with a lot of overhead. The
expected runtime of this function could easily exceed 6 months for a
single call as a result. Fix by performing a linear-time first pass,
which reduces the worst case to something on the order of seconds.

Backpatch all the way, since this has been wrong forever.

Per bug #15518 from report from irc user "dymk", analysis and patch by
me.

Discussion: https://postgr.es/m/15518-799e426c3b4f8358@postgresql.org

doc: Fix typo

doc:  adjust time zone names text, v2

Removed one too many words.  Fix for
7906de847f229f391b9e6b5892b4b4a89f29edb4.

Reported-by: Thomas Munro
Backpatch-through: 9.4

doc:  adjust time zone names text

Reported-by: Kevin <kcolagio@gmail.com>
Discussion: https://postgr.es/m/154082462281.30897.14043119084654378035@wrigleys.postgresql.org

Backpatch-through: 9.4

doc: Clarify CREATE TYPE ENUM documentation

The documentation claimed that an enum type requires "one or more"
labels, but since 1fd9883ff49, zero labels are also allowed.

Reported-by: Lukas Eder <lukas.eder@gmail.com>
Bug: #15356

Add needed #include.

Per POSIX, WIFSIGNALED and related macros are provided by <sys/wait.h>.
Apparently on Linux they're also pulled in by some other inclusions,
but BSD-ish systems are pickier.  Fixes portability issue in ffa4cbd62.

Per buildfarm.

Handle EPIPE more sanely when we close a pipe reading from a program.

Previously, any program launched by COPY TO/FROM PROGRAM inherited the
server's setting of SIGPIPE handling, i.e. SIG_IGN.  Hence, if we were
doing COPY FROM PROGRAM and closed the pipe early, the child process
would see EPIPE on its output file and typically would treat that as
a fatal error, in turn causing the COPY to report error.  Similarly,
one could get a failure report from a query that didn't read all of
the output from a contrib/file_fdw foreign table that uses file_fdw's
PROGRAM option.

To fix, ensure that child programs inherit SIG_DFL not SIG_IGN processing
of SIGPIPE.  This seems like an all-around better situation since if
the called program wants some non-default treatment of SIGPIPE, it would
expect to have to set that up for itself.  Then in COPY, if it's COPY
FROM PROGRAM and we stop reading short of detecting EOF, treat a SIGPIPE
exit from the called program as a non-error condition.  This still allows
us to report an error for any case where the called program gets SIGPIPE
on some other file descriptor.

As coded, we won't report a SIGPIPE if we stop reading as a result of
seeing an in-band EOF marker (e.g. COPY BINARY EOF marker).  It's
somewhat debatable whether we should complain if the called program
continues to transmit data after an EOF marker.  However, it seems like
we should avoid throwing error in any questionable cases, especially in a
back-patched fix, and anyway it would take additional code to make such
an error get reported consistently.

Back-patch to v10.  We could go further back, since COPY FROM PROGRAM
has been around awhile, but AFAICS the only way to reach this situation
using core or contrib is via file_fdw, which has only supported PROGRAM
sources since v10.  The COPY statement per se has no feature whereby
it'd stop reading without having hit EOF or an error already.  Therefore,
I don't see any upside to back-patching further that'd outweigh the
risk of complaints about behavioral change.

Per bug #15449 from Eric Cyr.

Patch by me, review by Etsuro Fujita and Kyotaro Horiguchi

Discussion: https://postgr.es/m/15449-1cf737dd5929450e@postgresql.org

Fix configure's AC_CHECK_DECLS tests to work correctly with clang.

The test case that Autoconf uses to discover whether a function has
been declared doesn't work reliably with clang, because clang reports
a warning not an error if the name is a known built-in function.
On some platforms, this results in a lot of compile-time warnings about
strlcpy and related functions not having been declared.

There is a fix for this (by Noah Misch) in the upstream Autoconf sources,
but since they've not made a release in years and show no indication of
doing so anytime soon, let's just absorb their fix directly.  We can
revert this when and if we update to a newer Autoconf release.

Back-patch to all supported branches.

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

Disallow COPY FREEZE on partitioned tables

This didn't actually work: COPY would fail to flush the right files, and
instead would try to flush a non-existing file, causing the whole
transaction to fail.

Cope by raising an error as soon as the command is sent instead, to
avoid a nasty later surprise.  Of course, it would be much better to
make it work, but we don't have a patch for that yet, and we don't know
if we'll want to backpatch one when we do.

Reported-by: Tomas Vondra
Author: David Rowley
Reviewed-by: Amit Langote, Steve Singer, Tomas Vondra

PANIC on fsync() failure.

On some operating systems, it doesn't make sense to retry fsync(),
because dirty data cached by the kernel may have been dropped on
write-back failure.  In that case the only remaining copy of the
data is in the WAL.  A subsequent fsync() could appear to succeed,
but not have flushed the data.  That means that a future checkpoint
could apparently complete successfully but have lost data.

Therefore, violently prevent any future checkpoint attempts by
panicking on the first fsync() failure.  Note that we already
did the same for WAL data; this change extends that behavior to
non-temporary data files.

Provide a GUC data_sync_retry to control this new behavior, for
users of operating systems that don't eject dirty data, and possibly
forensic/testing uses.  If it is set to on and the write-back error
was transient, a later checkpoint might genuinely succeed (on a
system that does not throw away buffers on failure); if the error is
permanent, later checkpoints will continue to fail.  The GUC defaults
to off, meaning that we panic.

Back-patch to all supported releases.

There is still a narrow window for error-loss on some operating
systems: if the file is closed and later reopened and a write-back
error occurs in the intervening time, but the inode has the bad
luck to be evicted due to memory pressure before we reopen, we could
miss the error.  A later patch will address that with a scheme
for keeping files with dirty data open at all times, but we judge
that to be too complicated to back-patch.

Author: Craig Ringer, with some adjustments by Thomas Munro
Reported-by: Craig Ringer
Reviewed-by: Robert Haas, Thomas Munro, Andres Freund
Discussion: https://postgr.es/m/20180427222842.in2e4mibx45zdth5%40alap3.anarazel.de

Don't forget about failed fsync() requests.

If fsync() fails, md.c must keep the request in its bitmap, so that
future attempts will try again.

Back-patch to all supported releases.

Author: Thomas Munro
Reviewed-by: Amit Kapila
Reported-by: Andrew Gierth
Discussion: https://postgr.es/m/87y3i1ia4w.fsf%40news-spur.riddles.org.uk

Add valgrind suppressions for wcsrtombs optimizations

wcsrtombs (called through wchar2char from common functions like lower,
upper, etc.) uses various optimizations that may look like access to
uninitialized data, triggering valgrind reports.

For example AVX2 instructions load data in 256-bit chunks, and  gconv
does something similar with 32-bit chunks.  This is faster than accessing
the bytes one by one, and the uninitialized part of the buffer is not
actually used. So suppress the bogus reports.

The exact stack depends on possible optimizations - it might be AVX, SSE
(as in the report by Aleksander Alekseev) or something else. Hence the
last frame is wildcarded, to deal with this.

Backpatch all the way back to 9.4.

Author: Tomas Vondra
Discussion: https://www.postgresql.org/message-id/flat/90ac0452-e907-e7a4-b3c8-15bd33780e62%402ndquadrant.com
Discussion: https://www.postgresql.org/message-id/20180220150838.GD18315@e733.localdomain

Doc: remove claim that all \pset format options are unique in 1 letter.

This hasn't been correct since 9.3 added "latex-longtable".

I left the phraseology "Unique abbreviations are allowed" alone.
It's correct as far as it goes, and we are studiously refraining
from specifying exactly what happens if you give a non-unique
abbreviation.  (The answer in the back branches is "you get a
backwards-compatible choice", and the answer in HEAD will shortly
be "you get an error", but there seems no need to mention such
details here.)

Daniel Vérité

Discussion: https://postgr.es/m/cb7e1caf-3ea6-450d-af28-f524903a030c@manitou-mail.org

Second try at fixing numeric data passed through an ECPG SQLDA.

In commit ecfd55795, I removed sqlda.c's checks for ndigits != 0 on the
grounds that we should duplicate the state of the numeric value's digit
buffer even when all the digits are zeroes.  However, that still isn't
quite right, because another possible state of the digit buffer is
buf == digits == NULL (this occurs for a NaN).  As the code now stands,
it'll invoke memcpy with a NULL source address and zero bytecount,
which we know a few platforms crash on.  Hence, reinstate the no-copy
short-circuit, but make it test specifically for buf != NULL rather than
some other condition.  In hindsight, the ndigits test (added by commit
f2ae9f9c3) was almost certainly meant to fix the NaN case not the
all-zeroes case as the associated thread alleged.

As before, back-patch to all supported versions.

Discussion: https://postgr.es/m/1803D792815FC24D871C00D17AE95905C71161@g01jpexmbkw24

Initialize TransactionState and user ID consistently at transaction start

If a failure happens when a transaction is starting between the moment
the transaction status is changed from TRANS_DEFAULT to TRANS_START and
the moment the current user ID and security context flags are fetched
via GetUserIdAndSecContext(), or before initializing its basic fields,
then those may get reset to incorrect values when the transaction
aborts, leaving the session in an inconsistent state.

One problem reported is that failing a starting transaction at the first
query of a session could cause several kinds of system crashes on the
follow-up queries.

In order to solve that, move the initialization of the transaction state
fields and the call of GetUserIdAndSecContext() in charge of fetching
the current user ID close to the point where the transaction status is
switched to TRANS_START, where there cannot be any error triggered
in-between, per an idea of Tom Lane.  This properly ensures that the
current user ID, the security context flags and that the basic fields of
TransactionState remain consistent even if the transaction fails while
starting.

Reported-by: Richard Guo
Diagnosed-By: Richard Guo
Author: Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAN_9JTxECSb=pEPcb0a8d+6J+bDcOZ4=DgRo_B7Y5gRHJUM=Rw@mail.gmail.com
Backpatch-through: 9.4

Fix incorrect results for numeric data passed through an ECPG SQLDA.

Numeric values with leading zeroes were incorrectly copied into a
SQLDA (SQL Descriptor Area), leading to wrong results in ECPG programs.

Report and patch by Daisuke Higuchi.  Back-patch to all supported
versions.

Discussion: https://postgr.es/m/1803D792815FC24D871C00D17AE95905C71161@g01jpexmbkw24

Fix the initialization of atomic variable introduced by the
group clearing mechanism.

Commit 0e141c0fbb introduced initialization of atomic variable in
InitProcess which means that it's not safe to look at it for backends that
aren't currently in use.  Fix that by initializing the same during postmaster
startup.

Reported-by: Andres Freund
Author: Amit Kapila
Backpatch-through: 9.6
Discussion:https://postgr.es/m/20181027104138.qmbbelopvy7cw2qv@alap3.anarazel.de

Fix possible buffer overrun in hba.c.

Coverty reports a possible buffer overrun in the code that populates the
pg_hba_file_rules view.  It may not be a live bug due to restrictions
on options that can be used together, but let's increase MAX_HBA_OPTIONS
and correct a nearby misleading comment.

Back-patch to 10 where this code arrived.

Reported-by: Julian Hsiao
Discussion: https://postgr.es/m/CADnGQpzbkWdKS2YHNifwAvX5VEsJ5gW49U4o-7UL5pzyTv4vTg%40mail.gmail.com

Limit the number of index clauses considered in choose_bitmap_and().

classify_index_clause_usage() is O(N^2) in the number of distinct index
qual clauses it considers, because of its use of a simple search list to
store them.  For nearly all queries, that's fine because only a few clauses
will be considered.  But Alexander Kuzmenkov reported a machine-generated
query with 80000 (!) index qual clauses, which caused this code to take
forever.  Somewhat remarkably, this is the only O(N^2) behavior we now
have for such a query, so let's fix it.

We can get rid of the O(N^2) runtime for cases like this without much
damage to the functionality of choose_bitmap_and() by separating out
paths with "too many" qual or pred clauses, and deeming them to always
be nonredundant with other paths.  Then their clauses needn't go into
the search list, so it doesn't get too long, but we don't lose the
ability to consider bitmap AND plans altogether.  I set the threshold
for "too many" to be 100 clauses per path, which should be plenty to
ensure no change in planning behavior for normal queries.

There are other things we could do to make this go faster, but it's not
clear that it's worth any additional effort.  80000 qual clauses require
a whole lot of work in many other places, too.

The code's been like this for a long time, so back-patch to all supported
branches.  The troublesome query only works back to 9.5 (in 9.4 it fails
with stack overflow in the parser); so I'm not sure that fixing this in
9.4 has any real-world benefit, but perhaps it does.

Discussion: https://postgr.es/m/90c5bdfa-d633-dabe-9889-3cf3e1acd443@postgrespro.ru

Fix incorrect author name in release notes

Author: Alexander Lakhin
Discussion: https://postgr.es/m/2f55f6d2-3fb0-d4f6-5c47-18da3a1117e0@gmail.com

Fix missing role dependencies for some schema and type ACLs.

This patch fixes several related cases in which pg_shdepend entries were
never made, or were lost, for references to roles appearing in the ACLs of
schemas and/or types.  While that did no immediate harm, if a referenced
role were later dropped, the drop would be allowed and would leave a
dangling reference in the object's ACL.  That still wasn't a big problem
for normal database usage, but it would cause obscure failures in
subsequent dump/reload or pg_upgrade attempts, taking the form of
attempts to grant privileges to all-numeric role names.  (I think I've
seen field reports matching that symptom, but can't find any right now.)

Several cases are fixed here:

1. ALTER DOMAIN SET/DROP DEFAULT would lose the dependencies for any
existing ACL entries for the domain.  This case is ancient, dating
back as far as we've had pg_shdepend tracking at all.

2. If a default type privilege applies, CREATE TYPE recorded the
ACL properly but forgot to install dependency entries for it.
This dates to the addition of default privileges for types in 9.2.

3. If a default schema privilege applies, CREATE SCHEMA recorded the
ACL properly but forgot to install dependency entries for it.
This dates to the addition of default privileges for schemas in v10
(commit ab89e465c).

Another somewhat-related problem is that when creating a relation
rowtype or implicit array type, TypeCreate would apply any available
default type privileges to that type, which we don't really want
since such an object isn't supposed to have privileges of its own.
(You can't, for example, drop such privileges once they've been added
to an array type.)

ab89e465c is also to blame for a race condition in the regression tests:
privileges.sql transiently installed globally-applicable default
privileges on schemas, which sometimes got absorbed into the ACLs of
schemas created by concurrent test scripts.  This should have resulted
in failures when privileges.sql tried to drop the role holding such
privileges; but thanks to the bug fixed here, it instead led to dangling
ACLs in the final state of the regression database.  We'd managed not to
notice that, but it became obvious in the wake of commit da906766c, which
allowed the race condition to occur in pg_upgrade tests.

To fix, add a function recordDependencyOnNewAcl to encapsulate what
callers of get_user_default_acl need to do; while the original call
sites got that right via ad-hoc code, none of the later-added ones
have.  Also change GenerateTypeDependencies to generate these
dependencies, which requires adding the typacl to its parameter list.
(That might be annoying if there are any extensions calling that
function directly; but if there are, they're most likely buggy in the
same way as the core callers were, so they need work anyway.)  While
I was at it, I changed GenerateTypeDependencies to accept most of its
parameters in the form of a Form_pg_type pointer, making its parameter
list a bit less unwieldy and mistake-prone.

The test race condition is fixed just by wrapping the addition and
removal of default privileges into a single transaction, so that that
state is never visible externally.  We might eventually prefer to
separate out tests of default privileges into a script that runs by
itself, but that would be a bigger change and would make the tests
run slower overall.

Back-patch relevant parts to all supported branches.

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

Fix dependency handling of partitions and inheritance for ON COMMIT

This commit fixes a set of issues with ON COMMIT actions when used on
partitioned tables and tables with inheritance children:
- Applying ON COMMIT DROP on a partitioned table with partitions or on a
table with inheritance children caused a failure at commit time, with
complains about the children being already dropped as all relations are
dropped one at the same time.
- Applying ON COMMIT DELETE on a partition relying on a partitioned
table which uses ON COMMIT DROP would cause the partition truncation to
fail as the parent is removed first.

The solution to the first problem is to handle the removal of all the
dependencies in one go instead of dropping relations one-by-one, based
on a suggestion from Álvaro Herrera.  So instead all the relation OIDs
to remove are gathered and then processed in one round of multiple
deletions.

The solution to the second problem is to reorder the actions, with
truncation happening first and relation drop done after.  Even if it
means that a partition could be first truncated, then immediately
dropped if its partitioned table is dropped, this has the merit to keep
the code simple as there is no need to do existence checks on the
relations to drop.

Contrary to a manual TRUNCATE on a partitioned table, ON COMMIT DELETE
does not cascade to its partitions.  The ON COMMIT action defined on
each partition gets the priority.

Author: Michael Paquier
Reviewed-by: Amit Langote, Álvaro Herrera, Robert Haas
Discussion: https://postgr.es/m/68f17907-ec98-1192-f99f-8011400517f5@lab.ntt.co.jp
Backpatch-through: 10

Disallow setting client_min_messages higher than ERROR.

Previously it was possible to set client_min_messages to FATAL or PANIC,
which had the effect of suppressing transmission of regular ERROR messages
to the client.  Perhaps that seemed like a useful option in the past, but
the trouble with it is that it breaks guarantees that are explicitly made
in our FE/BE protocol spec about how a query cycle can end.  While libpq
and psql manage to cope with the omission, that's mostly because they
are not very bright; client libraries that have more semantic knowledge
are likely to get confused.  Notably, pgODBC doesn't behave very sanely.
Let's fix this by getting rid of the ability to set client_min_messages
above ERROR.

In HEAD, just remove the FATAL and PANIC options from the set of allowed
enum values for client_min_messages.  (This change also affects
trace_recovery_messages, but that's OK since these aren't useful values
for that variable either.)

In the back branches, there was concern that rejecting these values might
break applications that are explicitly setting things that way.  I'm
pretty skeptical of that argument, but accommodate it by accepting these
values and then internally setting the variable to ERROR anyway.

In all branches, this allows a couple of tiny simplifications in the
logic in elog.c, so do that.

Also respond to the point that was made that client_min_messages has
exactly nothing to do with the server's logging behavior, and therefore
does not belong in the "When To Log" subsection of the documentation.
The "Statement Behavior" subsection is a better match, so move it there.

Jonah Harris and Tom Lane

Discussion: https://postgr.es/m/7809.1541521180@sss.pgh.pa.us
Discussion: https://postgr.es/m/15479-ef0f4cc2fd995ca2@postgresql.org

Revise attribute handling code on partition creation

The original code to propagate NOT NULL and default expressions
specified when creating a partition was mostly copy-pasted from
typed-tables creation, but not being a great match it contained some
duplicity, inefficiency and bugs.

This commit fixes the bug that NOT NULL constraints declared in the
parent table would not be honored in the partition.  One reported issue
that is not fixed is that a DEFAULT declared in the child is not used
when inserting through the parent.  That would amount to a behavioral
change that's better not back-patched.

This rewrite makes the code simpler:

1. instead of checking for duplicate column names in its own block,
reuse the original one that already did that;

2. instead of concatenating the list of columns from parent and the one
declared in the partition and scanning the result to (incorrectly)
propagate defaults and not-null constraints, just scan the latter
searching the former for a match, and merging sensibly.  This works
because we know the list in the parent is already correct and there can
only be one parent.

This rewrite makes ColumnDef->is_from_parent unused, so it's removed
on branch master; on released branches, it's kept as an unused field in
order not to cause ABI incompatibilities.

This commit also adds a test case for creating partitions with
collations mismatching that on the parent table, something that is
closely related to the code being patched.  No code change is introduced
though, since that'd be a behavior change that could break some (broken)
working applications.

Amit Langote wrote a less invasive fix for the original
NOT NULL/defaults bug, but while I kept the tests he added, I ended up
not using his original code.  Ashutosh Bapat reviewed Amit's fix.  Amit
reviewed mine.

Author: Álvaro Herrera, Amit Langote
Reviewed-by: Ashutosh Bapat, Amit Langote
Reported-by: Jürgen Strobel (bug #15212)
Discussion: https://postgr.es/m/152746742177.1291.9847032632907407358@wrigleys.postgresql.org

GUC: adjust effective_cache_size SQL descriptions

Follow on patch for commit 3e0f1a4741f564c1a2fa6e944729d6967355d8c7.

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/369ec766-b947-51bd-4dad-6fb9e026439f@2ndquadrant.com

Backpatch-through: 9.4

Rename rbtree.c functions to use "rbt" prefix not "rb" prefix.

The "rb" prefix is used by Ruby, so that our existing code results
in name collisions that break plruby.  We discussed ways to prevent
that by adjusting dynamic linker options, but it seems that at best
we'd move the pain to other cases.  Renaming to avoid the collision
is the only portable fix anyway.  Fortunately, our rbtree code is
not (yet?) widely used --- in core, there's only a single usage
in GIN --- so it seems likely that we can get away with a rename.

I chose to do this basically as s/rb/rbt/g, except for places where
there already was a "t" after "rb".  The patch could have been made
smaller by only touching linker-visible symbols, but it would have
resulted in oddly inconsistent-looking code.  Better to make it look
like "rbt" was the plan all along.

Back-patch to v10.  The rbtree.c code exists back to 9.5, but
rb_iterate() which is the actual immediate source of pain was added
in v10, so it seems like changing the names before that would have
more risk than benefit.

Per report from Pavel Raiskup.

Discussion: https://postgr.es/m/4738198.8KVIIDhgEB@nb.usersys.redhat.com

Stamp 10.6.

Last-minute updates for release notes.

I removed the item about the pg_stat_statements change from
release-11.sgml, as part of a sweep to delete items already committed
in 11.0; but actually we'd best keep it to ensure that people who've
pg_upgraded their databases will take the requisite action.  Also make
said action more visible by making it into its own para.  Noted by
Jonathan Katz.

Fix copy-paste error in errhint() introduced in 691d79a07933.

Reported-By: Petr Jelinek
Discussion: https://postgr.es/m/c95a620b-34f0-7930-aeb5-f7ab804f26cb@2ndquadrant.com
Backpatch: 9.4-, like the previous commit

Last-minute updates for release notes.

Security: CVE-2018-16850

Translation updates

Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 4aac9391521d21fdecc378db4750a59795350b33

Block creation of partitions with open references to its parent

When a partition is created as part of a trigger processing, it is
possible that the partition which just gets created changes the
properties of the table the executor of the ongoing command relies on,
causing a subsequent crash.  This has been found possible when for
example using a BEFORE INSERT which creates a new partition for a
partitioned table being inserted to.

Any attempt to do so is blocked when working on a partition, with
regression tests added for both CREATE TABLE PARTITION OF and ALTER
TABLE ATTACH PARTITION.

Reported-by: Dmitry Shalashov
Author: Amit Langote
Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/15437-3fe01ee66bd1bae1@postgresql.org
Backpatch-through: 10