Revert Windows service check refactoring, and replace with a different fix.

This reverts commit 38bdba54a64bacec78e3266f0848b0b4a824132a, "Fix and
simplify check for whether we're running as Windows service". It turns out
that older versions of MinGW - like that on buildfarm member narwhal - do
not support the CheckTokenMembership() function. This replaces the
refactoring with a much smaller fix, to add a check for SE_GROUP_ENABLED to
pgwin32_is_service().

Only apply to back-branches, and keep the refactoring in HEAD. It's
unlikely that anyone is still really using such an old version of MinGW -
aside from narwhal - but let's not change the minimum requirements in
minor releases.

Discussion: https://www.postgresql.org/message-id/16609.1489773427@sss.pgh.pa.us
Patch: https://www.postgresql.org/message-id/CAB7nPqSvfu%3DKpJ%3DNX%2BYAHmgAmQdzA7N5h31BjzXeMgczhGCC%2BQ%40mail.gmail.com

doc: Fix a few typos and awkward links

Remove dead link.

David Christensen

Discussion: http://postgr.es/m/82299377-1480-4439-9ABA-5828D71AA22E@endpoint.com

Fix and simplify check for whether we're running as Windows service.

If the process token contains SECURITY_SERVICE_RID, but it has been
disabled by the SE_GROUP_USE_FOR_DENY_ONLY attribute, win32_is_service()
would incorrectly report that we're running as a service. That situation
arises, e.g. if postmaster is launched with a restricted security token,
with the "Log in as Service" privilege explicitly removed.

Replace the broken code with CheckProcessTokenMembership(), which does
this correctly. Also replace similar code in win32_is_admin(), even
though it got this right, for simplicity and consistency.

Per bug #13755, reported by Breen Hagan. Back-patch to all supported
versions. Patch by Takayuki Tsunakawa, reviewed by Michael Paquier.

Discussion: https://www.postgresql.org/message-id/20151104062315.2745.67143%40wrigleys.postgresql.org

Avoid having vacuum set reltuples to 0 on non-empty relations in the
presence of page pins, which leads to serious estimation errors in the
planner.  This particularly affects small heavily-accessed tables,
especially where locking (e.g. from FK constraints) forces frequent
vacuums for mxid cleanup.

Fix by keeping separate track of pages whose live tuples were actually
counted vs. pages that were only scanned for freezing purposes.  Thus,
reltuples can only be set to 0 if all pages of the relation were
actually counted.

Backpatch to all supported versions.

Per bug #14057 from Nicolas Baccelli, analyzed by me.

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

Fix ancient get_object_address_opf_member bug

The original coding was trying to use a TypeName as a string Value,
which doesn't work; an oversight in my commit a61fd533.  Repair.

Also, make sure we cover the broken case in the relevant test script.

Backpatch to 9.5.

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

Spelling fixes

From: Josh Soref <[email protected]>

Fix failure to mark init buffers as BM_PERMANENT.

This could result in corruption of the init fork of an unlogged index
if the ambuildempty routine for that index used shared buffers to
create the init fork, which was true for brin, gin, gist, and hash
indexes.

Patch by me, based on an earlier patch by Michael Paquier, who also
reviewed this one.  This also incorporates an idea from Artur
Zakirov.

Discussion: http://postgr.es/m/CACYUyc8yccE4xfxhqxfh_Mh38j7dRFuxfaK1p6dSNAEUakxUyQ@mail.gmail.com

Remove unnecessary dependency on statement_timeout in prepared_xacts test.

Rather than waiting around for statement_timeout to expire, we can just
try to take the table's lock in nowait mode.  This saves some fraction
under 4 seconds when running this test with prepared xacts available,
and it guards against timeout-expired-anyway failures on very slow
machines when prepared xacts are not available, as seen in a recent
failure on axolotl for instance.

This approach could fail if autovacuum were to take an exclusive lock
on the test table concurrently, but there's no reason for it to do so.

Since the main point here is to improve stability in the buildfarm,
back-patch to all supported branches.

Ecpg should support COMMIT PREPARED and ROLLBACK PREPARED.

The problem was that "begin transaction" was issued automatically
before executing COMMIT/ROLLBACK PREPARED if not in auto commit. This fix by
Masahiko Sawada fixes this.

Fix pg_file_write() error handling.

Detect fclose() failures; given "ln -s /dev/full $PGDATA/devfull",
"pg_file_write('devfull', 'x', true)" now fails as it should.  Don't
leak a stream when fwrite() fails.  Remove a born-ineffective test that
aimed to skip zero-length writes.  Back-patch to 9.2 (all supported
versions).

Fix ancient connection leak in dblink

When using unnamed connections with dblink, every time a new
connection is made, the old one is leaked. Fix that.

This has been an issue probably since dblink was first committed.
Someone complained almost ten years ago, but apparently I decided
not to pursue it at the time, and neither did anyone else, so it
slipped between the cracks. Now that someone else has complained,
fix in all supported branches.

Discussion: (orig) https://postgr.es/m/flat/F680AB59-6D6F-4026-9599-1BE28880273D%40decibel.org#F680AB59-6D6F-4026-9599-1BE28880273D@decibel.org
Discussion: (new) https://postgr.es/m/flat/0A3221C70F24FB45833433255569204D1F6ADF8C@G01JPEXMBYT05
Reported by: Jim Nasby and Takayuki Tsunakawa

Sanitize newlines in object names in "pg_restore -l" output.

Commits 89e0bac86 et al replaced newlines with spaces in object names
printed in SQL comments, but we neglected to consider that the same
names are also printed by "pg_restore -l", and a newline would render
the output unparseable by "pg_restore -L".  Apply the same replacement
in "-l" output.  Since "pg_restore -L" doesn't actually examine any
object names, only the dump ID field that starts each line, this is
enough to fix things for its purposes.

The previous fix was treated as a security issue, and we might have
done that here as well, except that the issue was reported publicly
to start with.  Anyway it's hard to see how this could be exploited
for SQL injection; "pg_restore -L" doesn't do much with the file
except parse it for leading integers.

Per bug #14587 from Milos Urbanek.  Back-patch to all supported versions.

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

Fix a potential double-free in ecpg.

Fix timestamptz regression test to still work with latest IANA zone data.

The IANA timezone crew continues to chip away at their project of removing
timezone abbreviations that have no real-world currency from their
database.  The tzdata2017a update removes all such abbreviations for
South American zones, as well as much of the Pacific.  This breaks some
test cases in timestamptz.sql that were expecting America/Santiago and
America/Caracas to have non-numeric abbreviations.

The test cases involving America/Santiago seem to have selected that
zone more or less at random, so just replace it with America/New_York,
which is of similar longitude.  The cases involving America/Caracas are
harder since they were chosen to test a time-varying zone abbreviation
around a point where it changed meaning in the backwards direction.
Fortunately, Europe/Moscow has a similar case in 2014, and the MSK/MSD
abbreviations are well enough attested that IANA seems unlikely to
decide to remove them from the database in future.

With these changes, this regression test should pass when using any IANA
zone database from 2015 or later.  One could wish that there were a few
years more daylight on how out-of-date your zone database can be ... but
really the --with-system-tzdata option is only meant for use on platforms
where the zone database is kept up-to-date pretty faithfully, so I do not
think this is a big objection.

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

Use doubly-linked block lists in aset.c to reduce large-chunk overhead.

Large chunks (those too large for any palloc freelist) are managed as
separate blocks.  Formerly, realloc'ing or pfree'ing such a chunk required
O(N) time in a context with N blocks, since we had to traipse down the
singly-linked block list to locate the block's predecessor before we could
fix the list links.  This can result in O(N^2) runtime in situations where
large numbers of such chunks are manipulated within one context.  Cases
like that were not foreseen in the original design of aset.c, and indeed
didn't arise until fairly recently.  But such problems can now occur in
reorderbuffer.c and in hash joining, both of which make repeated large
requests without scaling up their request size as they do so, and which
will free their requests in not-necessarily-LIFO order.

To fix, change the block list from singly-linked to doubly-linked.
This adds another 4 or 8 bytes to ALLOC_BLOCKHDRSZ, but that doesn't
seem like unacceptable overhead, since aset.c's blocks are normally
8K or more, and never less than 1K in current practice.

In passing, get rid of some redundant AllocChunkGetPointer() calls in
AllocSetRealloc (the compiler might be smart enough to optimize these
away anyway, but no need to assume that) and improve AllocSetCheck's
checking of block header fields.

Back-patch to 9.4 where reorderbuffer.c appeared.  We could take this
further back, but currently there's no evidence that it would be useful.

Discussion: https://postgr.es/m/CAMkU=1x1hvue1XYrZoWk_omG0Ja5nBvTdvgrOeVkkeqs71CV8g@mail.gmail.com

pg_xlogdump: Remove extra newline in error message

fatal_error() already prints out a trailing newline.

Fix grammar

Reported by Jeremy Finzel

Repair incorrect pg_dump labeling for some comments and security labels.

We attached no schema label to comments for procedural languages, casts,
transforms, operator classes, operator families, or text search objects.
The first three categories of objects don't really have schemas, but
pg_dump treats them as if they do, and it seems like the TocEntry fields
for their comments had better match the TocEntry fields for the parent
objects.  (As an example of a possible hazard, the type names in a CAST
will be formatted with the assumption of a particular search_path, so
failing to ensure that this same path is active for the COMMENT ON command
could lead to an error or to attaching the comment to the wrong cast.)
In the last six cases, this was a flat-out error --- possibly mine to
begin with, but it was a long time ago.

The security label for a procedural language was likewise not correctly
labeled as to schema, and both the comment and security label for a
procedural language were not correctly labeled as to owner.

In simple cases the restore would accidentally work correctly anyway, since
these comments and security labels would normally get emitted right after
the owning object, and so the search path and active user would be correct
anyhow.  But it could fail in corner cases; for example a schema-selective
restore would omit comments it should include.

Giuseppe Broccolo noted the oversight, and proposed the correct fix, for
text search dictionary objects; I found the rest by cross-checking other
dumpComment() calls.  These oversights are ancient, so back-patch all
the way.

Discussion: https://postgr.es/m/CAFzmHiWwwzLjzwM4x5ki5s_PDMR6NrkipZkjNnO3B0xEpBgJaA@mail.gmail.com

pg_upgrade: Fix large object COMMENTS, SECURITY LABELS

When performing a pg_upgrade, we copy the files behind pg_largeobject
and pg_largeobject_metadata, allowing us to avoid having to dump out and
reload the actual data for large objects and their ACLs.

Unfortunately, that isn't all of the information which can be associated
with large objects.  Currently, we also support COMMENTs and SECURITY
LABELs with large objects and these were being silently dropped during a
pg_upgrade as pg_dump would skip everything having to do with a large
object and pg_upgrade only copied the tables mentioned to the new
cluster.

As the file copies happen after the catalog dump and reload, we can't
simply include the COMMENTs and SECURITY LABELs in pg_dump's binary-mode
output but we also have to include the actual large object definition as
well.  With the definition, comments, and security labels in the pg_dump
output and the file copies performed by pg_upgrade, all of the data and
metadata associated with large objects is able to be successfully pulled
forward across a pg_upgrade.

In 9.6 and master, we can simply adjust the dump bitmask to indicate
which components we don't want.  In 9.5 and earlier, we have to put
explciit checks in in dumpBlob() and dumpBlobs() to not include the ACL
or the data when in binary-upgrade mode.

Adjustments made to the privileges regression test to allow another test
(large_object.sql) to be added which explicitly leaves a large object
with a comment in place to provide coverage of that case with
pg_upgrade.

Back-patch to all supported branches.

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

Avoid dangling pointer to relation name in RLS code path in DoCopy().

With RLS active, "COPY tab TO ..." failed under -DRELCACHE_FORCE_RELEASE,
and would sometimes fail without that, because it used the relation name
directly from the relcache as part of the parsetree it's building.  That
becomes a potentially-dangling pointer as soon as the relcache entry is
closed, a bit further down.  Typical symptom if the relcache entry chanced
to get cleared would be "relation does not exist" error with a garbage
relation name, or possibly a core dump; but if you were really truly
unlucky, the COPY might copy from the wrong table.

Per report from Andrew Dunstan that regression tests fail with
-DRELCACHE_FORCE_RELEASE.  The core tests now pass for me (but have
not tried "make check-world" yet).

Discussion: https://postgr.es/m/7b52f900-0579-cda9-ae2e-de5da17090e6@2ndQuadrant.com

In rebuild_relation(), don't access an already-closed relcache entry.

This reliably fails with -DRELCACHE_FORCE_RELEASE, as reported by
Andrew Dunstan, and could sometimes fail in normal operation, resulting
in a wrong persistence value being used for the transient table.
It's not immediately clear to me what effects that might have beyond
the risk of a crash while accessing OldHeap->rd_rel->relpersistence,
but it's probably not good.

Bug introduced by commit f41872d0c, and made substantially worse by
commit 85b506bbf, which added a second such access significantly
later than the heap_close.  I doubt the first reference could fail
in a production scenario, but the second one definitely could.

Discussion: https://postgr.es/m/7b52f900-0579-cda9-ae2e-de5da17090e6@2ndQuadrant.com

Fix incorrect variable datatype

Both datatypes map to the same underlying one which is why it still
worked, but we should use the correct type.

Author: Kyotaro HORIGUCHI

pg_upgrade docs:  clarify instructions on standby extensions

Previously the pg_upgrade standby upgrade instructions said not to
execute pgcrypto.sql, but it should have referenced the extension
command "CREATE EXTENSION pgcrypto".  This patch makes that doc change.

Reported-by: a private bug report
Backpatch-through: 9.4, where standby instructions were added

Fix contrib/pg_trgm's extraction of trigrams from regular expressions.

The logic for removing excess trigrams from the result was faulty.
It intends to avoid merging the initial and final states of the NFA,
which is necessary, but in testing whether removal of a specific trigram
would cause that, it failed to consider the combined effects of all the
state merges that that trigram's removal would cause.  This could result
in a broken final graph that would never match anything, leading to GIN
or GiST indexscans not finding anything.

To fix, add a "tentParent" field that is used only within this loop,
and set it to show state merges that we are tentatively going to do.
While examining a particular arc, we must chase up through tentParent
links as well as regular parent links (the former can only appear atop
the latter), and we must account for state init/fin flag merges that
haven't actually been done yet.

To simplify the latter, combine the separate init and fin bool fields
into a bitmap flags field.  I also chose to get rid of the "children"
state list, which seems entirely inessential.

Per bug #14563 from Alexey Isayko, which the added test cases are based on.
Back-patch to 9.3 where this code was added.

Report: https://postgr.es/m/20170222111446[email protected]
Discussion: https://postgr.es/m/8816.1487787594@sss.pgh.pa.us

Make walsender always initialize the buffers.

Walsender uses the local buffers for each outgoing and incoming message.
Previously when creating replication slot, walsender forgot to initialize
one of them and which can cause the segmentation fault error. To fix this
issue, this commit changes walsender so that it always initialize them
before it executes the requested replication command.

Back-patch to 9.4 where replication slot was introduced.

Problem report and initial patch by Stas Kelvich, modified by me.
Report: https://www.postgresql.org/message-id/A1E9CB90-1FAC-4CAD-8DBA-9AA62A6E97C5@postgrespro.ru

Fix sloppy handling of corner-case errors in fd.c.

Several places in fd.c had badly-thought-through handling of error returns
from lseek() and close().  The fact that those would seldom fail on valid
FDs is probably the reason we've not noticed this up to now; but if they
did fail, we'd get quite confused.

LruDelete and LruInsert actually just Assert'd that lseek never fails,
which is pretty awful on its face.

In LruDelete, we indeed can't throw an error, because that's likely to get
called during error abort and so throwing an error would probably just lead
to an infinite loop.  But by the same token, throwing an error from the
close() right after that was ill-advised, not to mention that it would've
left the LRU state corrupted since we'd already unlinked the VFD from the
list.  I also noticed that really, most of the time, we should know the
current seek position and it shouldn't be necessary to do an lseek here at
all.  As patched, if we don't have a seek position and an lseek attempt
doesn't give us one, we'll close the file but then subsequent re-open
attempts will fail (except in the somewhat-unlikely case that a
FileSeek(SEEK_SET) call comes between and allows us to re-establish a known
target seek position).  This isn't great but it won't result in any state
corruption.

Meanwhile, having an Assert instead of an honest test in LruInsert is
really dangerous: if that lseek failed, a subsequent read or write would
read or write from the start of the file, not where the caller expected,
leading to data corruption.

In both LruDelete and FileClose, if close() fails, just LOG that and mark
the VFD closed anyway.  Possibly leaking an FD is preferable to getting
into an infinite loop or corrupting the VFD list.  Besides, as far as I can
tell from the POSIX spec, it's unspecified whether or not the file has been
closed, so treating it as still open could be the wrong thing anyhow.

I also fixed a number of other places that were being sloppy about
behaving correctly when the seekPos is unknown.

Also, I changed FileSeek to return -1 with EINVAL for the cases where it
detects a bad offset, rather than throwing a hard elog(ERROR).  It seemed
pretty inconsistent that some bad-offset cases would get a failure return
while others got elog(ERROR).  It was missing an offset validity check for
the SEEK_CUR case on a closed file, too.

Back-patch to all supported branches, since all this code is fundamentally
identical in all of them.

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

doc: Update URL for plr

Fix documentation of to_char/to_timestamp TZ, tz, OF formatting patterns.

These are only supported in to_char, not in the other direction, but the
documentation failed to mention that.  Also, describe TZ/tz as printing the
time zone "abbreviation", not "name", because what they print is elsewhere
referred to that way.  Per bug #14558.

Make src/interfaces/libpq/test clean up after itself.

It failed to remove a .o file during "make clean", and it lacked
a .gitignore file entirely.

Adjust PL/Tcl regression test to dodge a possible bug or zone dependency.

One case in the PL/Tcl tests is observed to fail on RHEL5 with a Turkish
time zone setting.  It's not clear if this is an old Tcl bug or something
odd about the zone data, but in any case that test is meant to see if the
Tcl [clock] command works at all, not what its corner-case behaviors are.
Therefore we have no need to test exactly which week a Sunday midnight is
considered to fall into.  Probe the following Tuesday instead.

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

Fix help message for pg_basebackup -R

The recovery.conf file that's generated is specifically for
replication, and not needed (or wanted) for regular backup restore, so
indicate that in the message.

Document usage of COPT environment variable for adjusting configure flags.

Also add to the existing rather half-baked description of PROFILE,
which does exactly the same thing, but I think people use it differently.

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

Doc: remove duplicate index entry.

This causes a warning with the old html-docs toolchain, though not with the
new.  I had originally supposed that we needed both <indexterm> entries to
get both a primary index entry and a see-also link; but evidently not,
as pointed out by Fabien Coelho.

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

Formatting and docs corrections for logical decoding output plugins.

Make the typedefs for output plugins consistent with project style;
they were previously not even consistent with each other as to layout
or inclusion of parameter names.  Make the documentation look the same,
and fix errors therein (missing and misdescribed parameters).

Back-patch because of the documentation bugs.

Doc: fix typo in logicaldecoding.sgml.

There's no such field as OutputPluginOptions.output_mode;
it's actually output_type.  Noted by T. Katsumata.

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

Make sure that hash join's bulk-tuple-transfer loops are interruptible.

The loops in ExecHashJoinNewBatch(), ExecHashIncreaseNumBatches(), and
ExecHashRemoveNextSkewBucket() are all capable of iterating over many
tuples without ever doing a CHECK_FOR_INTERRUPTS, so that the backend
might fail to respond to SIGINT or SIGTERM for an unreasonably long time.
Fix that.  In the case of ExecHashJoinNewBatch(), it seems useful to put
the added CHECK_FOR_INTERRUPTS into ExecHashJoinGetSavedTuple() rather
than directly in the loop, because that will also ensure that both
principal code paths through ExecHashJoinOuterGetTuple() will do a
CHECK_FOR_INTERRUPTS, which seems like a good idea to avoid surprises.

Back-patch to all supported branches.

Tom Lane and Thomas Munro

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

Fix YA unwanted behavioral difference with operator_precedence_warning.

Jeff Janes noted that the error cursor position shown for some errors
would vary when operator_precedence_warning is turned on.  We'd prefer
that option to have no undocumented effects, so this isn't desirable.
To fix, make sure that an AEXPR_PAREN node has the same exprLocation
as its child node.

(Note: it would be a little cheaper to use @2 here instead of an
exprLocation call, but there are cases where that wouldn't produce
the identical answer, so don't do it like that.)

Back-patch to 9.5 where this feature was introduced.

Discussion: https://postgr.es/m/CAMkU=1ykK+VhhcQ4Ky8KBo9FoaUJH3f3rDQB8TkTXi-ZsBRUkQ@mail.gmail.com

Ignore tablespace ACLs when ignoring schema ACLs.

The ALTER TABLE ALTER TYPE implementation can issue DROP INDEX and
CREATE INDEX to refit existing indexes for the new column type.  Since
this CREATE INDEX is an implementation detail of an index alteration,
the ensuing DefineIndex() should skip ACL checks specific to index
creation.  It already skips the namespace ACL check.  Make it skip the
tablespace ACL check, too.  Back-patch to 9.2 (all supported versions).

Reviewed by Tom Lane.

Blind try to fix portability issue in commit 8f93bd851 et al.

The S/390 members of the buildfarm are showing failures indicating
that they're having trouble with the rint() calls I added yesterday.
There's no good reason for that, and I wonder if it is a compiler bug
similar to the one we worked around in d9476b838.  Try to fix it using
the same method as before, namely to store the result of rint() back
into a "double" variable rather than immediately converting to int64.
(This isn't entirely waving a dead chicken, since on machines with
wider-than-double float registers, the extra store forces a width
conversion.  I don't know if S/390 is like that, but it seems worth
trying.)

In passing, merge duplicate ereport() calls in float8_timestamptz().

Per buildfarm.

Fix roundoff problems in float8_timestamptz() and make_interval().

When converting a float value to integer microseconds, we should be careful
to round the value to the nearest integer, typically with rint(); simply
assigning to an int64 variable will truncate, causing apparently off-by-one
values in cases that should work.  Most places in the datetime code got
this right, but not these two.

float8_timestamptz() is new as of commit e511d878f (9.6).  Previous
versions effectively depended on interval_mul() to do roundoff correctly,
which it does, so this fixes an accuracy regression in 9.6.

The problem in make_interval() dates to its introduction in 9.4.  Aside
from being careful to round not truncate, let's incorporate the hours and
minutes inputs into the result with exact integer arithmetic, rather than
risk introducing roundoff error where there need not have been any.

float8_timestamptz() problem reported by Erik Nordström, though this is
not his proposed patch.  make_interval() problem found by me.

Discussion: https://postgr.es/m/CAHuQZDS76jTYk3LydPbKpNfw9KbACmD=49dC4BrzHcfPv6yA1A@mail.gmail.com

Correct thinko in last-minute release note item.

The CREATE INDEX CONCURRENTLY bug can only be triggered by row updates,
not inserts, since the problem would arise from an update incorrectly
being made HOT.  Noted by Alvaro.

Stamp 9.5.6.

Release notes for 9.6.2, 9.5.6, 9.4.11, 9.3.16, 9.2.20.

Avoid returning stale attribute bitmaps in RelationGetIndexAttrBitmap().

The problem with the original coding here is that we might receive (and
clear) a relcache invalidation signal for the target relation down inside
one of the index_open calls we're doing.  Since the target is open, we
would not drop the relcache entry, just reset its rd_indexvalid and
rd_indexlist fields.  But RelationGetIndexAttrBitmap() kept going, and
would eventually cache and return potentially-obsolete attribute bitmaps.

The case where this matters is where the inval signal was from a CREATE
INDEX CONCURRENTLY telling us about a new index on a formerly-unindexed
column.  (In all other cases, the lock we hold on the target rel should
prevent any concurrent change in index state.)  Even just returning the
stale attribute bitmap is not such a problem, because it shouldn't matter
during the transaction in which we receive the signal.  What hurts is
caching the stale data, because it can survive into later transactions,
breaking CREATE INDEX CONCURRENTLY's expectation that later transactions
will not create new broken HOT chains.  The upshot is that there's a window
for building corrupted indexes during CREATE INDEX CONCURRENTLY.

This patch fixes the problem by rechecking that the set of index OIDs
is still the same at the end of RelationGetIndexAttrBitmap() as it was
at the start.  If not, we loop back and try again.  That's a little
more than is strictly necessary to fix the bug --- in principle, we
could return the stale data but not cache it --- but it seems like a
bad idea on general principles for relcache to return data it knows
is stale.

There might be more hazards of the same ilk, or there might be a better
way to fix this one, but this patch definitely improves matters and seems
unlikely to make anything worse.  So let's push it into today's releases
even as we continue to study the problem.

Pavan Deolasee and myself

Discussion: https://postgr.es/m/CABOikdM2MUq9cyZJi1KyLmmkCereyGp5JQ4fuwKoyKEde_mzkQ@mail.gmail.com

Translation updates

Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: e7df014526482b9ee2f736d01d09cf979a4e31e2

Add missing newline to error messages

Also improve the message style a bit while we're here.

Fix typo also in expected output.

Commit 181bdb90ba fixed the typo in the .sql file, but forgot to update the
expected output.

Fix typos in comments.

Backpatch to all supported versions, where applicable, to make backpatching
of future fixes go more smoothly.

Josh Soref

Discussion: https://www.postgresql.org/message-id/CACZqfqCf+5qRztLPgmmosr-B0Ye4srWzzw_mo4c_8_B_mtjmJQ@mail.gmail.com

Add KOI8-U map files to Makefile.

These were left out by mistake back when support for KOI8-U encoding was
added.

Extracted from Kyotaro Horiguchi's larger patch.

Update time zone data files to tzdata release 2016j.

DST law changes in northern Cyprus (new zone Asia/Famagusta), Russia (new
zone Europe/Saratov), Tonga, Antarctica/Casey.  Historical corrections for
Asia/Aqtau, Asia/Atyrau, Asia/Gaza, Asia/Hebron, Italy, Malta.  Replace
invented zone abbreviation "TOT" for Tonga with numeric UTC offset; but
as in the past, we'll keep accepting "TOT" for input.

Orthography fixes for new castNode() macro.

Clean up hastily-composed comment.  Normalize whitespace.

Erik Rijkers and myself

Check interrupts during hot standby waits

Add castNode(type, ptr) for safe casting between NodeTag based types.

The new function allows to cast from one NodeTag based type to
another, while asserting that the conversion is valid.  This replaces
the common pattern of doing a cast and a Assert(IsA(ptr, type))
close-by.

As this seems likely to be used pervasively, we decided to backpatch
this change the addition of this macro. Otherwise backpatched fixes
are more likely not to work on back-branches.

On branches before 9.6, where we do not yet rely on inline functions
being available, the type assertion is only performed if PG_USE_INLINE
support is detected. The cast obviously is performed regardless.

For the benefit of verifying the macro compiles in the back-branches,
this commit contains a single use of the new macro. On master, a
somewhat larger conversion will be committed separately.

Author: Peter Eisentraut and Andres Freund
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/c5d387d9-3440-f5e0-f9d4-71d53b9fbe52@2ndquadrant.com
Backpatch: 9.2-

Add missed update in expected file

Remove test for COMMENT ON DATABASE

Our current DDL only allows a database name to be specified in COMMENT
ON DATABASE, which Andrew Dunstan reports to make this test fail on the
buildfarm.  Remove the line until we gain a DDL command that allows the
current database to be operated on without having the specify it by
name.

Backpatch to 9.5, where these tests appeared.

Discussion: https://postgr.es/m/e6084b89-07a7-7e57-51ee-d7b8fc9ec864@2ndQuadrant.com

Reset hot standby xmin after restart

Hot_standby_feedback could be reset by reload and worked correctly, but if
the server was restarted rather than reloaded the xmin was not reset.
Force reset always if hot_standby_feedback is enabled at startup.

Ants Aasma, Craig Ringer

Reported-by: Ants Aasma

Ensure that a tsquery like '!foo' matches empty tsvectors.

!foo means "the tsvector does not contain foo", and therefore it should
match an empty tsvector.  ts_match_vq() overenthusiastically supposed
that an empty tsvector could never match any query, so it forcibly
returned FALSE, the wrong answer.  Remove the premature optimization.

Our behavior on this point was inconsistent, because while seqscans and
GIST index searches both failed to match empty tsvectors, GIN index
searches would find them, since GIN scans don't rely on ts_match_vq().
That makes this certainly a bug, not a debatable definition disagreement,
so back-patch to all supported branches.

Report and diagnosis by Tom Dunstan (bug #14515); added test cases by me.

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

Fix comments in StrategyNotifyBgWriter().

The interface for the function was changed in
d72731a70450b5e7084991b9caa15cb58a2820df but the comments of the
function was not updated.

Patch by Yugo Nagata.

doc: Update URL for Microsoft download site

Avoid useless respawining the autovacuum launcher at high speed.

When (1) autovacuum = off and (2) there's at least one database with
an XID age greater than autovacuum_freeze_max_age and (3) all tables
in that database that need vacuuming are already being processed by a
worker and (4) the autovacuum launcher is started, a kind of infinite
loop occurs.  The launcher starts a worker and immediately exits.  The
worker, finding no worker to do, immediately starts the launcher,
supposedly so that the next database can be processed.  But because
datfrozenxid for that database hasn't been advanced yet, the new
worker gets put right back into the same database as the old one,
where it once again starts the launcher and exits.  High-speed ping
pong ensues.

There are several possible ways to break the cycle; this seems like
the safest one.

Amit Khandekar (code) and Robert Haas (comments), reviewed by
Álvaro Herrera.

Discussion: http://postgr.es/m/CAJ3gD9eWejf72HKquKSzax0r+epS=nAbQKNnykkMA0E8c+rMDg@mail.gmail.com

Reset the proper GUC in create_index test.

Thinko in commit a4523c5aa.  It doesn't really affect anything at
present, but it would be a problem if any tests added later in this
file ought to get index-only-scan plans.  Back-patch, like the previous
commit, just to avoid surprises in case we add such a test and then
back-patch it.

Nikita Glukhov

Discussion: https://postgr.es/m/8b70135d-ad38-bdd8-ac92-71e2b3c273cf@postgrespro.ru

Change some test macros to return true booleans

These macros work fine when they are used directly in an "if" test or
similar, but as soon as the return values are assigned to boolean
variables (or passed as boolean arguments to some function), they become
bugs, hopefully caught by compiler warnings.  To avoid future problems,
fix the definitions so that they return actual booleans.

To further minimize the risk that somebody uses them in back-patched
fixes that only work correctly in branches starting from the current
master and not in old ones, back-patch the change to supported branches
as appropriate.

See also commit af4472bcb88ab36b9abbe7fd5858e570a65a2d1a, and the long
discussion (and larger patch) in the thread mentioned in its commit
message.

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

Disable transforms that replaced AT TIME ZONE with RelabelType.

These resulted in wrong answers if the relabeled argument could be matched
to an index column, as shown in bug #14504 from Evgeniy Kozlov.  We might
be able to resurrect these optimizations by adjusting the planner's
treatment of RelabelType, or by adjusting btree's rules for selecting
comparison functions, but either solution will take careful analysis
and does not sound like a fit candidate for backpatching.

I left the catalog infrastructure in place and just reduced the transform
functions to always-return-NULL.  This would be necessary anyway in the
back branches, and it doesn't seem important to be more invasive in HEAD.

Bug introduced by commit b8a18ad48.  Back-patch to 9.5 where that came in.

Report: https://postgr.es/m/20170118144828[email protected]
Discussion: https://postgr.es/m/18771.1484759439@sss.pgh.pa.us

Fix an assertion failure related to an exclusive backup.

Previously multiple sessions could execute pg_start_backup() and
pg_stop_backup() to start and stop an exclusive backup at the same time.
This could trigger the assertion failure of
"FailedAssertion("!(XLogCtl->Insert.exclusiveBackup)".
This happend because, even while pg_start_backup() was starting
an exclusive backup, other session could run pg_stop_backup()
concurrently and mark the backup as not-in-progress unconditionally.

This patch introduces ExclusiveBackupState indicating the state of
an exclusive backup. This state is used to ensure that there is only
one session running pg_start_backup() or pg_stop_backup() at
the same time, to avoid the assertion failure.

Back-patch to all supported versions.

Author: Michael Paquier
Reviewed-By: Kyotaro Horiguchi and me
Reported-By: Andreas Seltenreich
Discussion: <[email protected]>

Throw suitable error for COPY TO STDOUT/FROM STDIN in a SQL function.

A client copy can't work inside a function because the FE/BE wire protocol
doesn't support nesting of a COPY operation within query results.  (Maybe
it could, but the protocol spec doesn't suggest that clients should support
this, and libpq for one certainly doesn't.)

In most PLs, this prohibition is enforced by spi.c, but SQL functions don't
use SPI.  A comparison of _SPI_execute_plan() and init_execution_state()
shows that rejecting client COPY is the only discrepancy in what they
allow, so there's no other similar bugs.

This is an astonishingly ancient oversight, so back-patch to all supported
branches.

Report: https://postgr.es/m/BY2PR05MB2309EABA3DEFA0143F50F0D593780@BY2PR05MB2309.namprd05.prod.outlook.com

pg_upgrade: Fix for changed pg_ctl default stop mode

In 9.5, the default pg_ctl stop mode was changed from "smart" to "fast".
pg_upgrade still thought the default mode was "smart" and only specified
the mode when "fast" was asked for.  This results in using "fast" all
the time.  It's not clear what the effect in practice is, but fix it
nonetheless to restore the previous behavior.

pg_restore: Don't allow non-positive number of jobs

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

Fix invalid-parallel-jobs error message

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.

Fix ALTER TABLE / SET TYPE for irregular inheritance

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 9550e8348b79.  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[email protected]

BRIN revmap pages are not standard pages ...

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

Invalidate cached plans on FDW option changes.

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

Fix handling of empty arrays in array_fill().

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[email protected]

Handle OID column inheritance correctly in ALTER TABLE ... INHERIT.

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

Prefer int-wide pg_atomic_flag over char-wide when using gcc intrinsics.

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

Update copyright for 2017

Backpatch-through: certain files through 9.2

Remove bogus notice that older clients might not work with MD5 passwords.

That was written when we still had "crypt" authentication, and it was
referring to the fact that an older client might support "crypt"
authentication but not "md5". But we haven't supported "crypt" for years.
(As soon as we add a new authentication mechanism that doesn't work with
MD5 hashes, we'll need a similar notice again. But this text as it's worded
now is just wrong.)

Backpatch to all supported versions.

Discussion: https://www.postgresql.org/message-id/9a7263eb-0980-2072-4424-440bb2513dc7@iki.fi

Silence compiler warnings

Rearrange a bit of code to ensure that 'mode' in LWLockRelease is
obviously always set, which seems a bit cleaner and avoids a compiler
warning (thanks to Robert for the suggestion!).

Back-patch back to 9.5 where the warning is first seen.

Author: Stephen Frost
Discussion: https://postgr.es/m/20161129152102.GR13284%40tamriel.snowman.net

Silence compiler warnings

In GetCachedPlan(), initialize 'plan' to silence a compiler warning, but
also add an Assert() to make sure we don't ever actually fall through
with 'plan' still being set to NULL, since we are about to dereference
it.

Back-patch back to 9.2.

Author: Stephen Frost
Discussion: https://postgr.es/m/20161129152102.GR13284%40tamriel.snowman.net

Fix incorrect example of to_timestamp() usage.

Must use HH24 not HH to read a hour value exceeding 12.

This was already fixed in HEAD in commit d3cd36a13, but I didn't think
of backpatching it.

Report: https://postgr.es/m/20161229170043[email protected]

Fix interval_transform so it doesn't throw away non-no-op casts.

interval_transform() contained two separate bugs that caused it to
sometimes mistakenly decide that a cast from interval to restricted
interval is a no-op and throw it away.

First, it was wrong to rely on dt.h's field type macros to have an
ordering consistent with the field's significance; in one case they do
not.  This led to mistakenly treating YEAR as less significant than MONTH,
so that a cast from INTERVAL MONTH to INTERVAL YEAR was incorrectly
discarded.

Second, fls(1<<k) produces k+1 not k, so comparing its output directly
to SECOND was wrong.  This led to supposing that a cast to INTERVAL
MINUTE was really a cast to INTERVAL SECOND and so could be discarded.

To fix, get rid of the use of fls(), and make a function based on
intervaltypmodout to produce a field ID code adapted to the need here.

Per bug #14479 from Piotr Stefaniak.  Back-patch to 9.2 where transform
functions were introduced, because this code was born broken.

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

Explain unaccounted for space in pgstattuple.

In addition to space accounted for by tuple_len, dead_tuple_len and
free_space, the table_len includes page overhead, the item pointers
table and padding bytes.

Backpatch to live branches.

Remove triggerable Assert in hashname().

hashname() asserted that the key string it is given is shorter than
NAMEDATALEN.  That should surely always be true if the input is in fact a
regular value of type "name".  However, for reasons of coding convenience,
we allow plain old C strings to be treated as "name" values in many places.
Some SQL functions accept arbitrary "text" inputs, convert them to C
strings, and pass them otherwise-untransformed to syscache lookups for name
columns, allowing an overlength input value to trigger hashname's Assert.

This would be a DOS problem, except that it only happens in assert-enabled
builds which aren't recommended for production.  In a production build,
you'll just get a name lookup error, since regardless of the hash value
computed by hashname, the later equality comparison checks can't match.
Likewise, if the catalog lookup is done by seqscan or indexscan searches,
there will just be a lookup error, since the name comparison functions
don't contain any similar length checks, and will see an overlength input
as unequal to any stored entry.

After discussion we concluded that we should simply remove this Assert.
It's inessential to hashname's own functionality, and having such an
assertion in only some paths for name lookup is more of a foot-gun than
a useful check.  There may or may not be a case for the affected callers
to do something other than let the name lookup fail, but we'll consider
that separately; in any case we probably don't want to change such
behavior in the back branches.

Per report from Tushar Ahuja.  Back-patch to all supported branches.

Report: https://postgr.es/m/7d0809ee-6f25-c9d6-8e74-5b2967830d49@enterprisedb.com
Discussion: https://postgr.es/m/17691.1482523168@sss.pgh.pa.us

pg_dumpall: Include --verbose option in --help output

The -v/--verbose option was not included in the output from --help for
pg_dumpall even though it's in the pg_dumpall documentation and has
apparently been around since pg_dumpall was reimplemented in C in 2002.

Fix that by adding it.

Pointed out by Daniel Westermann.

Back-patch to all supported branches.

Discussion: https://www.postgresql.org/message-id/2020970042.4589542.1482482101585.JavaMail.zimbra%40dbi-services.com

Fix tab completion in psql for ALTER DEFAULT PRIVILEGES

When providing tab completion for ALTER DEFAULT PRIVILEGES, we are
including the list of roles as possible options for completion after the
GRANT or REVOKE.  Further, we accept FOR ROLE/IN SCHEMA at the same time
and in either order, but the tab completion was only working for one or
the other.  Lastly, we weren't using the actual list of allowed kinds of
objects for default privileges for completion after the 'GRANT X ON' but
instead were completeing to what 'GRANT X ON' supports, which isn't the
ssame at all.

Address these issues by improving the forward tab-completion for ALTER
DEFAULT PRIVILEGES and then constrain and correct how the tail
completion is done when it is for ALTER DEFAULT PRIVILEGES.

Back-patch the forward/tail tab-completion to 9.6, where we made it easy
to handle such cases.

For 9.5 and earlier, correct the initial tab-completion to at least be
correct as far as it goes and then add a check for GRANT/REVOKE to only
tab-complete when the GRANT/REVOKE is the start of the command, so we
don't try to do tab-completion after we get to the GRANT/REVOKE part of
the ALTER DEFAULT PRIVILEGES command, which is better than providing
incorrect completions.

Initial patch for master and 9.6 by Gilles Darold, though I cleaned it
up and added a few comments.  All bugs in the 9.5 and earlier patch are
mine.

Discussion: https://www.postgresql.org/message-id/1614593c-e356-5b27-6dba-66320a9bc68b@dalibo.com

Doc: improve index entry for "median".

We had an index entry for "median" attached to the percentile_cont function
entry, which was pretty useless because a person following the link would
never realize that that function was the one they were being hinted to use.

Instead, make the index entry point at the example in syntax-aggregates,
and add a <seealso> link to "percentile".

Also, since that example explicitly claims to be calculating the median,
make it use percentile_cont not percentile_disc.  This makes no difference
in terms of the larger goals of that section, but so far as I can find,
nearly everyone thinks that "median" means the continuous not discrete
calculation.

Per gripe from Steven Winfield.  Back-patch to 9.4 where we introduced
percentile_cont.

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

Improve RLS documentation with respect to COPY

Documentation for pg_restore said COPY TO does not support row security
when in fact it should say COPY FROM. Fix that.

While at it, make it clear that "COPY FROM" does not allow RLS to be
enabled and INSERT should be used instead. Also that SELECT policies
will apply to COPY TO statements.

Back-patch to 9.5 where RLS first appeared.

Author: Joe Conway
Reviewed-By: Dean Rasheed and Robert Haas
Discussion: https://postgr.es/m/5744FA24.3030008%40joeconway.com

Use TSConfigRelationId in AlterTSConfiguration()

When we are altering a text search configuration, we are getting the
tuple from pg_ts_config and using its OID, so use TSConfigRelationId
when invoking any post-alter hooks and setting the object address.

Further, in the functions called from AlterTSConfiguration(), we're
saving information about the command via
EventTriggerCollectAlterTSConfig(), so we should be setting
commandCollected to true.  Also add a regression test to
test_ddl_deparse for ALTER TEXT SEARCH CONFIGURATION.

Author: Artur Zakirov, a few additional comments by me
Discussion: https://www.postgresql.org/message-id/57a71eba-f2c7-e7fd-6fc0-2126ec0b39bd%40postgrespro.ru

Back-patch the fix for the InvokeObjectPostAlterHook() call to 9.3 where
it was introduced, and the fix for the ObjectAddressSet() call and
setting commandCollected to true to 9.5 where those changes to
ProcessUtilitySlow() were introduced.

Fix handling of expanded objects in CoerceToDomain and CASE execution.

When the input value to a CoerceToDomain expression node is a read-write
expanded datum, we should pass a read-only pointer to any domain CHECK
expressions and then return the original read-write pointer as the
expression result.  Previously we were blindly passing the same pointer to
all the consumers of the value, making it possible for a function in CHECK
to modify or even delete the expanded value.  (Since a plpgsql function
will absorb a passed-in read-write expanded array as a local variable
value, it will in fact delete the value on exit.)

A similar hazard of passing the same read-write pointer to multiple
consumers exists in domain_check() and in ExecEvalCase, so fix those too.

The fix requires adding MakeExpandedObjectReadOnly calls at the appropriate
places, which is simple enough except that we need to get the data type's
typlen from somewhere.  For the domain cases, solve this by redefining
DomainConstraintRef.tcache as okay for callers to access; there wasn't any
reason for the original convention against that, other than not wanting the
API of typcache.c to be any wider than it had to be.  For CASE, there's
no good solution except to add a syscache lookup during executor start.

Per bug #14472 from Marcos Castedo.  Back-patch to 9.5 where expanded
values were introduced.

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

Fix broken error check in _hash_doinsert.

You can't just cast a HashMetaPage to a Page, because the meta page
data is stored after the page header, not at offset 0.  Fortunately,
this didn't break anything because it happens to find hashm_bsize
at the offset at which it expects to find pd_pagesize_version, and
the values are close enough to the same that this works out.

Still, it's a bug, so back-patch to all supported versions.

Mithun Cy, revised a bit by me.

Make dblink try harder to form useful error messages

When libpq encounters a connection-level error, e.g. runs out of memory
while forming a result, there will be no error associated with PGresult,
but a message will be placed into PGconn's error buffer. postgres_fdw
takes care to use the PGconn error message when PGresult does not have
one, but dblink has been negligent in that regard. Modify dblink to mirror
what postgres_fdw has been doing.

Back-patch to all supported branches.

Author: Joe Conway
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/02fa2d90-2efd-00bc-fefc-c23c00eb671e%40joeconway.com

Protect dblink from invalid options when using postgres_fdw server

When dblink uses a postgres_fdw server name for its connection, it
is possible for the connection to have options that are invalid
with dblink (e.g. "updatable"). The recommended way to avoid this
problem is to use dblink_fdw servers instead. However there are use
cases for using postgres_fdw, and possibly other FDWs, for dblink
connection options, therefore protect against trying to use any
options that do not apply by using is_valid_dblink_option() when
building the connection string from the options.

Back-patch to 9.3. Although 9.2 supports FDWs for connection info,
is_valid_dblink_option() did not yet exist, and neither did
postgres_fdw, at least in the postgres source tree. Given the lack
of previous complaints, fixing that seems too invasive/not worth it.

Author: Corey Huinker
Reviewed-By: Joe Conway
Discussion: https://postgr.es/m/CADkLM%3DfWyXVEyYcqbcRnxcHutkP45UHU9WD7XpdZaMfe7S%3DRwA%40mail.gmail.com

Give a useful error message if uuid-ossp is built without preconfiguration.

Before commit b8cc8f947, it was possible to build contrib/uuid-ossp without
having told configure you meant to; you could just cd into that directory
and "make".  That no longer works because the code depends on configure to
have done header and library probes, but the ensuing error messages are
not so easy to interpret if you're not an old C hand.  We've gotten a
couple of complaints recently from people trying to do this the low-tech
way, so add an explicit #error directing the user to use --with-uuid.

(In principle we might want to do something similar in the other
optionally-built contrib modules; but I don't think any of the others have
ever worked without preconfiguration, so there are no bad habits to break
people of.)

Back-patch to 9.4 where the previous commit came in.

Report: https://postgr.es/m/CAHeEsBf42AWTnk=1qJvFv+mYgRFm07Knsfuc86Ono8nRjf3tvQ@mail.gmail.com
Report: https://postgr.es/m/CAKYdkBrUaZX+F6KpmzoHqMtiUqCtAW_w6Dgvr6F0WTiopuGxow@mail.gmail.com

Fix buffer overflow on particularly named files and clarify documentation about
output file naming.

Patch by Tsunakawa, Takayuki <[email protected]>

Improve dblink error message when remote does not provide it

When dblink or postgres_fdw detects an error on the remote side of the
connection, it will try to construct a local error message as best it
can using libpq's PQresultErrorField(). When no primary message is
available, it was bailing out with an unhelpful "unknown error". Make
that message better and more style guide compliant. Per discussion
on hackers.

Backpatch to 9.2 except postgres_fdw which didn't exist before 9.3.

Discussion: https://postgr.es/m/19872.1482338965%40sss.pgh.pa.us

Fix detection of unfinished Unicode surrogate pair at end of string.

The U&'...' and U&"..." syntaxes silently discarded a surrogate pair
start (that is, a code between U+D800 and U+DBFF) if it occurred at
the very end of the string.  This seems like an obvious oversight,
since we throw an error for every other invalid combination of surrogate
characters, including the very same situation in E'...' syntax.

This has been wrong since the pair processing was added (in 9.0),
so back-patch to all supported branches.

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

Improve ALTER TABLE documentation

The ALTER TABLE documentation wasn't terribly clear when it came to
which commands could be combined together and what it meant when they
were.

In particular, SET TABLESPACE *can* be combined with other commands,
when it's operating against a single table, but not when multiple tables
are being moved with ALL IN TABLESPACE.  Further, the actions are
applied together but not really in 'parallel', at least today.

Pointed out by: Amit Langote

Improved wording from Tom.

Back-patch to 9.4, where the ALL IN TABLESPACE option was added.

Discussion: https://www.postgresql.org/message-id/14c535b4-13ef-0590-1b98-76af355a0763%40lab.ntt.co.jp

Fix dumping of casts and transforms using built-in functions

In pg_dump.c dumpCast() and dumpTransform(), we would happily ignore the
cast or transform if it happened to use a built-in function because we
weren't including the information about built-in functions when querying
pg_proc from getFuncs().

Modify the query in getFuncs() to also gather information about
functions which are used by user-defined casts and transforms (where
"user-defined" means "has an OID >= FirstNormalObjectId").  This also
adds to the TAP regression tests for 9.6 and master to cover these
types of objects.

Back-patch all the way for casts, back to 9.5 for transforms.

Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net

For 8.0 servers, get last built-in oid from pg_database

We didn't start ensuring that all built-in objects had OIDs less than
16384 until 8.1, so for 8.0 servers we still need to query the value out
of pg_database.  We need this, in particular, to distinguish which casts
were built-in and which were user-defined.

For HEAD, we only worry about going back to 8.0, for the back-branches,
we also ensure that 7.0-7.4 work.

Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net

Fix order of operations in CREATE OR REPLACE VIEW.

When CREATE OR REPLACE VIEW acts on an existing view, don't update the
view options until after the view query has been updated.

This is necessary in the case where CREATE OR REPLACE VIEW is used on
an existing view that is not updatable, and the new view is updatable
and specifies the WITH CHECK OPTION. In this case, attempting to apply
the new options to the view before updating its query fails, because
the options are applied using the ALTER TABLE infrastructure which
checks that WITH CHECK OPTION is only applied to an updatable view.

If new columns are being added to the view, that is also done using
the ALTER TABLE infrastructure, but it is important that that still be
done before updating the view query, because the rules system checks
that the query columns match those on the view relation. Added a
comment to explain that, in case someone is tempted to move that to
where the view options are now being set.

Back-patch to 9.4 where WITH CHECK OPTION was added.

Report: https://postgr.es/m/CAEZATCUp%3Dz%3Ds4SzZjr14bfct_bdJNwMPi-gFi3Xc5k1ntbsAgQ%40mail.gmail.com