Guillaume Lelarge

Commit b8fd1a09 renamed XLOG_HINT to XLOG_FPI, but neglected two
places.

Backpatch to 9.3, like that commit.

This requires some core changes as well so that we can properly
WAL-log the truncation.  Specifically, it changes the format of the
XLOG_SMGR_TRUNCATE WAL record, so bump XLOG_PAGE_MAGIC.

Patch by me, reviewed but not fully endorsed by Andres Freund.

Commit 04ae11f6 removed some broken
code to apply the scan/join target to partial paths, but its theory
that this processing step is totally unnecessary turns out to be wrong.
Put similar code back again, but this time, check for parallel-safety
and avoid in-place modifications to paths that may already have been
used as part of some other path.

(This is not an entirely elegant solution to this problem; it might
be better, for example, to postpone generate_gather_paths for the
topmost scan/join rel until after the scan/join target has been
applied.  But this is not the time for such redesign work.)

Amit Kapila and Robert Haas

If you really want to vacuum every single page in the relation,
regardless of apparent visibility status or anything else, you can use
this option.  In previous releases, this behavior could be achieved
using VACUUM (FREEZE), but because we can now recognize all-frozen
pages as not needing to be frozen again, that no longer works.  There
should be no need for routine use of this option, but maybe bugs or
disaster recovery will necessitate its use.

Patch by me, reviewed by Andres Freund.

Almost all functions provided by this extension are PARALLEL
RESTRICTED.  Mostly, that's because the leader's TCP connections won't
be shared with the workers, but in some cases like dblink_get_pkey
it's because they obtain locks which might be released early if taken
within a parallel worker.  dblink_fdw_validator probably can't be used
in a query anyway, but there would be no problem from the point of
view of parallel query if it were, so it's PARALLEL SAFE.

Andreas Karlsson

Per gripe from Thomas Munro, who only complained about a more
localized problem, but I couldn't resist a bit more wordsmithing.

Thomas Munro

Discussion: <bfd204ab-ab1a-792a-b345-0274a09a4b5f@2ndquadrant.com>

In commit 8c1d9d56, I attempted to
add a regression test that would fail if the target list was pushed
into a parallel worker, but due to brain fade on my part, it just
randomly fails whether anything bad or not, because the error check
inside the parallel_restricted() function tests whether there is
*any process in the system* that is not connected to a client, not
whether the process running the query is not connected to a client.

A little experimentation has left me pessimistic about the
prospects of doing better here in a short amount of time, so let's
just fall back to checking that the plan is as we expect and leave
the execution-time check for another day.

The inet/cidr types sometimes failed to reject IPv6 inputs with too many
colon-separated fields, instead translating them to '::/0'.  This is the
result of a thinko in the original ISC code that seems to be as yet
unreported elsewhere.  Per bug #14198 from Stefan Kaltenbrunner.

Report: <20160616182222.5798.959@wrigleys.postgresql.org>

The fact that no workers were successfully launched in the previous
iteration does not excuse us from setting up properly to try again.
This appears to explain crashes I saw in parallel regression testing
due to error_mqh being NULL when it shouldn't be.

Minor other cosmetic fixes too.

The main point of doing this is to allow the cutoff to be set very small,
even zero, to allow parallel-query behavior to be tested on relatively
small tables such as we typically use in the regression tests.  But it
might be of use to users too.  The number-of-workers scaling behavior in
create_plain_partial_paths() is pretty ad-hoc and subject to change, so
we won't expose anything about that, but the notion of not considering
parallel query at all for tables below size X seems reasonably stable.

Amit Kapila, per a suggestion from me

Discussion: <17170.1465830165@sss.pgh.pa.us>

Emit "(null)" instead, which was the behavior all along on platforms
that don't crash, eg OS X.  Per report from Jehan-Guillaume de Rorthais.
Back-patch to 9.2 where -C option was introduced.

Michael Paquier

Report: <20160615204036.2d35d86a@firost>

Commit 6f56b41a removed function get_pg_database_relfilenode but left
its prototype in place.  Remove it.

The code in this area needs further revision, and it would be best
not to re-break the things we've already fixed.

Per a gripe from Tom Lane.

This allows the timestamps to follow local conventions (in particular,
they respond to the LC_TIME environment setting).  In C locale you get
the same results as before.  It seems like a good idea to do this now not
later because we already changed the format of \watch headers for 9.6.

Also, increase the buffer sizes a tad to ensure there's enough space for
translated strings.

Discussion: <20160612145532.GA22965@postgresql.kr>

Commit 14a254fb managed not to
exercise the code it was intended to test, and the comment explaining
why no "parallel worker" line showed up in the context wasn't right.

Amit Kapila, tweaked by me per Amit's analysis.

The new pg_check_visible() and pg_check_frozen() functions can be used to
verify that the visibility map bits for a relation's data pages match the
actual state of the tuples on those pages.

Amit Kapila and Robert Haas, reviewed (in earlier versions) by Andres
Freund.  Additional testing help by Thomas Munro.

Commit a892234f added a new bit per
page to the visibility map fork indicating whether the page is
all-frozen, but incorrectly assumed that if lazy_scan_heap chose to
freeze a tuple then that tuple would not need to later be frozen
again. This turns out to be false, because xmin and xmax (and
conceivably xvac, if dealing with tuples from very old releases) could
be frozen at separate times.

Thanks to Andres Freund for help in uncovering and tracking down this
issue.

currtid() and currtid2() call GetLatestSnapshot(), which fails in
parallel mode.  pg_export_snapshot() calls ExportSnapshot() which
attempts to assign an XID for the current transaction if it does not
already have one; that, too, will fail in parallel mode.

Andreas Seltenreich

We disable statement_timeout and lock_timeout during dump and restore, to
prevent any global settings that might exist from breaking routine backups.
Commit c6dda1f4 should have added idle_in_transaction_session_timeout to
that list, but failed to.

Another place where these timeouts get turned off is autovacuum.  While
I doubt an idle timeout could fire there, it seems better to be safe than
sorry.

pg_dump issue noted by Bernd Helmle, the other one found by grepping.

Report: <352F9B77DB5D3082578D17BB@eje.land.credativ.lan>

Format the example and test code more to Python style standards.
Improve whitespace.  Improve documentation formatting.

Also explain how generic plans are created.
Link to PREPARE docs from wire-protocol prepare docs.

Reported-by: Jonathan Rogers

Discussion: https://www.postgresql.org/message-id/flat/561E749D.4090301%40socialserve.com

All functions provided by this extension are PARALLEL SAFE.

Andreas Karlsson

All functions provided by this extension are PARALLEL SAFE.

Andreas Karlsson

All functions provided by this extension are PARALLEL SAFE.

Andreas Karlsson

All functions provided by this extension are PARALLEL RESTRICTED,
because they provide information about the connection state.  Parallel
workers don't have this information and therefore these functions
can't be executed in a worker (but they can be present in a query some
other part of which uses parallelism).

Andreas Karlsson

Commit 749a787c bumped the extension
version on all of these extensions already, and we haven't had a
release since then, so we can make further changes without bumping the
extension version again.  Take this opportunity to mark all of the
functions exported by these modules PARALLEL SAFE -- except for
pg_trgm's set_limit().  Mark that one PARALLEL RESTRICTED, because it
makes a persistent change to a GUC value.

Note that some of the markings added by this commit don't have any
effect; for example, gseg_picksplit() isn't likely to be mentioned
explicitly in a query and therefore it's parallel-safety marking will
never be consulted.  But this commit just marks everything for
consistency: if it were somehow used in a query, that would be fine as
far as parallel query is concerned, since it does not consult any
backend-private state, attempt to write data, etc.

Andreas Karlsson, with a few revisions by me.

As discovered by Andreas Seltenreich via sqlsmith, it's possible for a
remote join to need to generate a target list which contains a
PlaceHolderVar which would need to be evaluated on the remote server.
This happens when we try to push down a join tree which contains outer
joins and the nullable side of the join contains a subquery which
evauates some expression which can go to NULL above the level of the
join.  Since the deparsing logic can't build a remote query that
involves subqueries, it fails while trying to produce an SQL query
that can be sent to the remote side.  Detect such cases and don't try
to push down the join at all.

It's actually fine to push down the join if the PlaceHolderVar needs
to be evaluated at the current join level.  This patch makes a small
change to build_tlist_to_deparse so that this case will work.

Amit Langote, Ashutosh Bapat, and me.

Extension scripts should never use CREATE OR REPLACE for initial object
creation.  If there is a collision with a pre-existing (probably
user-created) object, we want extension installation to fail, not silently
overwrite the user's object.  Bloom and sslinfo both violated this precept.

Also fix a number of scripts that had no standard header (the file name
comment and the \echo...\quit guard).  Probably the \echo...\quit hack
is less important now than it was in 9.1 days, but that doesn't mean
that individual extensions get to choose whether to use it or not.

And fix a couple of evident copy-and-pasteos in file name comments.

No need for back-patch: the REPLACE bugs are both new in 9.6, and the
rest of this is pretty much cosmetic.

Andreas Karlsson and Tom Lane

Andreas Seltenreich reports that it is possible for a PlaceHolderVar
to creep into this tlist, and I fear that even after that's fixed we
might have other, similar bugs in this area either now or in the
future.  There's a lot of action-at-a-distance here, because the
validity of this assertion depends on core planner behavior; so, let's
use elog() to make sure we catch this even in non-assert builds,
rather than just crashing.

pg_type_aclmask reported the wrong type's OID when complaining that
it could not find a type's typelem.  It also failed to provide a
suitable errcode when the initially given OID doesn't exist (which
is a user-facing error, since that OID can be user-specified).
pg_foreign_data_wrapper_aclmask and pg_foreign_server_aclmask likewise
lacked errcode specifications.  Trivial cosmetic adjustments too.

The wrong-type-OID problem was reported by Petru-Florin Mihancea in
bug #14186; the other issues noted by me while reading the code.
These errors all seem to be aboriginal in the respective routines, so
back-patch as necessary.

Report: <20160613163159.5798.52928@wrigleys.postgresql.org>

The struct definition for PathTarget specifies that a NULL sortgrouprefs
pointer means no sortgroupref labels.  While it's likely that there
should always be at least one labeled column in the places that were
unconditionally fetching through the pointer, it seems wiser to adhere to
the data structure specification and test first.  Add a macro to make this
convenient.  Per experimentation with running the regression tests with a
very small parallelization threshold --- the crash I observed may well
represent a bug elsewhere, but still this coding was not very robust.

Report: <20756.1465834072@sss.pgh.pa.us>

Apparently, at least some versions of Microsoft's shell fail on variable
assignments that have leading whitespace.  This instance, introduced in
commit 680513ab, managed to escape notice for awhile because it's only
invoked if building with OpenSSL.  Per bug #14185 from Torben Dannhauer.

Report: <20160613140119.5798.78501@wrigleys.postgresql.org>

Every whole-tree perltidy run has used this version, firmly establishing
it as the de facto standard.

Rename schema -> schema_name etc. to remain consistent with C API and
PL/pgSQL.

While beneficial, both for throughput and average/worst case latency, in
a significant number of workloads, there are other workloads in which
backend_flush_after can cause significant performance regressions in
comparison to < 9.6 releases. The regression is most likely when the hot
data set is bigger than shared buffers, but significantly smaller than
the operating system's page cache.

I personally think that the benefit of enabling backend flush control is
considerably bigger than the potential downsides, but a fair argument
can be made that not regressing is more important than improving
performance/latency. As the latter is the consensus, change the default
to 0.

The other settings introduced in 428b1d6b do not have the same
potential for regressions, so leave them enabled.

Benchmarks leading up to changing the default have been performed by
Mithun Cy, Ashutosh Sharma and Robert Haas.

Discussion: CAD__OuhPmc6XH=wYRm_+Q657yQE88DakN4=Ybh2oveFasHkoeA@mail.gmail.com