During pg_upgrade's restore run, all relfilenode choices should be
overridden by commands in the dump script.  If we ever find ourselves
choosing a relfilenode in the ordinary way, someone blew it.  Likewise for
pg_type OIDs.  Since pg_upgrade might well succeed anyway, if there happens
not to be a conflict during the regression test run, we need assertions
here to keep us on the straight and narrow.

We might someday be able to remove the assertion in GetNewRelFileNode,
if pg_upgrade is rewritten to remove its assumption that old and new
relfilenodes always match.  But it's hard to see how to get rid of the
pg_type OID constraint, since those OIDs are embedded in user tables
in some cases.

Back-patch as far as 9.5, because of the risk of back-patches breaking
something here even if it works in HEAD.  I'd prefer to go back further,
but 9.4 fails both assertions due to get_rel_infos()'s use of a temporary
table.  We can't use the later-branch solution of a CTE for compatibility
reasons (cf commit 5d16332e), and it doesn't seem worth inventing some
other way to do the query.  (I did check, by dint of changing the Asserts
to elog(WARNING), that there are no other cases of unwanted OID assignments
during 9.4's regression test run.)

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

It's not necessary for it to do that, since OWNED BY requires only ordinary
catalog updates and doesn't affect future sequence values.  And pg_upgrade
needs to use OWNED BY without having it change the sequence's relfilenode.
Commit 3d79013b broke this by making all forms of ALTER SEQUENCE change
the relfilenode; that seems to be the explanation for the hard-to-reproduce
buildfarm failures we've been seeing since then.

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

This was apparently forgotten in the original patch.

The original code only added ICU_CFLAGS to the backend build.  But it is
also needed for building external modules that include pg_locale.h.  So
add it to the global CPPFLAGS.  (This is only relevant if ICU is not in
a compiler default path, so it apparently hasn't bitten many.)

It appears to be more confusing than useful.
Reported-by: Jeff Janes <jeff.janes@gmail.com>

Author: Ashutosh Sharma <ashu.coek88@gmail.com>
Reviewed-by: Michael Paquier <michael.paquier@gmail.com>

and also any platform that does not have locale_t but enabled ICU.

Author: Ashutosh Sharma <ashu.coek88@gmail.com>

When a table sync worker is in waiting state and the subscription table
entry is removed because of a concurrent subscription refresh, the
worker could be left orphaned.  To avoid that, explicitly stop the
worker when the pg_subscription_rel entry is removed.
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>

Patch by Yugo Nagata <nagata@sraoss.co.jp>.  Confirmed by Amit
Langote, who is the original author of the document part.

generateSerialExtraStmts() was sloppy about handling the case where
SEQUENCE NAME is given with a not-schema-qualified name.  It was generating
a CreateSeqStmt with an unqualified sequence name, and an AlterSeqStmt
whose "owned_by" DefElem contained a T_String Value with a null string
pointer in the schema-name position.  The generated nextval() argument was
also underqualified.  This accidentally failed to fail at runtime, but only
so long as the current default creation namespace at runtime is the right
namespace.  That's bogus; the parse-time transformation is supposed to be
inserting the right schema name in all cases, so as to avoid any possible
skew in that selection.  I'm not sure this could fail in pg_dump's usage,
but it's still wrong; we have had real bugs in this area before adopting
the policy that parse_utilcmd.c should generate only fully-qualified
auxiliary commands.  A slightly lesser problem, which is what led me to
notice this in the first place, is that pprint() dumped core on the
AlterSeqStmt because of the bogus T_String.

Noted while poking into the open problem with ALTER SEQUENCE breaking
pg_upgrade.

The new partitioned table capability added a new relkind, namely
RELKIND_PARTITIONED_TABLE. Update fireRIRrules() to apply RLS
policies on RELKIND_PARTITIONED_TABLE as it does RELKIND_RELATION.

In addition, add RLS regression test coverage for partitioned tables.

Issue raised by Fakhroutdinov Evgenievich and patch by Mike Palmiotto.
Regression test editorializing by me.

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

This reverts commit 56b6ef89 and instead
makes vcregress.pl parse out PROVE_FLAGS from a command line argument
when doing a TAP test, thus making it consistent with the makefile
treatment.

Discussion: https://postgr.es/m/c26a7416-2fb9-34ab-7991-618c922f896e%402ndquadrant.com

Backpatch to 9.4 like previous patch.

Also, fix client interface JDBC language name to Java.

Author: Sehrope Sarkuni <sehrope@jackdb.com>

Reported-by: Nikolaus Thiel <klt@fsfe.org>

If the compiler doesn't notice that the switch-statement handles all
possible values of the enum, it might complain that 'ret' is being used
without initialization. Jeff Janes reported that on gcc 4.4.7.

Discussion: https://www.postgresql.org/message-id/CAMkU=1x31RvP+cpooFbmc8K8nt-gNO8woGFhXcgQYYZ5ozYpFA@mail.gmail.com

Author: Neha Khatri <nehakhatri5@gmail.com>

Author: Masahiko Sawada <sawada.mshk@gmail.com>

Author: Masahiko Sawada <sawada.mshk@gmail.com>

When a table is removed from a subscription before the tablesync worker
could start, this would previously result in an error when reading
pg_subscription_rel.  Now we just ignore this.

Author: Masahiko Sawada <sawada.mshk@gmail.com>

If you accidentally pass a host name in the hostaddr option, e.g.
hostaddr=localhost, you get an error like:

psql: could not translate host name "localhost" to address: Name or service not known

That's a bit confusing, because it implies that we tried to look up
"localhost" in DNS, but it failed. To make it more clear that we tried to
parse "localhost" as a numeric network address, change the message to:

psql: could not parse network address "localhost": Name or service not known

Discussion: https://www.postgresql.org/message-id/10badbc6-4d5a-a769-623a-f7ada43e14dd@iki.fi

The script was rewritten in Perl, and renamed from regress.sh to regress.pl,
back in 2012.

Previously the exit handling was only able to exit from within the
main loop, and not from within the backend code it calls.  Fix that by
using the standard die() SIGTERM handler, and adding the necessary
CHECK_FOR_INTERRUPTS() call.

This requires adding yet another process-type-specific branch to
ProcessInterrupts(), which hints that we probably should generalize
that handling.  But that's work for another day.

Author: Petr Jelinek
Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/fe072153-babd-3b5d-8052-73527a6eb657@2ndquadrant.com

Since 7c4f5240 (merged in v10), a shutdown master is reported as
  FATAL:  unexpected result after CommandComplete: server closed the connection unexpectedly
by walsender. It used to be
  LOG:  replication terminated by primary server
  FATAL:  could not send end-of-streaming message to primary: no COPY in progress
while the old message clearly is not perfect, it's definitely better
than what's reported now.

The change comes from the attempt to handle finished COPYs without
erroring out, needed for the new logical replication, which wasn't
needed before.

There's probably better ways to handle this, but for now just
explicitly check for a closed connection.

Author: Petr Jelinek
Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/f7c7dd08-855c-e4ed-41f4-d064a6c0665a@2ndquadrant.com
Backpatch: -

This make them consistent with the text function and means they can be
used in functional indexes.

Catalog version bumped.

Per gripe from Josh Berkus.

Doing a cross-version upgrade test with test.sh evidently hasn't been
tested since circa 9.2, because the script lacked case branches for
old-version servers newer than 9.1.  Future-proof that a bit, and
clean up breakage induced by our recent drop of V0 function call
protocol (namely that oldstyle_length() isn't in the regression
suite anymore).

(This isn't enough to make the test work perfectly cleanly across
versions, but at least it finishes and provides dump files that
you can diff manually.  One issue I didn't touch is that we might
want to execute the "reindex_hash.sql" file in the new DB before
dumping it, so that the hash indexes don't vanish from the dump.)

Improve the TESTING doc file: put the tl;dr version at the top not
the bottom, and bring its explanation of how to run a cross-version
test up to speed, since the installcheck target isn't there and won't
be resurrected.  Improve the comment in the Makefile about why not.

In passing, teach .gitignore and "make clean" about a couple more
junk output files.

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

Most of the improvements were in the new SCRAM code:

* In SCRAM protocol violation messages, use errdetail to provide the
  details.

* If pg_backend_random() fails, throw an ERROR rather than just LOG. We
  shouldn't continue authentication if we can't generate a random nonce.

* Use ereport() rather than elog() for the "invalid SCRAM verifier"
  messages. They shouldn't happen, if everything works, but it's not
  inconceivable that someone would have invalid scram verifiers in
  pg_authid, e.g. if a broken client application was used to generate the
  verifier.

But this change applied to old code:

* Use ERROR rather than COMMERROR for protocol violation errors. There's
  no reason to not tell the client what they did wrong. The client might be
  confused already, so that it cannot read and display the error correctly,
  but let's at least try. In the "invalid password packet size" case, we
  used to actually continue with authentication anyway, but that is now a
  hard error.

Patch by Michael Paquier and me. Thanks to Daniel Varrazzo for spotting
the typo in one of the messages that spurred the discussion and these
larger changes.

Discussion: https://www.postgresql.org/message-id/CA%2Bmi_8aZYLhuyQi1Jo0hO19opNZ2OEATEOM5fKApH7P6zTOZGg%40mail.gmail.com

Commit 7b504eb2 overlooked this.

Report and patch by Kyotaro Horiguchi

Discussion: http://postgr.es/m/20170608.145852.54673832.horiguchi.kyotaro@lab.ntt.co.jp

Commit 15ce775f changed tuple-routing constraint checking logic.
This affects the expected output for contrib/sepgsql, because
there's no longer LOG entries reporting allowance of int4eq()
execution. Per buildfarm.

Clarify in the syntax synopsis that partition bound values must be
exactly numeric literals or string literals; previously it
said "bound_literal" which was defined nowhere.

Replace confusing --- and, I think, incorrect in detail --- definition
of how range bounds work with a reference to row-wise comparison plus
a concrete example (which I stole from Robert Haas).

Minor copy-editing in the same area.

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

Commit f039eaac, later back-patched
with commit 1b812afb, allowed many of
the queries issued by postgres_fdw to fetch remote data to respond to
cancel interrupts in a timely fashion.  However, it didn't do anything
about the transaction control commands, which remained
noninterruptible.

Improve the situation by changing do_sql_command() to retrieve query
results using pgfdw_get_result(), which uses the asynchronous
interface to libpq so that it can check for interrupts every time
libpq returns control.  Since this might result in a situation
where we can no longer be sure that the remote transaction state
matches the local transaction state, add a facility to force all
levels of the local transaction to abort if we've lost track of
the remote state; without this, an apparently-successful commit of
the local transaction might fail to commit changes made on the
remote side.  Also, add a 60-second timeout for queries issue during
transaction abort; if that expires, give up and mark the state of
the connection as unknown.  Drop all such connections when we exit
the local transaction.  Together, these changes mean that if we're
aborting the local toplevel transaction anyway, we can just drop the
remote connection in lieu of waiting (possibly for a very long time)
for it to complete an abort.

This still leaves quite a bit of room for improvement.  PQcancel()
has no asynchronous interface, so if we get stuck sending the cancel
request we'll still hang.  Also, PQsetnonblocking() is not used, which
means we could block uninterruptibly when sending a query.  There
might be some other optimizations possible as well.  Nonetheless,
this allows us to escape a wait for an unresponsive remote server
quickly in many more cases than previously.

Report by Suraj Kharage.  Patch by me and Rafia Sabih.  Review
and testing by Amit Kapila and Tushar Ahuja.

Discussion: http://postgr.es/m/CAF1DzPU8Kx+fMXEbFoP289xtm3bz3t+ZfxhmKavr98Bh-C0TqQ@mail.gmail.com

A logical replication worker should not insert new rows into
pg_subscription_rel, only update existing rows, so that there are no
races if a concurrent refresh removes rows.  Adjust the API to be able
to choose that behavior.

Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reported-by: tushar <tushar.ahuja@enterprisedb.com>

Since tuple-routing implicitly checks the partitioning constraints
at least for the levels of the partitioning hierarchy it traverses,
there's normally no need to revalidate the partitioning constraint
after performing tuple routing.  However, if there's a BEFORE trigger
on the target partition, it could modify the tuple, causing the
partitioning constraint to be violated.  Catch that case.

Also, instead of checking the root table's partition constraint after
tuple-routing, check it beforehand.  Otherwise, the rules for when
the partitioning constraint gets checked get too complicated, because
you sometimes have to check part of the constraint but not all of it.
This effectively reverts commit 39162b20
in favor of a different approach altogether.

Report by me.  Initial debugging by Jeevan Ladhe.  Patch by Amit
Langote, reviewed by me.

Discussion: http://postgr.es/m/CA+Tgmoa9DTgeVOqopieV8d1QRpddmP65aCdxyjdYDoEO5pS5KA@mail.gmail.com

If authentication over an SSL connection fails, with sslmode=prefer,
libpq will reconnect without SSL and retry. However, we did not clear
the variables related to GSS, SSPI, and SASL authentication state, when
reconnecting. Because of that, the second authentication attempt would
always fail with a "duplicate GSS/SASL authentication request" error.
pg_SSPI_startup did not check for duplicate authentication requests like
the corresponding GSS and SASL functions, so with SSPI, you would leak
some memory instead.

Another way this could manifest itself, on version 10, is if you list
multiple hostnames in the "host" parameter. If the first server requests
Kerberos or SCRAM authentication, but it fails, the attempts to connect to
the other servers will also fail with "duplicate authentication request"
errors.

To fix, move the clearing of authentication state from closePGconn to
pgDropConnection, so that it is cleared also when re-connecting.

Patch by Michael Paquier, with some kibitzing by me.

Backpatch down to 9.3. 9.2 has the same bug, but the code around closing
the connection is somewhat different, so that this patch doesn't apply.
To fix this in 9.2, I think we would need to back-port commit 210eb9b7
first, and then apply this patch. However, given that we only bumped into
this in our own testing, we haven't heard any reports from users about
this, and that 9.2 will be end-of-lifed in a couple of months anyway, it
doesn't seem worth the risk and trouble.

Discussion: https://www.postgresql.org/message-id/CAB7nPqRuOUm0MyJaUy9L3eXYJU3AKCZ-0-03=-aDTZJGV4GyWw@mail.gmail.com

The logic to free the buffer after the gss_init_sec_context() call was
always a bit wonky. Because gss_init_sec_context() sets the GSS context
variable, conn->gctx, we would in fact always attempt to free the buffer.
That only works, because previously conn->ginbuf.value was initialized to
NULL, and free(NULL) is a no-op. Commit 61bf96ca refactored things so
that the GSS input token buffer is allocated locally in pg_GSS_continue,
and not held in the PGconn object. After that, the now-local ginbuf.value
variable isn't initialized when it's not used, so we pass a bogus pointer
to free().

To fix, only try to free the input buffer if we allocated it. That was the
intention, certainly after the refactoring, and probably even before that.
But because there's no live bug before the refactoring, I refrained from
backpatching this.

The bug was also independently reported by Graham Dutton, as bug #14690.
Patch reviewed by Michael Paquier.

Discussion: https://www.postgresql.org/message-id/6288d80e-a0bf-d4d3-4e12-7b79c77f1771%40iki.fi
Discussion: https://www.postgresql.org/message-id/20170605130954.1438.90535%40wrigleys.postgresql.org

The logical replication apply worker uses the subscription name as
application name, except for table sync.  This was incorrectly set to
use the replication slot name, which might be different, in one case.
Also add a comment why the other case is different.