Amit Langote

pg_restore will currently accept invalid values for the number of
parallel jobs to run (eg: -1), unlike pg_dump which does check that the
value provided is reasonable.

Worse, '-1' is actually a valid, independent, parameter (as an alias for
--single-transaction), leading to potentially completely unexpected
results from a command line such as:

  -> pg_restore -j -1

Where a user would get neither parallel jobs nor a single-transaction.

Add in validity checking of the parallel jobs option, as we already have
in pg_dump, before we try to open up the archive.  Also move the check
that we haven't been asked to run more parallel jobs than possible on
Windows to the same place, so we do all the option validity checking
before opening the archive.

Back-patch all the way, though for 9.2 we're adding the Windows-specific
check against MAXIMUM_WAIT_OBJECTS as that check wasn't back-patched
originally.

Discussion: https://www.postgresql.org/message-id/20170110044815.GC18360%40tamriel.snowman.net

Masahiko Sawada

The previous --path documentation and --help output were wrong in both
its meaning and the defaults.

Reviewed-by: Michael Paquier

Backpatch-through: 9.6

When using pg_dump --strict-names and a schema pattern which doesn't
match any schemas (eg: --schema='nonexistant*'), we were incorrectly
throwing an error claiming no tables were found when, really, there
were no schemas found:

  -> pg_dump --strict-names --schema='nonexistant*'
  pg_dump: no matching tables were found for pattern "nonexistant*"

Fix that by changing the error message to say 'schemas' instead, since
that is what we are actually complaining about.

Noticed while testing pg_dump error cases.

Back-patch to 9.6 where --strict-names and this error message were
introduced.

A few thinkos I introduced in fa2fa995.  Also, amend a similarly
broken comment.

Report by Daniel Vérité.
Authors: Daniel Vérité, Álvaro Herrera
Discussion: https://postgr.es/m/1706e85e-60d2-494e-8a64-9af1e1b2186e@manitou-mail.org

Instead of relying on the page contents to know whether we have
advanced from the primary bucket page to an overflow page, track
that explicitly.

Amit Kapila, per a complaint by me.

Including the program name twice is not helpful:

-> pg_dump -j -1
pg_dump: pg_dump: invalid number of parallel jobs

Correct by removing the progname from the exit_horribly() call used when
validating the number of parallel jobs.

Noticed while testing various pg_dump error cases.

Back-patch to 9.3 where parallel pg_dump was added.

We can't throw elog(ERROR) out of a Tcl command procedure; we have
to catch the error and return TCL_ERROR to the Tcl interpreter.
pltcl_returnnext failed to meet this requirement, so that errors
detected by pltcl_build_tuple_result or other functions called here
led to longjmp'ing out of the Tcl interpreter and thereby leaving it
in a bad state.  Use the existing subtransaction support to prevent
that.  Oversight in commit 26abb50c, found more or less accidentally
by the buildfarm thanks to the tests added in 961bed02.

Report: https://postgr.es/m/30647.1483989734@sss.pgh.pa.us

If inherited tables don't have exactly the same schema, the USING clause
in an ALTER TABLE / SET DATA TYPE misbehaves when applied to the
children tables since commit 9550e834.  Starting with that commit,
the attribute numbers in the USING expression are fixed during parse
analysis.  This can lead to bogus errors being reported during
execution, such as:
   ERROR:  attribute 2 has wrong type
   DETAIL:  Table has type smallint, but query expects integer.

Since it wouldn't do to revert to the original coding, we now apply a
transformation to map the attribute numbers to the correct ones for each
child.

Reported by Justin Pryzby
Analysis by Tom Lane; patch by me.
Discussion: https://postgr.es/m/20170102225618.GA10071@telsasoft.com

... and therefore we ought not to tell XLogRegisterBuffer the opposite,
when writing XLog for a brin update that moves the index tuple to a
different page.  Otherwise, xlog insertion would try to "compress the
hole" when producing a full-page image for it; but since we don't update
pd_lower/upper, the hole covers the whole page.  On WAL replay, the
revmap page becomes empty and so the entire portion of the index is
useless and needs to be recomputed.

This is low-probability: a BRIN update only moves an index tuple to a
different page when the summary tuple is larger than the existing one,
which doesn't happen with fixed-width datatypes.  Also, the revmap
page must be first after a checkpoint.

Report and patch: Kuntal Ghosh
Bug is alleged to have detected by a WAL-consistency-checking tool.
Discussion: https://postgr.es/m/CAGz5QCJ=00UQjScSEFbV=0qO5ShTZB9WWz_Fm7+Wd83zPs9Geg@mail.gmail.com

I posted a test case demonstrating the problem, but I'm refraining from
adding it to the test suite; if the WAL consistency tool makes it in,
that will be a better way to catch this from regressing.  (We should
definitely have someting that causes not-same-page updates, though.)

This raises the test coverage (by line count) in pltcl.c from about 70%
to 86%.

Karl Lehenbauer and Jim Nasby

Discussion: https://postgr.es/m/92a1670d-21b6-8f03-9c13-e4fb2207ab7b@BlueTreble.com

This makes the code easier to read as it becomes more explicit what the
different allowed combinations really are.

Suggested by Michael Paquier

I noticed that p_value_substitute, which is a single-purpose kluge I added
in 2002 (commit b0422b21), could be replaced by having domainAddConstraint
install a parser hook that looks for the name "value".  The parser hook
code only dates back to 2009, so it's not surprising that we had to kluge
this in 2002, but we can do it more cleanly now.

I got annoyed about how some fields of ParseState were documented in the
struct's block comment and some weren't; not all of the latter are trivial.
Fix that.  Also reorder a couple of fields that seem to have been placed
rather randomly, or maybe with an idea of avoiding padding space; but there
are never so many ParseStates in existence at one time that we ought to
value pad space over readability.

For reasons unknown, pg_dumpall and pg_restore managed to escape the
basic set of TAP tests that were added for pg_dump in 6bd356c3, so
let's get them added now.  A few minor adjustments are also made to the
dump/restore tests to improve code coverage for pg_restore/pg_dumpall.

Make pltcl_trigger_handler() construct modified tuples using
pltcl_build_tuple_result(), rather than its own copy of essentially
the same logic.  This results in slightly different message wording for
the error cases, and in one case a different SQLSTATE, but it seems
unlikely that any existing applications are depending on any of those
details.

While at it, fix a typo in commit 26abb50c: pltcl_build_tuple_result was
applying encoding conversion in the wrong direction.  That would be a
back-patchable bug fix, except the code hasn't shipped yet.

Jim Nasby, reviewed by me

Discussion: https://postgr.es/m/d2c6425a-d9e0-f034-f774-4a872c234d89@BlueTreble.com

findTableByOid() is allowed to return NULL and we should therefore be
checking for that case.  getOwnedSeqs() and dumpSequence() shouldn't
ever actually see this happen, but given odd circumstances it might and
commit f9e439b1 probably shouldn't have removed that check.

Pointed out by Coverity.  Initial patch from Michael Paquier.

Back-patch to 9.6, where that commit had removed the check.

This fixes problems where a plan must change but fails to do so,
as seen in a bug report from Rajkumar Raghuwanshi.

For ALTER FOREIGN TABLE OPTIONS, do this through the standard method of
forcing a relcache flush on the table.  For ALTER FOREIGN DATA WRAPPER
and ALTER SERVER, just flush the whole plan cache on any change in
pg_foreign_data_wrapper or pg_foreign_server.  That matches the way
we handle some other low-probability cases such as opclass changes, and
it's unclear that the case arises often enough to be worth working harder.
Besides, that gives a patch that is simple enough to back-patch with
confidence.

Back-patch to 9.3.  In principle we could apply the code change to 9.2 as
well, but (a) we lack postgres_fdw to test it with, (b) it's doubtful that
anyone is doing anything exciting enough with FDWs that far back to need
this desperately, and (c) the patch doesn't apply cleanly.

Patch originally by Amit Langote, reviewed by Etsuro Fujita and Ashutosh
Bapat, who each contributed substantial changes as well.

Discussion: https://postgr.es/m/CAKcux6m5cA6rRPTKkqVdJ-R=KKDfe35Q_ZuUqxDSV_4hwga=og@mail.gmail.com

This commit purported to use a variable hash seed for Partial
HashAggregate, but actually did the opposite - it made us use a
variable seed for any HashAggregate that is NOT partial.  Woops.

Commit 4aec4989 reorganized the order
of operations here so that we no longer increment the number of "extra
waits" before locking the semaphore, but it did not change the
starting value of extraWaits from 0 to -1 to compensate.  In the worst
case, this could leak a semaphore count, but that seems to be unlikely
in practice.

Discussion: http://postgr.es/m/CAA4eK1JyVqXiMba+-a589Rk0pyHsyKkGxeumVKjU6Y74hdrVLQ@mail.gmail.com

Amit Kapila, per an off-list report by Dilip Kumar.  Reviewed by me.

With the old code, a backend that read pg_stat_activity without ever
having executed a parallel query might see a backend in the midst of
executing one waiting on a DSA LWLock, resulting in a crash.  The
solution is for backends to register the tranche at startup time, not
the first time a parallel query is executed.

Report by Andreas Seltenreich.  Patch by me, reviewed by Thomas Munro.

array_fill(..., array[0]) produced an empty array, which is probably
what users expect, but it was a one-dimensional zero-length array
which is not our standard representation of empty arrays.  Also, for
no very good reason, it rejected empty input arrays; that case should
be allowed and produce an empty output array.

In passing, remove the restriction that the input array(s) have lower
bound 1.  That seems rather pointless, and it would have needed extra
complexity to make the check deal with empty input arrays.

Per bug #14487 from Andrew Gierth.  It's been broken all along, so
back-patch to all supported branches.

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

Small number of fixes to perl docs for TAP tests.
Plus two comments that use "xlog" rather than WAL

Michael Paquier

Inheritance operations must treat the OID column, if any, much like
regular user columns.  But MergeAttributesIntoExisting() neglected to
do that, leading to weird results after a table with OIDs is associated
to a parent with OIDs via ALTER TABLE ... INHERIT.

Report and patch by Amit Langote, reviewed by Ashutosh Bapat, some
adjustments by me.  It's been broken all along, so back-patch to
all supported branches.

Discussion: https://postgr.es/m/cb13cfe7-a48c-5720-c383-bb843ab28298@lab.ntt.co.jp

Be more clear that we represent timestamps in microseconds when
integer timestamps are used, and in fractional seconds when
floating-point timestamps are used.

Discussion: http://postgr.es/m/20161212135045.GB15488@e733.localdomain

Report by Alexander Alekseev.  Wording by me with a suggestion
from Tom Lane.

Michael Paquier, per Coverity.

RelationGetPartitionQual() and generate_partition_qual() are always
called with recurse = true, so we don't need an argument for that.

Extracted by me from a larger patch by Amit Langote.

After a tuple is routed to a partition, it has been converted from the
root table's row type to the partition's row type.  ExecConstraints
needs to report the failure using the original tuple and the parent's
tuple descriptor rather than the ones for the selected partition.

Amit Langote

Craig Ringer, reviewed by Euler Taveira and Naoki Okano

Allow pg_recvlogical to specify an ending LSN, complementing
the existing -—startpos=LSN option.

Craig Ringer, reviewed by Euler Taveira and Naoki Okano

configure can only probe the existence of gcc intrinsics, not how well
they're implemented, and unfortunately the answer is sometimes "badly".
In particular we've found that multiple compilers fail to implement
char-width __sync_lock_test_and_set() correctly on PPC; and even a correct
implementation would necessarily be pretty inefficient, since that hardware
has only a word-wide primitive to work with.

Given the knowledge we've accumulated in s_lock.h, it appears that it's
best to rely on int-width TAS operations on most non-Intel architectures.
Hence, pick int not char when both are nominally available to us in
generic-gcc.h (note that that code is not used for x86[_64]).

Back-patch to fix regression test failures on FreeBSD/PPC.  Ordinarily
back-patching a change like this would be verboten because of ABI breakage.
But since pg_atomic_flag is not yet used in any Postgres data structure,
there's no ABI to break.  It seems safer to back-patch to avoid possible
gotchas, if someday we do back-patch something that uses pg_atomic_flag.

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

Commit 2ac3ef7a added a TupleTapleSlot
for partition tuple slot to EState (es_partition_tuple_slot) but it's
more logical to have it as part of ModifyTableState
(mt_partition_tuple_slot) and CopyState (partition_tuple_slot).

Discussion: http://postgr.es/m/1bd459d9-4c0c-197a-346e-e5e59e217d97@lab.ntt.co.jp

Amit Langote, per a gripe from me

Leave OpenSSL's default passphrase collection callback in place during
the first call of secure_initialize() in server startup.  Although that
doesn't work terribly well in daemon contexts, some people feel we should
not break it for anyone who was successfully using it before.  We still
block passphrase demands during SIGHUP, meaning that you can't adjust SSL
configuration on-the-fly if you used a passphrase, but this is no worse
than what it was before commit de41869b.  And we block passphrase demands
during EXEC_BACKEND reloads; that behavior wasn't useful either, but at
least now it's documented.

Tweak some related log messages for more readability, and avoid issuing
essentially duplicate messages about reload failure caused by a passphrase.

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

The typical size of an LWLock is now 16 bytes even on 64-bit platforms,
and the size of slock_t is now irrelevant.  But pg_atomic_uint32 can
(perhaps surprisingly) still be larger than 4 bytes, so there's still
some marginal point to allowing LWLOCK_MINIMAL_SIZE == 64.

Commit 008608b9 made the changes
that led to the need for these updates.

Add new tests for physical repl slots and hot standby feedback.

Craig Ringer, reviewed by Aleksander Alekseev and Simon Riggs

Add methods to the core test framework PostgresNode.pm to allow us to
test that standby nodes have caught up with the master, as well as
basic LSN handling.  Used in tests recovery/t/001_stream_rep.pl and
recovery/t/004_timeline_switch.pl

Craig Ringer, reviewed by Aleksander Alekseev and Simon Riggs

The purpose of the test was to check access to the sequence relation on
a hot standby, so change the test to read a different column from the
sequence, instead of just reading the catalog.

From: Andreas Karlsson <andreas@proxel.se>

These files are deleted but not yet gone from the filesystem. Operations
on them will return ERROR_DELETE_PENDING.

With this we start treating that as ENOENT, meaning files does not
exist (which is the state it will soon reach). This should be safe in
every case except when we try to recreate a file with exactly the same
name. This is an operation that PostgreSQL does very seldom, so
hopefully that won't happen much -- and even if it does, this treatment
should be no worse than treating it as an unhandled error.

We've been un able to reproduce the bug reliably, so pushing this to
master to get buildfarm coverage and other testing. Once it's proven to
be stable, it should be considered for backpatching.

Discussion: https://postgr.es/m/20160712083220.1426.58667%40wrigleys.postgresql.org

Patch by me and Michael Paquier