This restriction is implicit in the check-only-once implementation we use
for table and domain constraints, but it wasn't spelled out anywhere, nor
was there any advice about how to alter a constraint's behavior safely.
Improve that.

I was also dissatisfied with the documentation of ALTER DOMAIN VALIDATE
CONSTRAINT, which entirely failed to explain the use of that feature; and
thence decided that ALTER TABLE VALIDATE CONSTRAINT could be documented
better as well.

Perhaps we should back-patch this, along with the related commit 36d442a2,
but for now I refrained.

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

Missed in commit 578b2297.

This is an astonishingly ancient bit of silliness, dating AFAICS to
commit edb519b1 of 27-Jul-1996 which added the pipe close stanza in
the wrong place.  It happens to be harmless given that the code above
this won't enable the pager if html3 output mode is selected.  Still,
somebody might try to relax that restriction someday, and in any case
it could confuse readers and static analysis tools, so let's fix it in
HEAD.

Per bug #15541 from Pan Bian.

Discussion: https://postgr.es/m/15541-c835d8b9a903f7ad@postgresql.org

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 d42358ef 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 d42358ef; 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 d42358ef so that some of them
use tables with toast tables.

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

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

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 665d1fad).
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

Jeff Janes

Discussion: https://postgr.es/m/CAMkU=1w7Tn2M9BhK+rt8Shtz1AkU+ty7By8gj5C==z65=U4vyQ@mail.gmail.com

As per the error message style guide of the documentation, those should
be full sentences.

Author: Daniel Gustafsson
Reviewed-by: Michael Paquier, Álvaro Herrera
Discussion: https://1E8D49B4-16BC-4420-B4ED-58501D9E076B@yesql.se

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

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

In dumputils, we may have successfully parsed the acls when we discover
that we can't parse the reverse ACLs and then return- check and free
aclitems if that happens.

In dumpTableSchema, move ftoptions and srvname under the relkind !=
RELKIND_VIEW branch (since they're only used there) and then check if
they've been allocated and, if so, free them at the end of that block.

Pointed out by Pavel Raiskup, though I didn't use those patches.

Discussion: https://postgr.es/m/2183976.vkCJMhdhmF@nb.usersys.redhat.com

Skipping over the "hole" in full page images in the XLOG code was
described as being a form of compression, but this got a bit confusing
since we now have PGLZ-based compression happening, so adjust the
wording to discuss "removing" the "hole" and keeping the talk about
compression to where we're talking about using PGLZ-based compression of
the full page images.

Reviewed-By: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20181127234341.GM3415@tamriel.snowman.net

We prefer to use on/off than true/false for boolean configuration
parameters in the documentation, but there were a few places where
true/false were still used.

Dicussion: https://postgr.es/m/20181202.072508.618341295047874293.t-ishii%40sraoss.co.jp

When an indexes is created on a partitioned table using ONLY (don't
recurse to partitions), it gets marked invalid until index partitions
are attached for each table partition.  But there's no reason to do this
if there are no partitions ... and moreover, there's no way to get the
index to become valid afterwards, because all partitions that get
created/attached get their own index partition already attached to the
parent index, so there's no chance to do ALTER INDEX ... ATTACH PARTITION
that would make the parent index valid.

Fix by not marking the index as invalid to begin with.

This is very similar to 9139aa19, but the pg_dump aspect does not
appear to be relevant until we add FKs that can point to PKs on
partitioned tables.  (I tried to cause the pg_upgrade test to break by
leaving some of these bogus tables around, but wasn't able to.)

Making this change means that an index that was supposed to be invalid
in the insert_conflict regression test is no longer invalid; reorder the
DDL so that the test continues to verify the behavior we want it to.

Author: Álvaro Herrera
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/20181203225019.2vvdef2ybnkxt364@alvherre.pgsql

"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

Backends don't typically exist uncleanly, but they can certainly exit
uncleanly, and it's exiting uncleanly that's being discussed here.

In commit 7012b132, which added aggregate
pushdown to postgres_fdw, we didn't account for the evaluation cost and the
selectivity of HAVING quals attached to ForeignPaths performing aggregate
pushdown, as core had never accounted for that for AggPaths and GroupPaths.
And we didn't set these values of the locally-checked quals (ie, fpinfo's
local_conds_cost and local_conds_sel), which were initialized to zeros, but
since estimate_path_cost_size factors in these to estimate the result size
and the evaluation cost of such a ForeignPath when the use_remote_estimate
option is enabled, this caused it to produce underestimated results in that
case.

By commit 7b6c0754 core was changed so that
it accounts for the evaluation cost and the selectivity of HAVING quals in
aggregation paths, so change the postgres_fdw's aggregate pushdown code as
well as such.  This not only fixes the underestimation issue mentioned
above, but improves the estimation using local statistics in that function
when that option is disabled.

This would be a bug fix rather than an improvement, but apply it to HEAD
only to avoid destabilizing existing plan choices.

Author: Etsuro Fujita
Discussion: https://postgr.es/m/5BFD3EAD.2060301%40lab.ntt.co.jp

Expand section 5.6 "Privileges" to include the full definition of
each privilege type, and an explanation of aclitem privilege displays,
along with some helpful summary tables.  Most of this material came
out of the GRANT reference page, although some of it is new.
Adjust a bunch of links that were pointing to GRANT to point to 5.6.

Fabien Coelho and Tom Lane, reviewed by Bradley DeJong

Discussion: https://postgr.es/m/alpine.DEB.2.21.1807311735200.20743@lancre

This does not improve the security and reliability of the touched areas,
but it makes the style more consistent.

Author: Michael Paquier
Reviewed-by- Noah Misch
Discussion: https://postgr.es/m/20180309075538.GD9376@paquier.xyz

The following options are added for extensions:
- TAP_TESTS, to allow an extention to run TAP tests which are the ones
present in t/*.pl.  A subset of tests can always be run with the
existing PROVE_TESTS for developers.
- ISOLATION, to define a list of isolation tests.
- ISOLATION_OPTS, to pass custom options to isolation_tester.

A couple of custom Makefile rules have been accumulated across the tree
to cover the lack of facility in PGXS for a couple of releases when
using those test suites, which are all now replaced with the new flags,
without reducing the test coverage.  Note that tests of contrib/bloom/
are not enabled yet, as those are proving unstable in the buildfarm.

Author: Michael Paquier
Reviewed-by: Adam Berlin, Álvaro Herrera, Tom Lane, Nikolay Shaplov,
Arthur Zakirov
Discussion: https://postgr.es/m/20180906014849.GG2726@paquier.xyz

Re-making ecpglib's typename.o is dangerous because another make thread
could be doing that at the same time.  While we've not heard field
complaints traceable to this, it seems inevitable that it'd bite someone
eventually.  Instead, symlink typename.c into the preproc directory and
recompile it there.  That file is small enough that compiling it twice
isn't much of a penalty.  Furthermore, this way we get a .o file that's
made without shlib CFLAGS, which seems cleaner.

This requires adding more stuff to the module's -I list.  The MSVC
aspect of that is untested, but I'm sure the buildfarm will tell me
if I got it wrong.

Per a suggestion from Peter Eisentraut.  Although this is theoretically
a bug fix, the lack of field reports makes me feel we needn't back-patch.

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

This should reduce confusion, and in particular make it safe to
copy typename.c into preproc/ and compile it there.

This doesn't affect anything outside ecpg, and particularly not
end users, because these files don't get installed; they just
exist to share declarations among the .c files of each subdirectory.

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

This allows control of the directory in which the postmaster sockets
are created for the temporary postmasters started by pg_upgrade.
The default location remains the current working directory, which is
typically fine, but if it is deeply nested then its pathname might
be too long to be a socket name.

In passing, clean up some messiness in pg_upgrade's option handling,
particularly the confusing and undocumented way that configuration-only
datadirs were handled.  And fix check_required_directory's substantially
under-baked cleanup of directory pathnames.

Daniel Gustafsson, reviewed by Hironobu Suzuki, some code cleanup by me

Discussion: https://postgr.es/m/E72DD5C3-2268-48A5-A907-ED4B34BEC223@yesql.se

TAP tests on msys need to run with the DTK perl, which understands msys
virtualized paths.  Postgres, however, does not understand such paths,
so before a path can be used safely with CREATE TABLESPACE, it needs to
be translated into a path on the underlying file system.

Per report from buildfarm member jacana.  Suggested fix is from Andrew
Dunstan.

Discussion: https://postgr.es/m/20181130053555.GF2267@paquier.xyz

My original coding was questionable anyway.

Reported-by: Sergei Kornilov
Discussion: https://postgr.es/m/9645101543575886@myt6-27270b78ac4f.qloud-c.yandex.net

Three issues are fixed in this patch:
- Base backups forgot to ignore files specific to EXEC_BACKEND, leading
to spurious warnings when checksums are enabled, per analysis from me.
- pg_verify_checksums forgot about files specific to EXEC_BACKEND,
leading to failures of the tool on any such build, particularly Windows.
This error was originally found by newly-introduced TAP tests in various
buildfarm members using EXEC_BACKEND.
- pg_verify_checksums forgot to count for temporary files and temporary
paths, which could be valid relation files, without checksums, per
report from Andres Freund.  More tests are added to cover this case.

A new test case which emulates corruption for a file in a different
tablespace is added, coming from from Michael Banck, while I have coded
the main code and refactored the test code.

Author: Michael Banck, Michael Paquier
Reviewed-by: Stephen Frost, David Steele
Discussion: https://postgr.es/m/20181021134206.GA14282@paquier.xyz

This basically reverts commit d55241af,
leaving around a portion of the regression tests still adapted with
empty relation files, and corrupted cases.  This is also proving to be
failing to check properly relation files located in a non-default
tablespace path.

Per discussion with various folks, including Stephen Frost, David
Steele, Andres Freund, Michael Banck and myself.

Reported-by: Michael Banck
Discussion: https://postgr.es/m/20181021134206.GA14282@paquier.xyz
Backpatch-through: 11

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

This allows to set a lower log_min_duration_statement value without
incurring excessive log traffic (which reduces performance).  This can
be useful to analyze workloads with lots of short queries.

Author: Adrien Nayrat
Reviewed-by: David Rowley, Vik Fearing
Discussion: https://postgr.es/m/c30ee535-ee1e-db9f-fa97-146b9f62caed@anayrat.info

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

This used to be on the web site but was removed.  The documentation is
a better place for it anyway.

Author: David Rowley <david.rowley@2ndquadrant.com>
Reviewed-by: John Naylor <jcnaylor@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAKJS1f_dKdejdKB94nKZC9S5NzB-UZRcAKkE84e=JEEecDuotg@mail.gmail.com/

This fixes an oversight from d5eec4ee.

Author: Michael Paquier
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/20181129072719.GC9004@paquier.xyz

When fetching a list of tests for a given extension in contrib/ or
src/test/modules/, NO_INSTALLCHECK now gets checked first.  If present,
an empty list of tests is returned to let the caller know that tests
for this module need to be bypassed.

This actually fixes a set of issues with MSVC with modules using
REGRESS_OPTS, as an incorrect parsing caused the launched command
to eat the first test listed.  The actual effect on the tree is that
several modules listed a single test, so regressions have been running
with no actual tests.  pg_stat_statements, test_rls_hooks and commit_ts
were impacted by that.  Some other modules like test_decoding (or
snapshot_too_old) don't use yet PGXS rules, but their makefiles will
soon be refactored with an upcoming patch.

Author: Michael Paquier
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/20181126054302.GI1776@paquier.xyz

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

This bypasses installcheck if specified, which makes sense for those
modules as they require non-default configuration, something which
typical users don't have.  Those have been missing from the start, still
no back-patch is done.

This will be used by an upcoming patch for MSVC scripts adding support
for NO_INSTALLCHECK as installcheck is the default mode for contrib and
modules for performance reasons in the buildfarm.

Author: Michael Paquier
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/20181126054302.GI1776@paquier.xyz

This fixes an oversight from c6c33343 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

Move the responsibility for checking for and reporting a failure from
the only current BufFileSize() caller, logtape.c, to BufFileSize()
itself.  Code within buffile.c is generally responsible for interfacing
with fd.c to report irrecoverable failures.  This seems like a
convention that's worth sticking to.

Reorganizing things this way makes it easy to make the error message
raised in the event of BufFileSize() failure descriptive of the
underlying problem.  We're now clear on the distinction between
temporary file name and BufFile name, and can show errno, confident that
its value actually relates to the error being reported.  In passing, an
existing, similar buffile.c ereport() + errcode_for_file_access() site
is changed to follow the same conventions.

The API of the function BufFileSize() is changed by this commit, despite
already being in a stable release (Postgres 11).  This seems acceptable,
since the BufFileSize() ABI was changed by commit aa551830, which
hasn't made it into a point release yet.  Besides, it's difficult to
imagine a third party BufFileSize() caller not just raising an error
anyway, since BufFile state should be considered corrupt when
BufFileSize() fails.

Per complaint from Tom Lane.

Discussion: https://postgr.es/m/26974.1540826748@sss.pgh.pa.us
Backpatch: 11-, where shared BufFiles were introduced.

The previous recovery.conf regime accepted multiple recovery_target*
settings and used the last one.  This does not translate well to the
general GUC system.  Specifically, under EXEC_BACKEND, the settings
are written out not in any particular order, so the order in which
they were originally set is not available to new processes.

Rather than redesign the GUC system, it was decided to abandon the old
behavior and only allow one recovery target setting.  A second setting
will cause an error.  However, it is allowed to set the same parameter
multiple times or unset a parameter and set a different one.

Discussion: https://www.postgresql.org/message-id/flat/27802171543235530%40iva2-6ec8f0a6115e.qloud-c.yandex.net#701a59c837ad0bf8c244344aaf3ef5a4

Reported-by: Etsuro Fujita

Discussion: https://postgr.es/m/5BFE34DE.1080404@lab.ntt.co.jp

Author: Etsuro Fujita

Backpatch-through: 10

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