postgresql.git
6 years agodocs: clarify plpython SD and GD dictionary behavior
Bruce Momjian [Sat, 25 Aug 2018 15:52:29 +0000 (11:52 -0400)]
docs:  clarify plpython SD and GD dictionary behavior

Reported-by: Adam Bielański
Discussion: https://postgr.es/m/153484305538.1370.7605856225879294548@wrigleys.postgresql.org

Backpatch-through: 9.3

6 years agoFix lexing of standard multi-character operators in edge cases.
Andrew Gierth [Thu, 23 Aug 2018 17:29:18 +0000 (18:29 +0100)]
Fix lexing of standard multi-character operators in edge cases.

Commits c6b3c939b (which fixed the precedence of >=, <=, <> operators)
and 865f14a2d (which added support for the standard => notation for
named arguments) created a class of lexer tokens which look like
multi-character operators but which have their own token IDs distinct
from Op. However, longest-match rules meant that following any of
these tokens with another operator character, as in (1<>-1), would
cause them to be incorrectly returned as Op.

The error here isn't immediately obvious, because the parser would
usually still find the correct operator via the Op token, but there
were more subtle problems:

1. If immediately followed by a comment or +-, >= <= <> would be given
   the old precedence of Op rather than the correct new precedence;

2. If followed by a comment, != would be returned as Op rather than as
   NOT_EQUAL, causing it not to be found at all;

3. If followed by a comment or +-, the => token for named arguments
   would be lexed as Op, causing the argument to be mis-parsed as a
   simple expression, usually causing an error.

Fix by explicitly checking for the operators in the {operator} code
block in addition to all the existing special cases there.

Backpatch to 9.5 where the problem was introduced.

Analysis and patch by me; review by Tom Lane.
Discussion: https://postgr.es/m/[email protected]

6 years agoReduce an unnecessary O(N^3) loop in lexer.
Andrew Gierth [Thu, 23 Aug 2018 19:00:50 +0000 (20:00 +0100)]
Reduce an unnecessary O(N^3) loop in lexer.

The lexer's handling of operators contained an O(N^3) hazard when
dealing with long strings of + or - characters; it seems hard to
prevent this case from being O(N^2), but the additional N multiplier
was not needed.

Backpatch all the way since this has been there since 7.x, and it
presents at least a mild hazard in that trying to do Bind, PREPARE or
EXPLAIN on a hostile query could take excessive time (without
honouring cancels or timeouts) even if the query was never executed.

6 years agoIn libpq, don't look up all the hostnames at once.
Tom Lane [Thu, 23 Aug 2018 20:39:20 +0000 (16:39 -0400)]
In libpq, don't look up all the hostnames at once.

Historically, we looked up the target hostname in connectDBStart, so that
PQconnectPoll did not need to do DNS name resolution.  The patches that
added multiple-target-host support to libpq preserved this division of
labor; but it's really nonsensical now, because it means that if any one
of the target hosts fails to resolve in DNS, the connection fails.  That
negates the no-single-point-of-failure goal of the feature.  Additionally,
DNS lookups aren't exactly cheap, but the code did them all even if the
first connection attempt succeeds.

Hence, rearrange so that PQconnectPoll does the lookups, and only looks
up a hostname when it's time to try that host.  This does mean that
PQconnectPoll could block on a DNS lookup --- but if you wanted to avoid
that, you should be using hostaddr, as the documentation has always
specified.  It seems fairly unlikely that any applications would really
care whether the lookup occurs inside PQconnectStart or PQconnectPoll.

In addition to calling out that fact explicitly, do some other minor
wordsmithing in the docs around the multiple-target-host feature.

Since this seems like a bug in the multiple-target-host feature,
backpatch to v10 where that was introduced.  In the back branches,
avoid moving any existing fields of struct pg_conn, just in case
any third-party code is looking into that struct.

Tom Lane, reviewed by Fabien Coelho

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

6 years agoReturn type of txid_status is text, not txid_status
Alvaro Herrera [Thu, 23 Aug 2018 14:40:30 +0000 (11:40 -0300)]
Return type of txid_status is text, not txid_status

Thinko in commit 857ee8e39.

Discovered-by: Gianni Ciolli
6 years agoDo not dump identity sequences with excluded parent table
Michael Paquier [Wed, 22 Aug 2018 05:23:03 +0000 (14:23 +0900)]
Do not dump identity sequences with excluded parent table

This commit prevents a crash of pg_dump caused by the exclusion of a
table which has identity columns, as the table would be correctly
excluded but not its identity sequence.  In order to fix that, identity
sequences are excluded if the parent table is defined as such.  Knowing
about such sequences has no meaning without their parent table anyway.

Reported-by: Andy Abelisto
Author: David Rowley
Reviewed-by: Peter Eisentraut, Michael Paquier
Discussion: https://postgr.es/m/153479393218.1316.8472285660264976457@wrigleys.postgresql.org
Backpatch-through: 10

6 years agoFix typo
Alvaro Herrera [Tue, 21 Aug 2018 20:16:10 +0000 (17:16 -0300)]
Fix typo

6 years agofix typo
Alvaro Herrera [Tue, 21 Aug 2018 20:03:35 +0000 (17:03 -0300)]
fix typo

6 years agoFix set of NLS translation issues
Michael Paquier [Tue, 21 Aug 2018 06:17:38 +0000 (15:17 +0900)]
Fix set of NLS translation issues

While monitoring the code, a couple of issues related to string
translation has showed up:
- Some routines for auto-updatable views return an error string, which
sometimes missed the shot.  A comment regarding string translation is
added for each routine to help with future features.
- GSSAPI authentication missed two translations.
- vacuumdb handles non-translated strings.

Reported-by: Kyotaro Horiguchi
Author: Kyotaro Horiguchi
Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/20180810.152131.31921918[email protected]
Backpatch-through: 9.3

6 years agoEnsure schema qualification in pg_restore DISABLE/ENABLE TRIGGER commands.
Tom Lane [Fri, 17 Aug 2018 21:12:21 +0000 (17:12 -0400)]
Ensure schema qualification in pg_restore DISABLE/ENABLE TRIGGER commands.

Previously, this code blindly followed the common coding pattern of
passing PQserverVersion(AH->connection) as the server-version parameter
of fmtQualifiedId.  That works as long as we have a connection; but in
pg_restore with text output, we don't.  Instead we got a zero from
PQserverVersion, which fmtQualifiedId interpreted as "server is too old to
have schemas", and so the name went unqualified.  That still accidentally
managed to work in many cases, which is probably why this ancient bug went
undetected for so long.  It only became obvious in the wake of the changes
to force dump/restore to execute with restricted search_path.

In HEAD/v11, let's deal with this by ripping out fmtQualifiedId's server-
version behavioral dependency, and just making it schema-qualify all the
time.  We no longer support pg_dump from servers old enough to need the
ability to omit schema name, let alone restoring to them.  (Also, the few
callers outside pg_dump already didn't work with pre-schema servers.)

In older branches, that's not an acceptable solution, so instead just
tweak the DISABLE/ENABLE TRIGGER logic to ensure it will schema-qualify
its output regardless of server version.

Per bug #15338 from Oleg somebody.  Back-patch to all supported branches.

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

6 years agoSet scan direction appropriately for SubPlans (bug #15336)
Andrew Gierth [Fri, 17 Aug 2018 14:04:26 +0000 (15:04 +0100)]
Set scan direction appropriately for SubPlans (bug #15336)

When executing a SubPlan in an expression, the EState's direction
field was left alone, resulting in an attempt to execute the subplan
backwards if it was encountered during a backwards scan of a cursor.
Also, though much less likely, it was possible to reach the execution
of an InitPlan while in backwards-scan state.

Repair by saving/restoring estate->es_direction and forcing forward
scan mode in the relevant places.

Backpatch all the way, since this has been broken since 8.3 (prior to
commit c7ff7663e, SubPlans had their own EStates rather than sharing
the parent plan's, so there was no confusion over scan direction).

Per bug #15336 reported by Vladimir Baranoff; analysis and patch by
me, review by Tom Lane.

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

6 years agopg_upgrade: issue helpful error message for use on standbys
Bruce Momjian [Fri, 17 Aug 2018 14:25:48 +0000 (10:25 -0400)]
pg_upgrade:  issue helpful error message for use on standbys

Commit 777e6ddf1723306bd2bf8fe6f804863f459b0323 checked for a shut down
message from a standby and allowed it to continue.  This patch reports a
helpful error message in these cases, suggesting to use rsync as
documented.

Diagnosed-by: Martín Marqués
Discussion: https://postgr.es/m/CAPdiE1xYCow-reLjrhJ9DqrMu-ppNq0ChUUEvVdxhdjGRD5_eA@mail.gmail.com

Backpatch-through: 9.3

6 years agoMention ownership requirements for REFRESH MATERIALIZED VIEW in docs
Michael Paquier [Fri, 17 Aug 2018 02:29:15 +0000 (11:29 +0900)]
Mention ownership requirements for REFRESH MATERIALIZED VIEW in docs

Author: Dian Fay
Discussion: https://postgr.es/m/745abbd2-a1a0-ead8-2cb2-768c16747d97@gmail.com
Backpatch-through: 9.3

6 years agoProof-reading for documentation.
Thomas Munro [Thu, 16 Aug 2018 23:38:44 +0000 (11:38 +1200)]
Proof-reading for documentation.

Somebody accidentally a word.  Back-patch to 9.6.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20180816195431.GA23707%40telsasoft.com

6 years agoClose the file descriptor in ApplyLogicalMappingFile
Tomas Vondra [Thu, 16 Aug 2018 14:49:10 +0000 (16:49 +0200)]
Close the file descriptor in ApplyLogicalMappingFile

The function was forgetting to close the file descriptor, resulting
in failures like this:

  ERROR:  53000: exceeded maxAllocatedDescs (492) while trying to open
  file "pg_logical/mappings/map-4000-4eb-1_60DE1E08-5376b5-537c6b"
  LOCATION:  OpenTransientFile, fd.c:2161

Simply close the file at the end, and backpatch to 9.4 (where logical
decoding was introduced). While at it, fix a nearby typo.

Discussion: https://www.postgresql.org/message-id/flat/738a590a-2ce5-9394-2bef-7b1caad89b37%402ndquadrant.com

6 years agoMake snprintf.c follow the C99 standard for snprintf's result value.
Tom Lane [Wed, 15 Aug 2018 21:25:23 +0000 (17:25 -0400)]
Make snprintf.c follow the C99 standard for snprintf's result value.

C99 says that the result should be the number of bytes that would have
been emitted given a large enough buffer, not the number we actually
were able to put in the buffer.  It's time to make our substitute
implementation comply with that.  Not doing so results in inefficiency
in buffer-enlargement cases, and also poses a portability hazard for
third-party code that might expect C99-compliant snprintf behavior
within Postgres.

In passing, remove useless tests for str == NULL; neither C99 nor
predecessor standards ever allowed that except when count == 0,
so I see no reason to expend cycles on making that a non-crash case
for this implementation.  Also, don't waste a byte in pg_vfprintf's
local I/O buffer; this might have performance benefits by allowing
aligned writes during flushbuffer calls.

Back-patch of commit 805889d7d.  There was some concern about this
possibly breaking code that assumes pre-C99 behavior, but there is
much more risk (and reality, in our own code) of code that assumes
C99 behavior and hence fails to detect buffer overrun without this.

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

6 years agoUpdate FSM on WAL replay of page all-visible/frozen
Alvaro Herrera [Wed, 15 Aug 2018 21:09:29 +0000 (18:09 -0300)]
Update FSM on WAL replay of page all-visible/frozen

We aren't very strict about keeping FSM up to date on WAL replay,
because per-page freespace values aren't critical in replicas (can't
write to heap in a replica; and if the replica is promoted, the values
would be updated by VACUUM anyway).  However, VACUUM since 9.6 can skip
processing pages marked all-visible or all-frozen, and if such pages are
recorded in FSM with wrong values, those values are blindly propagated
to FSM's upper layers by VACUUM's FreeSpaceMapVacuum.  (This rationale
assumes that crashes are not very frequent, because those would cause
outdated FSM to occur in the primary.)

Even when the FSM is outdated in standby, things are not too bad
normally, because, most per-page FSM values will be zero (other than
those propagated with the base-backup that created the standby); only
once the remaining free space is less than 0.2*BLCKSZ the per-page value
is maintained by WAL replay of heap ins/upd/del.  However, if
wal_log_hints=on causes complete FSM pages to be propagated to a standby
via full-page images, many too-optimistic per-page values can end up
being registered in the standby.

Incorrect per-page values aren't critical in most cases, since an
inserter that is given a page that doesn't actually contain the claimed
free space will update FSM with the correct value, and retry until it
finds a usable page.  However, if there are many such updates to do, an
inserter can spend a long time doing them before a usable page is found;
in a heavily trafficked insert-only table with many concurrent inserters
this has been observed to cause several second stalls, causing visible
application malfunction.

To fix this problem, it seems sufficient to have heap_xlog_visible
(replay of setting all-visible and all-frozen VM bits for a heap page)
update the FSM value for the page being processed.  This fixes the
per-page counters together with making the page skippable to vacuum, so
when vacuum does FreeSpaceMapVacuum, the values propagated to FSM upper
layers are the correct ones, avoiding the problem.

While at it, apply the same fix to heap_xlog_clean (replay of tuple
removal by HOT pruning and vacuum).  This makes any space freed by the
cleaning available earlier than the next vacuum in the promoted replica.

Backpatch to 9.6, where this problem was diagnosed on an insert-only
table with all-frozen pages, which were introduced as a concept in that
release.  Theoretically it could apply with all-visible pages to older
branches, but there's been no report of that and it doesn't backpatch
cleanly anyway.

Author: Álvaro Herrera 
Discussion: https://postgr.es/m/20180802172857[email protected]

6 years agoClean up assorted misuses of snprintf()'s result value.
Tom Lane [Wed, 15 Aug 2018 20:29:32 +0000 (16:29 -0400)]
Clean up assorted misuses of snprintf()'s result value.

Fix a small number of places that were testing the result of snprintf()
but doing so incorrectly.  The right test for buffer overrun, per C99,
is "result >= bufsize" not "result > bufsize".  Some places were also
checking for failure with "result == -1", but the standard only says
that a negative value is delivered on failure.

(Note that this only makes these places correct if snprintf() delivers
C99-compliant results.  But at least now these places are consistent
with all the other places where we assume that.)

Also, make psql_start_test() and isolation_start_test() check for
buffer overrun while constructing their shell commands.  There seems
like a higher risk of overrun, with more severe consequences, here
than there is for the individual file paths that are made elsewhere
in the same functions, so this seemed like a worthwhile change.

Also fix guc.c's do_serialize() to initialize errno = 0 before
calling vsnprintf.  In principle, this should be unnecessary because
vsnprintf should have set errno if it returns a failure indication ...
but the other two places this coding pattern is cribbed from don't
assume that, so let's be consistent.

These errors are all very old, so back-patch as appropriate.  I think
that only the shell command overrun cases are even theoretically
reachable in practice, but there's not much point in erroneous error
checks.

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

6 years agopg_upgrade: fix shutdown check for standby servers
Bruce Momjian [Tue, 14 Aug 2018 21:19:02 +0000 (17:19 -0400)]
pg_upgrade:  fix shutdown check for standby servers

Commit 244142d32afd02e7408a2ef1f249b00393983822 only tested for the
pg_controldata output for primary servers, but standby servers have
different "Database cluster state" output, so check for that too.

Diagnosed-by: Michael Paquier
Discussion: https://postgr.es/m/20180810164240[email protected]

Backpatch-through: 9.3

6 years agoRemove obsolete comment
Peter Eisentraut [Mon, 13 Aug 2018 19:07:31 +0000 (21:07 +0200)]
Remove obsolete comment

The sequence name is no longer stored in the sequence relation, since
1753b1b027035029c2a2a1649065762fafbf63f3.

6 years agoFix libpq's implementation of per-host connection timeouts.
Tom Lane [Mon, 13 Aug 2018 17:07:53 +0000 (13:07 -0400)]
Fix libpq's implementation of per-host connection timeouts.

Commit 5f374fe7a attempted to turn the connect_timeout from an overall
maximum time limit into a per-host limit, but it didn't do a great job of
that.  The timer would only get restarted if we actually detected timeout
within connectDBComplete(), not if we changed our attention to a new host
for some other reason.  In that case the old timeout continued to run,
possibly causing a premature timeout failure for the new host.

Fix that, and also tweak the logic so that if we do get a timeout,
we advance to the next available IP address, not to the next host name.
There doesn't seem to be a good reason to assume that all the IP
addresses supplied for a given host name will necessarily fail the
same way as the current one.  Moreover, this conforms better to the
admittedly-vague documentation statement that the timeout is "per
connection attempt".  I changed that to "per host name or IP address"
to be clearer.  (Note that reconnections to the same server, such as for
switching protocol version or SSL status, don't get their own separate
timeout; that was true before and remains so.)

Also clarify documentation about the interpretation of connect_timeout
values less than 2.

This seems like a bug, so back-patch to v10 where this logic came in.

Tom Lane, reviewed by Fabien Coelho

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

6 years agoAdjust comment atop ExecShutdownNode.
Amit Kapila [Mon, 13 Aug 2018 04:52:32 +0000 (10:22 +0530)]
Adjust comment atop ExecShutdownNode.

After commits a315b967cc and b805b63ac2, part of the comment atop
ExecShutdownNode is redundant.  Adjust it.

Author: Amit Kapila
Backpatch-through: 10 where both the mentioned commits are present.
Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info

6 years agoProhibit shutting down resources if there is a possibility of back up.
Amit Kapila [Mon, 13 Aug 2018 03:13:33 +0000 (08:43 +0530)]
Prohibit shutting down resources if there is a possibility of back up.

Currently, we release the asynchronous resources as soon as it is evident
that no more rows will be needed e.g. when a Limit is filled.  This can be
problematic especially for custom and foreign scans where we can scan
backward.  Fix that by disallowing the shutting down of resources in such
cases.

Reported-by: Robert Haas
Analysed-by: Robert Haas and Amit Kapila
Author: Amit Kapila
Reviewed-by: Robert Haas
Backpatch-through: 9.6 where this code was introduced
Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info

6 years agoAvoid query-lifetime memory leaks in XMLTABLE (bug #15321)
Andrew Gierth [Mon, 13 Aug 2018 00:45:35 +0000 (01:45 +0100)]
Avoid query-lifetime memory leaks in XMLTABLE (bug #15321)

Multiple calls to XMLTABLE in a query (e.g. laterally applying it to a
table with an xml column, an important use-case) were leaking large
amounts of memory into the per-query context, blowing up memory usage.

Repair by reorganizing memory context usage in nodeTableFuncscan; use
the usual per-tuple context for row-by-row evaluations instead of
perValueCxt, and use the explicitly created context -- renamed from
perValueCxt to perTableCxt -- for arguments and state for each
individual table-generation operation.

Backpatch to PG10 where this code was introduced.

Original report by IRC user begriffs; analysis and patch by me.
Reviewed by Tom Lane and Pavel Stehule.

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

6 years agoAdd missing documentation for argument of amcostestimate()
Alexander Korotkov [Fri, 10 Aug 2018 11:14:36 +0000 (14:14 +0300)]
Add missing documentation for argument of amcostestimate()

5262f7a4fc44 have introduced parallel index scan.  In order to estimate the
number of parallel workers, it adds extra argument to amcostestimate() index
access method API function.  However, this extra argument was missed in the
documentation.  This commit fixes that.

Discussion: https://postgr.es/m/4128fdb4-8b63-2e05-38f6-3125f8c27263%40lab.ntt.co.jp
Author: Tatsuro Yamada, Alexander Korotkov
Backpatch-through: 10

6 years agodocs: Only first instance of a PREPARE parameter sets data type
Bruce Momjian [Thu, 9 Aug 2018 14:13:15 +0000 (10:13 -0400)]
docs:  Only first instance of a PREPARE parameter sets data type

If the first reference to $1 is "($1 = col) or ($1 is null)", the data
type can be determined, but not for "($1 is null) or ($1 = col)".  This
change documents this.

Reported-by: Morgan Owens
Discussion: https://postgr.es/m/153233728858.1404.15268121695358514937@wrigleys.postgresql.org

Backpatch-through: 9.3

6 years agoDoc: Correct description of amcheck example query.
Peter Geoghegan [Wed, 8 Aug 2018 19:56:28 +0000 (12:56 -0700)]
Doc: Correct description of amcheck example query.

The amcheck documentation incorrectly claimed that its example query
verifies every catalog index in the database.  In fact, the query only
verifies the 10 largest indexes (as determined by pg_class.relpages).
Adjust the description accordingly.

Backpatch: 10-, where contrib/amcheck was introduced.

6 years agoDon't run atexit callbacks in quickdie signal handlers.
Heikki Linnakangas [Wed, 8 Aug 2018 16:08:10 +0000 (19:08 +0300)]
Don't run atexit callbacks in quickdie signal handlers.

exit() is not async-signal safe. Even if the libc implementation is, 3rd
party libraries might have installed unsafe atexit() callbacks. After
receiving SIGQUIT, we really just want to exit as quickly as possible, so
we don't really want to run the atexit() callbacks anyway.

The original report by Jimmy Yih was a self-deadlock in startup_die().
However, this patch doesn't address that scenario; the signal handling
while waiting for the startup packet is more complicated. But at least this
alleviates similar problems in the SIGQUIT handlers, like that reported
by Asim R P later in the same thread.

Backpatch to 9.3 (all supported versions).

Discussion: https://www.postgresql.org/message-id/CAOMx_OAuRUHiAuCg2YgicZLzPVv5d9_H4KrL_OFsFP%3DVPekigA%40mail.gmail.com

6 years agoDon't record FDW user mappings as members of extensions.
Tom Lane [Tue, 7 Aug 2018 20:32:50 +0000 (16:32 -0400)]
Don't record FDW user mappings as members of extensions.

CreateUserMapping has a recordDependencyOnCurrentExtension call that's
been there since extensions were introduced (very possibly my fault).
However, there's no support anywhere else for user mappings as members
of extensions, nor are they listed as a possible member object type in
the documentation.  Nor does it really seem like a good idea for user
mappings to belong to extensions when roles don't.  Hence, remove the
bogus call.

(As we saw in bug #15310, the lack of any pg_dump support for this case
ensures that any such membership record would silently disappear during
pg_upgrade.  So there's probably no need for us to do anything else
about cleaning up after this mistake.)

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

6 years agoFix incorrect initialization of BackendActivityBuffer.
Tom Lane [Tue, 7 Aug 2018 20:00:44 +0000 (16:00 -0400)]
Fix incorrect initialization of BackendActivityBuffer.

Since commit c8e8b5a6e, this has been zeroed out using the wrong length.
In practice the length would always be too small, leading to not zeroing
the whole buffer rather than clobbering additional memory; and that's
pretty harmless, both because shmem would likely start out as zeroes
and because we'd reinitialize any given entry before use.  Still,
it's bogus, so fix it.

Reported by Petru-Florin Mihancea (bug #15312)

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

6 years agoFix pg_upgrade to handle event triggers in extensions correctly.
Tom Lane [Tue, 7 Aug 2018 19:43:49 +0000 (15:43 -0400)]
Fix pg_upgrade to handle event triggers in extensions correctly.

pg_dump with --binary-upgrade must emit ALTER EXTENSION ADD commands
for all objects that are members of extensions.  It forgot to do so for
event triggers, as per bug #15310 from Nick Barnes.  Back-patch to 9.3
where event triggers were introduced.

Haribabu Kommi

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

6 years agoEnsure pg_dump_sort.c sorts null vs non-null namespace consistently.
Tom Lane [Tue, 7 Aug 2018 17:13:42 +0000 (13:13 -0400)]
Ensure pg_dump_sort.c sorts null vs non-null namespace consistently.

The original coding here (which is, I believe, my fault) supposed that
it didn't need to concern itself with the possibility that one object
of a given type-priority has a namespace while another doesn't.  But
that's not reliably true anymore, if it ever was; and if it does happen
then it's possible that DOTypeNameCompare returns self-inconsistent
comparison results.  That leads to unspecified behavior in qsort()
and a resultant weird output order from pg_dump.

This should end up being only a cosmetic problem, because any ordering
constraints that actually matter should be enforced by the later
dependency-based sort.  Still, it's a bug, so back-patch.

Report and fix by Jacob Champion, though I editorialized on his
patch to the extent of making NULL sort after non-NULL, for consistency
with our usual sorting definitions.

Discussion: https://postgr.es/m/CABAq_6Hw+V-Kj7PNfD5tgOaWT_-qaYkc+SRmJkPLeUjYXLdxwQ@mail.gmail.com

6 years agoStamp 10.5. REL_10_5
Tom Lane [Mon, 6 Aug 2018 20:05:31 +0000 (16:05 -0400)]
Stamp 10.5.

6 years agoTranslation updates
Peter Eisentraut [Mon, 6 Aug 2018 18:03:55 +0000 (20:03 +0200)]
Translation updates

Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 3463878fc340cb1436153b18a300f8cfdcb12adb

6 years agoLast-minute updates for release notes.
Tom Lane [Mon, 6 Aug 2018 17:13:41 +0000 (13:13 -0400)]
Last-minute updates for release notes.

Security: CVE-2018-10915, CVE-2018-10925

6 years agoFix failure to reset libpq's state fully between connection attempts.
Tom Lane [Mon, 6 Aug 2018 14:53:35 +0000 (10:53 -0400)]
Fix failure to reset libpq's state fully between connection attempts.

The logic in PQconnectPoll() did not take care to ensure that all of
a PGconn's internal state variables were reset before trying a new
connection attempt.  If we got far enough in the connection sequence
to have changed any of these variables, and then decided to try a new
server address or server name, the new connection might be completed
with some state that really only applied to the failed connection.

While this has assorted bad consequences, the only one that is clearly
a security issue is that password_needed didn't get reset, so that
if the first server asked for a password and the second didn't,
PQconnectionUsedPassword() would return an incorrect result.  This
could be leveraged by unprivileged users of dblink or postgres_fdw
to allow them to use server-side login credentials that they should
not be able to use.

Other notable problems include the possibility of forcing a v2-protocol
connection to a server capable of supporting v3, or overriding
"sslmode=prefer" to cause a non-encrypted connection to a server that
would have accepted an encrypted one.  Those are certainly bugs but
it's harder to paint them as security problems in themselves.  However,
forcing a v2-protocol connection could result in libpq having a wrong
idea of the server's standard_conforming_strings setting, which opens
the door to SQL-injection attacks.  The extent to which that's actually
a problem, given the prerequisite that the attacker needs control of
the client's connection parameters, is unclear.

These problems have existed for a long time, but became more easily
exploitable in v10, both because it introduced easy ways to force libpq
to abandon a connection attempt at a late stage and then try another one
(rather than just giving up), and because it provided an easy way to
specify multiple target hosts.

Fix by rearranging PQconnectPoll's state machine to provide centralized
places to reset state properly when moving to a new target host or when
dropping and retrying a connection to the same host.

Tom Lane, reviewed by Noah Misch.  Our thanks to Andrew Krasichkov
for finding and reporting the problem.

Security: CVE-2018-10915

6 years agoAdjust error message
Peter Eisentraut [Mon, 11 Jun 2018 21:19:11 +0000 (17:19 -0400)]
Adjust error message

Makes it look more similar to other ones, and avoids the need for
pluralization.

6 years agoRelease notes for 10.5, 9.6.10, 9.5.14, 9.4.19, 9.3.24.
Tom Lane [Sun, 5 Aug 2018 20:38:43 +0000 (16:38 -0400)]
Release notes for 10.5, 9.6.10, 9.5.14, 9.4.19, 9.3.24.

6 years agoDoc: fix incorrectly stated argument list for pgcrypto's hmac() function.
Tom Lane [Sun, 5 Aug 2018 17:03:42 +0000 (13:03 -0400)]
Doc: fix incorrectly stated argument list for pgcrypto's hmac() function.

The bytea variant takes (bytea, bytea, text).
Per unsigned report.

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

6 years agoFix INSERT ON CONFLICT UPDATE through a view that isn't just SELECT *.
Tom Lane [Sat, 4 Aug 2018 23:38:58 +0000 (19:38 -0400)]
Fix INSERT ON CONFLICT UPDATE through a view that isn't just SELECT *.

When expanding an updatable view that is an INSERT's target, the rewriter
failed to rewrite Vars in the ON CONFLICT UPDATE clause.  This accidentally
worked if the view was just "SELECT * FROM ...", as the transformation
would be a no-op in that case.  With more complicated view targetlists,
this omission would often lead to "attribute ... has the wrong type" errors
or even crashes, as reported by Mario De Frutos Dieguez.

Fix by adding code to rewriteTargetView to fix up the data structure
correctly.  The easiest way to update the exclRelTlist list is to rebuild
it from scratch looking at the new target relation, so factor the code
for that out of transformOnConflictClause to make it sharable.

In passing, avoid duplicate permissions checks against the EXCLUDED
pseudo-relation, and prevent useless view expansion of that relation's
dummy RTE.  The latter is only known to happen (after this patch) in cases
where the query would fail later due to not having any INSTEAD OF triggers
for the view.  But by exactly that token, it would create an unintended
and very poorly tested state of the query data structure, so it seems like
a good idea to prevent it from happening at all.

This has been broken since ON CONFLICT was introduced, so back-patch
to 9.5.

Dean Rasheed, based on an earlier patch by Amit Langote;
comment-kibitzing and back-patching by me

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

6 years agoReset properly errno before calling write()
Michael Paquier [Sat, 4 Aug 2018 20:32:12 +0000 (05:32 +0900)]
Reset properly errno before calling write()

6cb3372 enforces errno to ENOSPC when less bytes than what is expected
have been written when it is unset, though it forgot to properly reset
errno before doing a system call to write(), causing errno to
potentially come from a previous system call.

Reported-by: Tom Lane
Author: Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/31797.1533326676@sss.pgh.pa.us

6 years agoAdd table relcache invalidation to index builds.
Peter Geoghegan [Fri, 3 Aug 2018 21:44:56 +0000 (14:44 -0700)]
Add table relcache invalidation to index builds.

It's necessary to make sure that owning tables have a relcache
invalidation prior to advancing the command counter to make
newly-entered catalog tuples for the index visible.  inval.c must be
able to maintain the consistency of the local caches in the event of
transaction abort.  There is usually only a problem when CREATE INDEX
transactions abort, since there is a generic invalidation once we reach
index_update_stats().

This bug is of long standing.  Problems were made much more likely by
the addition of parallel CREATE INDEX (commit 9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers().  (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)

Author: Peter Geoghegan
Reported-By: Luca Ferrari
Diagnosed-By: Andres Freund
Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/CAKoxK+5fVodiCtMsXKV_1YAKXbzwSfp7DgDqUmcUAzeAhf=HEQ@mail.gmail.com
Backpatch: 9.3-

6 years agoAdd 'n' to list of possible values to pg_default_acl.defaclobjtype
Alvaro Herrera [Fri, 3 Aug 2018 20:45:08 +0000 (16:45 -0400)]
Add 'n' to list of possible values to pg_default_acl.defaclobjtype

This was missed in commit ab89e465cb20; backpatch to v10.

Author: Fabien Coelho 
Discussion: https://postgr.es/m/alpine.DEB.2.21.1807302243001.13230@lancre

6 years agoFix pg_replication_slot example output
Alvaro Herrera [Fri, 3 Aug 2018 20:34:59 +0000 (16:34 -0400)]
Fix pg_replication_slot example output

The example output of pg_replication_slot is wrong.  Correct it and make
the output stable by explicitly listing columns to output.

Author: Kyotaro Horiguchi 
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/20180731.190909.42582169[email protected]

6 years agoRemove no-longer-appropriate special case in psql's \conninfo code.
Tom Lane [Fri, 3 Aug 2018 16:20:47 +0000 (12:20 -0400)]
Remove no-longer-appropriate special case in psql's \conninfo code.

\conninfo prints the results of PQhost() and some other libpq functions.
It used to override the PQhost() result with the hostaddr parameter if
that'd been given, but that's unhelpful when multiple hosts were listed
in the connection string.  Furthermore, it seems unnecessary in the wake
of commit 1944cdc98, since PQhost does any useful substitution itself.
So let's just remove the extra code and print PQhost()'s result without
any editorialization.

Back-patch to v10, as 1944cdc98 (just) was.

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

6 years agoChange libpq's internal uses of PQhost() to inspect host field directly.
Tom Lane [Fri, 3 Aug 2018 16:12:10 +0000 (12:12 -0400)]
Change libpq's internal uses of PQhost() to inspect host field directly.

Commit 1944cdc98 changed PQhost() to return the hostaddr value when that
is specified and host isn't.  This is a good idea in general, but
fe-auth.c and related files contain PQhost() calls for which it isn't.
Specifically, when we compare SSL certificates or other server identity
information to the host field, we do not want to use hostaddr instead;
that's not what's documented, that's not what happened pre-v10, and
it doesn't seem like a good idea.

Instead, we can just look at connhost[].host directly.  This does what
we want in v10 and up; in particular, if neither host nor hostaddr
were given, the host field will be replaced with the default host name.
That seems useful, and it's likely the reason that these places were
coded to call PQhost() originally (since pre-v10, the stored field was
not replaced with the default).

Back-patch to v10, as 1944cdc98 (just) was.

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

6 years agolibpq: PQhost to return active connected host or hostaddr
Tom Lane [Fri, 3 Aug 2018 15:30:34 +0000 (11:30 -0400)]
libpq: PQhost to return active connected host or hostaddr

Previously, PQhost didn't return the connected host details when the
connection type was CHT_HOST_ADDRESS (i.e., via hostaddr).  Instead, it
returned the complete host connection parameter (which could contain
multiple hosts) or the default host details, which was confusing and
arguably incorrect.

Change this to return the actually connected host or hostaddr
irrespective of the connection type.  When hostaddr but no host was
specified, hostaddr is now returned.  Never return the original host
connection parameter, and document that PQhost cannot be relied on
before the connection is established.

PQport is similarly changed to always return the active connection port
and never the original connection parameter.

Back-patch of commit 1944cdc98273dbb8439ad9b387ca2858531afcf0
into the v10 branch.

Author: Hari Babu 
Reviewed-by: Michael Paquier
Reviewed-by: Kyotaro HORIGUCHI
Reviewed-by: David G. Johnston
6 years agoFix buffer usage stats for parallel nodes.
Amit Kapila [Fri, 3 Aug 2018 05:57:11 +0000 (11:27 +0530)]
Fix buffer usage stats for parallel nodes.

The buffer usage stats is accounted only for the execution phase of the
node.  For Gather and Gather Merge nodes, such stats are accumulated at
the time of shutdown of workers which is done after execution of node due
to which we missed to account them for such nodes.  Fix it by treating
nodes as running while we shut down them.

We can also miss accounting for a Limit node when Gather or Gather Merge
is beneath it, because it can finish the execution before shutting down
such nodes.  So we allow a Limit node to shut down the resources before it
completes the execution.

In the passing fix the gather node code to allow workers to shut down as
soon as we find that all the tuples from the workers have been retrieved.
The original code use to do that, but is accidently removed by commit
01edb5c7fc.

Reported-by: Adrien Nayrat
Author: Amit Kapila and Robert Haas
Reviewed-by: Robert Haas and Andres Freund
Backpatch-through: 9.6 where this code was introduced
Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info

6 years agoMatch the buffer usage tracking for leader and worker backends.
Amit Kapila [Fri, 3 Aug 2018 04:20:24 +0000 (09:50 +0530)]
Match the buffer usage tracking for leader and worker backends.

In the leader backend, we don't track the buffer usage for ExecutorStart
phase whereas in worker backend we track it for ExecutorStart phase as
well.  This leads to different value for buffer usage stats for the
parallel and non-parallel query.  Change the code so that worker backend
also starts tracking buffer usage after ExecutorStart.

Author: Amit Kapila and Robert Haas
Reviewed-by: Robert Haas and Andres Freund
Backpatch-through: 9.6 where this code was introduced
Discussion:https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info

6 years agoFix libpq's code for searching .pgpass; rationalize empty-list-item cases.
Tom Lane [Wed, 1 Aug 2018 16:30:36 +0000 (12:30 -0400)]
Fix libpq's code for searching .pgpass; rationalize empty-list-item cases.

Before v10, we always searched ~/.pgpass using the host parameter,
and nothing else, to match to the "hostname" field of ~/.pgpass.
(However, null host or host matching DEFAULT_PGSOCKET_DIR was replaced by
"localhost".)  In v10, this got broken by commit 274bb2b38, repaired by
commit bdac9836d, and broken again by commit 7b02ba62e; in the code
actually shipped, we'd search with hostaddr if both that and host were
specified --- though oddly, *not* if only hostaddr were specified.
Since this is directly contrary to the documentation, and not
backwards-compatible, it's clearly a bug.

However, the change wasn't totally without justification, even though it
wasn't done quite right, because the pre-v10 behavior has arguably been
buggy since we added hostaddr.  If hostaddr is specified and host isn't,
the pre-v10 code will search ~/.pgpass for "localhost", and ship that
password off to a server that most likely isn't local at all.  That's
unhelpful at best, and could be a security breach at worst.

Therefore, rather than just revert to that old behavior, let's define
the behavior as "search with host if provided, else with hostaddr if
provided, else search for localhost".  (As before, a host name matching
DEFAULT_PGSOCKET_DIR is replaced by localhost.)  This matches the
behavior of the actual connection code, so that we don't pick up an
inappropriate password; and it allows useful searches to happen when
only hostaddr is given.

While we're messing around here, ensure that empty elements within a
host or hostaddr list select the same behavior as a totally-empty
field would; for instance "host=a,,b" is equivalent to "host=a,/tmp,b"
if DEFAULT_PGSOCKET_DIR is /tmp.  Things worked that way in some cases
already, but not consistently so, which contributed to the confusion
about what key ~/.pgpass would get searched with.

Update documentation accordingly, and also clarify some nearby text.

Back-patch to v10 where the host/hostaddr list functionality was
introduced.

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

6 years agopg_upgrade: fix --check for live source server checks
Bruce Momjian [Tue, 31 Jul 2018 22:10:06 +0000 (18:10 -0400)]
pg_upgrade:  fix --check for live source server checks

Fix for commit 244142d32afd02e7408a2ef1f249b00393983822.

Backpatch-through: 9.3

6 years agoFurther fixes for quoted-list GUC values in pg_dump and ruleutils.c.
Tom Lane [Tue, 31 Jul 2018 17:00:08 +0000 (13:00 -0400)]
Further fixes for quoted-list GUC values in pg_dump and ruleutils.c.

Commits 742869946 et al turn out to be a couple bricks shy of a load.
We were dumping the stored values of GUC_LIST_QUOTE variables as they
appear in proconfig or setconfig catalog columns.  However, although that
quoting rule looks a lot like SQL-identifier double quotes, there are two
critical differences: empty strings ("") are legal, and depending on which
variable you're considering, values longer than NAMEDATALEN might be valid
too.  So the current technique fails altogether on empty-string list
entries (as reported by Steven Winfield in bug #15248) and it also risks
truncating file pathnames during dump/reload of GUC values that are lists
of pathnames.

To fix, split the stored value without any downcasing or truncation,
and then emit each element as a SQL string literal.

This is a tad annoying, because we now have three copies of the
comma-separated-string splitting logic in varlena.c as well as a fourth
one in dumputils.c.  (Not to mention the randomly-different-from-those
splitting logic in libpq...)  I looked at unifying these, but it would
be rather a mess unless we're willing to tweak the API definitions of
SplitIdentifierString, SplitDirectoriesString, or both.  That might be
worth doing in future; but it seems pretty unsafe for a back-patched
bug fix, so for now accept the duplication.

Back-patch to all supported branches, as the previous fix was.

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

6 years agoSet ActiveSnapshot when logically replaying inserts
Alvaro Herrera [Mon, 30 Jul 2018 20:30:07 +0000 (16:30 -0400)]
Set ActiveSnapshot when logically replaying inserts

Input functions for the inserted tuples may require a snapshot, when
they are replayed by native logical replication.  An example is a domain
with a constraint using a SQL-language function, which prior to this
commit failed to apply on the subscriber side.

Reported-by: Mai Peng
Co-authored-by: Minh-Quan TRAN
Co-authored-by: Álvaro Herrera
Discussion: https://postgr.es/m/4EB4BD78-BFC3-4D04-B8DA-D53DF7160354@webedia-group.com
Discussion: https://postgr.es/m/153211336163.1404.11721804383024050689@wrigleys.postgresql.org

6 years agoFix pg_dump's failure to dump REPLICA IDENTITY for constraint indexes.
Tom Lane [Mon, 30 Jul 2018 16:35:49 +0000 (12:35 -0400)]
Fix pg_dump's failure to dump REPLICA IDENTITY for constraint indexes.

pg_dump knew about printing ALTER TABLE ... REPLICA IDENTITY USING INDEX
for indexes declared as indexes, but it failed to print that for indexes
declared as unique or primary-key constraints.  Per report from Achilleas
Mantzios.

This has been broken since the feature was introduced, AFAICS.
Back-patch to 9.4.

Discussion: https://postgr.es/m/1e6cc5ad-b84a-7c07-8c08-a4d0c3cdc938@matrix.gatewaynet.com

6 years agoDoc: fix oversimplified example for CREATE POLICY.
Tom Lane [Mon, 30 Jul 2018 15:54:41 +0000 (11:54 -0400)]
Doc: fix oversimplified example for CREATE POLICY.

As written, this policy constrained only the post-image not the pre-image
of rows, meaning that users could delete other users' rows or take
ownership of such rows, contrary to what the docs claimed would happen.
We need two separate policies to achieve the documented effect.

While at it, try to explain what's happening a bit more fully.

Per report from Олег Самойлов.  Back-patch to 9.5 where this was added.
Thanks to Stephen Frost for off-list discussion.

Discussion: https://postgr.es/m/3298321532002010@sas1-2b3c3045b736.qloud-c.yandex.net

6 years agoFix earthdistance test suite function name typo.
Noah Misch [Sun, 29 Jul 2018 19:02:07 +0000 (12:02 -0700)]
Fix earthdistance test suite function name typo.

Affected test queries have been testing the wrong thing since their
introduction in commit 4c1383efd132e4f532213c8a8cc63a455f55e344.
Back-patch to 9.3 (all supported versions).

6 years agoDocument security implications of qualified names.
Noah Misch [Sun, 29 Jul 2018 03:08:01 +0000 (20:08 -0700)]
Document security implications of qualified names.

Commit 5770172cb0c9df9e6ce27c507b449557e5b45124 documented secure schema
usage, and that advice suffices for using unqualified names securely.
Document, in typeconv-func primarily, the additional issues that arise
with qualified names.  Back-patch to 9.3 (all supported versions).

Reviewed by Jonathan S. Katz.

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

6 years agopgtest: run clean, build, and check stages separately
Bruce Momjian [Sat, 28 Jul 2018 19:34:06 +0000 (15:34 -0400)]
pgtest:  run clean, build, and check stages separately

This allows for cleaner error reporting.

Backpatch-through: 9.5

6 years agopg_upgrade: check for clean server shutdowns
Bruce Momjian [Sat, 28 Jul 2018 19:01:55 +0000 (15:01 -0400)]
pg_upgrade:  check for clean server shutdowns

Previously pg_upgrade checked for the pid file and started/stopped the
server to force a clean shutdown.  However, "pg_ctl -m immediate"
removes the pid file but doesn't do a clean shutdown, so check
pg_controldata for a clean shutdown too.

Diagnosed-by: Vimalraj A
Discussion: https://postgr.es/m/CAFKBAK5e4Q-oTUuPPJ56EU_d2Rzodq6GWKS3ncAk3xo7hAsOZg@mail.gmail.com

Backpatch-through: 9.3

6 years agopgtest: grab possible warnings from install.log
Bruce Momjian [Sat, 28 Jul 2018 15:35:52 +0000 (11:35 -0400)]
pgtest:  grab possible warnings from install.log

Since PG 9.5, 'make check' records the build output in install.log, so
look in there for warnings too.

Backpatch-through: 9.5

6 years agoFix the buffer release order for parallel index scans.
Amit Kapila [Fri, 27 Jul 2018 05:35:06 +0000 (11:05 +0530)]
Fix the buffer release order for parallel index scans.

During parallel index scans, if the current page to be read is deleted, we
skip it and try to get the next page for a scan without releasing the buffer
lock on the current page.  To get the next page, sometimes it needs to wait
for another process to complete its scan and advance it to the next page.
Now, it is quite possible that the master backend has errored out before
advancing the scan and issued a termination signal for all workers.  The
workers failed to notice the termination request during wait because the
interrupts are held due to buffer lock on the previous page.  This lead to
all workers being stuck.

The fix is to release the buffer lock on current page before trying to get
the next page.  We are already doing same in backward scans, but missed
it for forward scans.

Reported-by: Victor Yegorov
Bug: 15290
Diagnosed-by: Thomas Munro and Amit Kapila
Author: Amit Kapila
Reviewed-by: Thomas Munro
Tested-By: Thomas Munro and Victor Yegorov
Backpatch-through: 10 where parallel index scans were introduced
Discussion: https://postgr.es/m/153228422922.1395.1746424054206154747@wrigleys.postgresql.org

6 years agoPad semaphores to avoid false sharing.
Thomas Munro [Tue, 24 Jul 2018 22:58:54 +0000 (10:58 +1200)]
Pad semaphores to avoid false sharing.

In a USE_UNNAMED_SEMAPHORES build, the default on Linux and FreeBSD
since commit ecb0d20a, we have an array of sem_t objects.  This
turned out to reduce performance compared to the previous default
USE_SYSV_SEMAPHORES on an 8 socket system.  Testing showed that the
lost performance could be regained by padding the array elements so
that they have their own cache lines.  This matches what we do for
similar hot arrays (see LWLockPadded, WALInsertLockPadded).

Back-patch to 10, where unnamed semaphores were adopted as the default
semaphore interface on those operating systems.

Author: Thomas Munro
Reviewed-by: Andres Freund
Reported-by: Mithun Cy
Tested-by: Mithun Cy, Tom Lane, Thomas Munro
Discussion: https://postgr.es/m/CAD__OugYDM3O%2BdyZnnZSbJprSfsGFJcQ1R%3De59T3hcLmDug4_w%40mail.gmail.com

6 years agodoc: Fix reference to "decoder" to instead be the correct "output plugin".
Andres Freund [Tue, 24 Jul 2018 17:51:09 +0000 (10:51 -0700)]
doc: Fix reference to "decoder" to instead be the correct "output plugin".

Author: Jonathan Katz
Discussion: https://postgr.es/m/DD02DD86-5989-4BFD-8712-468541F68383@postgresql.org
Backpatch: 9.4-, where logical decoding was added

6 years agoFurther portability hacking in pg_upgrade's test script.
Tom Lane [Sat, 21 Jul 2018 19:40:51 +0000 (15:40 -0400)]
Further portability hacking in pg_upgrade's test script.

I blew the dust off a Bourne shell (file date 1996, yea verily) and
tried to run test.sh with it.  It mostly worked, but I found that the
temp-directory creation code introduced by commit be76a6d39 was not
compatible, for a couple of reasons: this shell thinks "set -e" should
force an exit if a command within backticks fails, and it also thinks code
within braces should be executed by a sub-shell, meaning that variable
settings don't propagate back up to the parent shell.  In view of Victor
Wagner's report that Solaris is still using pre-POSIX shells, seems like
we oughta make this case work.  It's not like the code is any less
idiomatic this way; the prior coding technique appeared nowhere else.

(There is a remaining bash-ism here, which is that $RANDOM doesn't do
what the code hopes in non-bash shells.  But the use of $$ elsewhere in
that path should be enough to ensure uniqueness and some amount of
randomness, so I think it's okay as-is.)

Back-patch to all supported branches, as the previous commit was.

Discussion: https://postgr.es/m/20180720153820.69e9ae6c@fafnir.local.vm

6 years agoGuard against rare RAND_bytes() failures in pg_strong_random().
Dean Rasheed [Fri, 20 Jul 2018 07:58:37 +0000 (08:58 +0100)]
Guard against rare RAND_bytes() failures in pg_strong_random().

When built using OpenSSL, pg_strong_random() uses RAND_bytes() to
generate the random number. On very rare occasions that can fail, if
its PRNG has not been seeded with enough data. Additionally, once it
does fail, all subsequent calls will also fail until more seed data is
added. Since this is required during backend startup, this can result
in all new backends failing to start until a postmaster restart.

Guard against that by checking the state of OpenSSL's PRNG using
RAND_status(), and if necessary (very rarely), seeding it using
RAND_poll().

Back-patch to v10, where pg_strong_random() was introduced.

Dean Rasheed and Michael Paquier.

Discussion: https://postgr.es/m/CAEZATCXMtxbzSAvyKKk5uCRf9pNt4UV%2BF_5v%3DgLfJUuPxU4Ytg%40mail.gmail.com

6 years agoRemove undocumented restriction against duplicate partition key columns.
Tom Lane [Thu, 19 Jul 2018 19:41:46 +0000 (15:41 -0400)]
Remove undocumented restriction against duplicate partition key columns.

transformPartitionSpec rejected duplicate simple partition columns
(e.g., "PARTITION BY RANGE (x,x)") but paid no attention to expression
columns, resulting in inconsistent behavior.  Worse, cases like
"PARTITION BY RANGE (x,(x))") were accepted but would then result in
dump/reload failures, since the expression (x) would get simplified
to a plain column later.

There seems no better reason for this restriction than there was for
the one against duplicate included index columns (cf commit 701fd0bbc),
so let's just remove it.

Back-patch to v10 where this code was added.

Report and patch by Yugo Nagata.

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

6 years agoFix handling of empty uncompressed posting list pages in GIN
Alexander Korotkov [Thu, 19 Jul 2018 18:04:17 +0000 (21:04 +0300)]
Fix handling of empty uncompressed posting list pages in GIN

PostgreSQL 9.4 introduces posting list compression in GIN.  This feature
supports online upgrade, so that after pg_upgrade uncompressed posting
lists are compressed on-the-fly.  Underlying code appears to always
expect at least one item on uncompressed posting list page.  But there
could be completely empty pages, because VACUUM never deletes leftmost
and rightmost pages from posting trees.  This commit fixes that.

Reported-by: Sivasubramanian Ramasubramanian
Discussion: https://postgr.es/m/1531867212836.63354%40amazon.com
Author: Sivasubramanian Ramasubramanian, Alexander Korotkov
Backpatch-through: 9.4

6 years agoFix error message when a hostaddr cannot be parsed.
Heikki Linnakangas [Thu, 19 Jul 2018 17:24:29 +0000 (20:24 +0300)]
Fix error message when a hostaddr cannot be parsed.

We were incorrectly passing hostname, not hostaddr, in the error message,
and because of that, you got:

$ psql 'hostaddr=foo'
psql: could not parse network address "(null)": Name or service not known

Backpatch to v10, where this was broken (by commit 7b02ba62e9).

Report and fix by Robert Haas.

Discussion: https://www.postgresql.org/message-id/CA+TgmoapFQA30NomGKEaZCu3iN7mF7fux8fbbk9SouVOT2JP7w@mail.gmail.com

6 years agoRephrase a few comments for clarity.
Heikki Linnakangas [Thu, 19 Jul 2018 13:08:09 +0000 (16:08 +0300)]
Rephrase a few comments for clarity.

I was confused by what "intended to be parallel serially" meant, until
Robert Haas and David G. Johnston explained it. Rephrase the comment to
make it more clear, using David's suggested wording.

Discussion: https://www.postgresql.org/message-id/1fec9022-41e8-e484-70ce-2179b08c2092%40iki.fi

6 years agoFix print of Path nodes when using OPTIMIZER_DEBUG
Michael Paquier [Thu, 19 Jul 2018 00:55:15 +0000 (09:55 +0900)]
Fix print of Path nodes when using OPTIMIZER_DEBUG

GatherMergePath (introduced in 10) and CustomPath (introduced in 9.5)
have gone missing.  The order of the Path nodes was inconsistent with
what is listed in nodes.h, so make the order consistent at the same time
to ease future checks and additions.

Author: Sawada Masahiko
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAD21AoBQMLoc=ohH-oocuAPsELrmk8_EsRJjOyR8FQLZkbE0wA@mail.gmail.com

6 years agoRemove race-prone hot_standby_feedback test cases in 001_stream_rep.pl.
Tom Lane [Wed, 18 Jul 2018 21:39:27 +0000 (17:39 -0400)]
Remove race-prone hot_standby_feedback test cases in 001_stream_rep.pl.

This script supposed that if it turned hot_standby_feedback on and then
shut down the standby server, at least one feedback message would be
guaranteed to be sent before the standby stops.  But there is no such
guarantee, if the standby's walreceiver process is slow enough --- and
we've seen multiple failures in the buildfarm showing that that does
happen in practice.  While we could rearrange the walreceiver logic to
make it less likely, it seems probably impossible to create a really
bulletproof guarantee of that sort; and if we tried, we might create
situations where the walreceiver wouldn't react in a timely manner to
shutdown commands.  It seems better instead to remove the script's
assumption that feedback will occur before shutdown.

But once we do that, these last few tests seem quite redundant with
the earlier tests in the script.  So let's just drop them altogether
and save some buildfarm cycles.

Backpatch to v10 where these tests were added.

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

6 years agoFix misc typos, mostly in comments.
Heikki Linnakangas [Mon, 9 Jul 2018 13:10:44 +0000 (16:10 +0300)]
Fix misc typos, mostly in comments.

A collection of typos I happened to spot while reading code, as well as
grepping for common mistakes.

Backpatch to all supported versions, as applicable, to avoid conflicts
when backporting other commits in the future.

6 years agoAdd subtransaction handling for table synchronization workers.
Robert Haas [Mon, 16 Jul 2018 21:33:22 +0000 (17:33 -0400)]
Add subtransaction handling for table synchronization workers.

Since the old logic was completely unaware of subtransactions, a
change made in a subsequently-aborted subtransaction would still cause
workers to be stopped at toplevel transaction commit.  Fix that by
managing a stack of worker lists rather than just one.

Amit Khandekar and Robert Haas

Discussion: http://postgr.es/m/CAJ3gD9eaG_mWqiOTA2LfAug-VRNn1hrhf50Xi1YroxL37QkZNg@mail.gmail.com

6 years agoFix hashjoin costing mistake introduced with inner_unique optimization.
Tom Lane [Sat, 14 Jul 2018 15:59:12 +0000 (11:59 -0400)]
Fix hashjoin costing mistake introduced with inner_unique optimization.

In final_cost_hashjoin(), commit 9c7f5229a allowed inner_unique cases
to follow a code path previously used only for SEMI/ANTI joins; but it
neglected to fix an if-test within that path that assumed SEMI and ANTI
were the only possible cases.  This resulted in a wrong value for
hashjointuples, and an ensuing bad cost estimate, for inner_unique normal
joins.  Fortunately, for inner_unique normal joins we can assume the number
of joined tuples is the same as for a SEMI join; so there's no need for
more code, we just have to invert the test to check for ANTI not SEMI.

It turns out that in two contrib tests in which commit 9c7f5229a
changed the plan expected for a query, the change was actually wrong
and induced by this estimation error, not by any real improvement.
Hence this patch also reverts those changes.

Per report from RK Korlapati.  Backpatch to v10 where the error was
introduced.

David Rowley

Discussion: https://postgr.es/m/CA+SNy03bhq0fodsfOkeWDCreNjJVjsdHwUsb7AG=jpe0PtZc_g@mail.gmail.com

6 years agoFix crash in contrib/ltree's lca() function for empty input array.
Tom Lane [Fri, 13 Jul 2018 22:45:30 +0000 (18:45 -0400)]
Fix crash in contrib/ltree's lca() function for empty input array.

lca_inner() wasn't prepared for the possibility of getting no inputs.
Fix that, and make some cosmetic improvements to the code while at it.

Also, I thought the documentation of this function as returning the
"longest common prefix" of the paths was entirely misleading; it really
returns a path one shorter than the longest common prefix, for the typical
definition of "prefix".  Don't use that term in the docs, and adjust the
examples to clarify what really happens.

This has been broken since its beginning, so back-patch to all supported
branches.

Per report from Hailong Li.  Thanks to Pierre Ducroquet for diagnosing
and for the initial patch, though I whacked it around some and added
test cases.

Discussion: https://postgr.es/m/5b0d8e4f-f2a3-1305-d612-e00e35a7be66@qunar.com

6 years agoFix inadequate buffer locking in FSM and VM page re-initialization.
Tom Lane [Fri, 13 Jul 2018 15:52:50 +0000 (11:52 -0400)]
Fix inadequate buffer locking in FSM and VM page re-initialization.

When reading an existing FSM or VM page that was found to be corrupt by the
buffer manager, the code applied PageInit() to reinitialize the page, but
did so without any locking.  There is thus a hazard that two backends might
concurrently do PageInit, which in itself would still be OK, but the slower
one might then zero over subsequent data changes applied by the faster one.
Even that is unlikely to be fatal; but it's not desirable, so add locking
to prevent it.

This does not add any locking overhead in the normal code path where the
page is OK.  It's not immediately obvious that that's safe, but I believe
it is, for reasons explained in the added comments.

Problem noted by R P Asim.  It's been like this for a long time, so
back-patch to all supported branches.

Discussion: https://postgr.es/m/CANXE4Te4G0TGq6cr0-TvwP0H4BNiK_-hB5gHe8mF+nz0mcYfMQ@mail.gmail.com

6 years agodocs: Remove "New" description of the libpqxx interface
Bruce Momjian [Fri, 13 Jul 2018 15:16:55 +0000 (11:16 -0400)]
docs:  Remove "New" description of the libpqxx interface

Backpatch-through: 9.3

6 years agoDoc: minor improvement in pl/pgsql FETCH/MOVE documentation.
Tom Lane [Thu, 12 Jul 2018 16:28:43 +0000 (12:28 -0400)]
Doc: minor improvement in pl/pgsql FETCH/MOVE documentation.

Explain that you can use any integer expression for the "count" in
pl/pgsql's versions of FETCH/MOVE, unlike the SQL versions which only
allow a constant.

Remove the duplicate version of this para under MOVE.  I don't see
a good reason to maintain two identical paras when we just said that
MOVE works exactly like FETCH.

Per Pavel Stehule, though I didn't use his text.

Discussion: https://postgr.es/m/CAFj8pRAcvSXcNdUGx43bOK1e3NNPbQny7neoTLN42af+8MYWEA@mail.gmail.com

6 years agoMake logical WAL sender report streaming state appropriately
Michael Paquier [Thu, 12 Jul 2018 01:20:08 +0000 (10:20 +0900)]
Make logical WAL sender report streaming state appropriately

WAL senders sending logically-decoded data fail to properly report in
"streaming" state when starting up, hence as long as one extra record is
not replayed, such WAL senders would remain in a "catchup" state, which
is inconsistent with the physical cousin.

This can be easily reproduced by for example using pg_recvlogical and
restarting the upstream server.  The TAP tests have been slightly
modified to detect the failure and strengthened so as future tests also
make sure that a node is in streaming state when waiting for its
catchup.

Backpatch down to 9.4 where this code has been introduced.

Reported-by: Sawada Masahiko
Author: Simon Riggs, Sawada Masahiko
Reviewed-by: Petr Jelinek, Michael Paquier, Vaishnavi Prabakaran
Discussion: https://postgr.es/m/CAD21AoB2ZbCCqOx=bgKMcLrAvs1V0ZMqzs7wBTuDySezTGtMZA@mail.gmail.com

6 years agoFix create_scan_plan's handling of sortgrouprefs for physical tlists.
Tom Lane [Wed, 11 Jul 2018 19:25:29 +0000 (15:25 -0400)]
Fix create_scan_plan's handling of sortgrouprefs for physical tlists.

We should only run apply_pathtarget_labeling_to_tlist if CP_LABEL_TLIST
was specified, because only in that case has use_physical_tlist checked
that the labeling will succeed; otherwise we may get an "ORDER/GROUP BY
expression not found in targetlist" error.  (This subsumes the previous
test about gating_clauses, because we reset "flags" to zero earlier
if there are gating clauses to apply.)

The only known case in which a failure can occur is with a ProjectSet
path directly atop a table scan path, although it seems likely that there
are other cases or will be such in future.  This means that the failure
is currently only visible in the v10 branch: 9.6 didn't have ProjectSet,
while in v11 and HEAD, apply_scanjoin_target_to_paths for some weird
reason is using create_projection_path not apply_projection_to_path,
masking the problem because there's a ProjectionPath in between.

Nonetheless this code is clearly wrong on its own terms, so back-patch
to 9.6 where this logic was introduced.

Per report from Regina Obe.

Discussion: https://postgr.es/m/001501d40f88$75186950$5f493bf0[email protected]

6 years agoBetter handle pseudotypes as partition keys
Alvaro Herrera [Tue, 10 Jul 2018 19:07:28 +0000 (15:07 -0400)]
Better handle pseudotypes as partition keys

We fail to handle polymorphic types properly when they are used as
partition keys: we were unnecessarily adding a RelabelType node on top,
which confuses code examining the nodes.  In particular, this makes
predtest.c-based partition pruning not to work, and ruleutils.c to emit
expressions that are uglier than needed.  Fix it by not adding RelabelType
when not needed.

In master/11 the new pruning code is separate so it doesn't suffer from
this problem, since we already fixed it (in essentially the same way) in
e5dcbb88a15d, which also added a few tests; back-patch those tests to
pg10 also.  But since UPDATE/DELETE still uses predtest.c in pg11, this
change improves partitioning for those cases too.  Add tests for this.
The ruleutils.c behavior change is relevant in pg11/master too.

Co-authored-by: Amit Langote
Co-authored-by: Álvaro Herrera
Reviewed-by: Álvaro Herrera
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/54745d13-7ed4-54ac-97d8-ea1eec95ae25@lab.ntt.co.jp

6 years agoFix typos
Peter Eisentraut [Tue, 10 Jul 2018 09:14:53 +0000 (11:14 +0200)]
Fix typos

6 years agoAvoid emitting a bogus WAL record when recycling an all-zero btree page.
Tom Lane [Mon, 9 Jul 2018 23:26:19 +0000 (19:26 -0400)]
Avoid emitting a bogus WAL record when recycling an all-zero btree page.

Commit fafa374f2 caused _bt_getbuf() to possibly emit a WAL record for
a page that it was about to recycle.  However, it failed to distinguish
all-zero pages from dead pages, which is important because only the
latter have valid btpo.xact values, or indeed any special space at all.
Recycling an all-zero page with XLogStandbyInfoActive() enabled therefore
led to an Assert failure, or to emission of a WAL record containing a
bogus cutoff XID, which might lead to unnecessary query cancellations
on hot standby servers.

Per reports from Antonin Houska and 自己.  Amit Kapila was first to
propose this fix, and Robert Haas, myself, and Kyotaro Horiguchi
reviewed it at various times.

This is an old bug, so back-patch to all supported branches.

Discussion: https://postgr.es/m/2628.1474272158@localhost
Discussion: https://postgr.es/m/48875502.f4a0.1635f0c27b0[email protected]

6 years agoPrevent accidental linking of system-supplied copies of libpq.so etc.
Tom Lane [Mon, 9 Jul 2018 21:23:31 +0000 (17:23 -0400)]
Prevent accidental linking of system-supplied copies of libpq.so etc.

Back-patch commit dddfc4cb2, which broke LDFLAGS and related Makefile
variables into two parts, one for within-build-tree library references and
one for external libraries, to ensure that the order of -L flags has all
of the former before all of the latter.  This turns out to fix a problem
recently noted on buildfarm member peripatus, that we attempted to
incorporate code from libpgport.a into a shared library.  That will fail on
platforms that are sticky about putting non-PIC code into shared libraries.
(It's quite surprising we hadn't seen such failures before, since the code
in question has been like that for a long time.)

I think that peripatus' problem could have been fixed with just a subset
of this patch; but since the previous issue of accidentally linking to the
wrong copy of a Postgres shlib seems likely to bite people in the field,
let's just back-patch the whole change.  Now that commit dddfc4cb2 has
survived some beta testing, I'm less afraid to back-patch it than I was
at the time.

This also fixes undesired inclusion of "-DFRONTEND" in pg_config's CPPFLAGS
output (in 9.6 and up) and undesired inclusion of "-L../../src/common" in
its LDFLAGS output (in all supported branches).

Back-patch to v10 and older branches; this is already in v11.

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

6 years agorel notes: mention enabling of parallelism in PG 10
Bruce Momjian [Mon, 9 Jul 2018 15:19:18 +0000 (11:19 -0400)]
rel notes:  mention enabling of parallelism in PG 10

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20180525010025[email protected]

Backpatch-through: 10

6 years agoRework order of end-of-recovery actions to delay timeline history write
Michael Paquier [Mon, 9 Jul 2018 01:26:18 +0000 (10:26 +0900)]
Rework order of end-of-recovery actions to delay timeline history write

A critical failure in some of the end-of-recovery actions before the
end-of-recovery record is written can cause PostgreSQL to react
inconsistently with the rest of the cluster in the event of a crash
before the final record is written.  Two such failures are for example
an error while processing a two-phase state files or when operating on
recovery.conf.  With this commit, the failures are still considered
FATAL, but the write of the timeline history file is delayed as much as
possible so as the window between the moment the file is written and the
end-of-recovery record is generated gets minimized. This way, in the
event of a crash or a failure, the new timeline decided at promotion
will not seem taken by other nodes in the cluster.  It is not really
possible to reduce to zero this window, hence one could still see
failures if a crash happens between the history file write and the
end-of-recovery record, so any future code should be careful when
adding new end-of-recovery actions.  The original report from Magnus
Hagander mentioned a renamed recovery.conf as original end-of-recovery
failure which caused a timeline to be seen as taken but the subsequent
processing on the now-missing recovery.conf cause the startup process to
issue stop on FATAL, which at follow-up startup made the system
inconsistent because of on-disk changes which already happened.

Processing of two-phase state files still needs some work as corrupted
entries are simply ignored now.  This is left as a future item and this
commit fixes the original complain.

Reported-by: Magnus Hagander
Author: Heikki Linnakangas
Reviewed-by: Alexander Korotkov, Michael Paquier, David Steele
Discussion: https://postgr.es/m/CABUevEz09XY2EevA2dLjPCY-C5UO4Hq=XxmXLmF6ipNFecbShQ@mail.gmail.com

6 years agoAdd note in pg_rewind documentation about read-only files
Michael Paquier [Fri, 6 Jul 2018 23:10:39 +0000 (08:10 +0900)]
Add note in pg_rewind documentation about read-only files

When performing pg_rewind, the presence of a read-only file which is not
accessible for writes will cause a failure while processing.  This can
cause the control file of the target data folder to be truncated,
causing it to not be reusable with a successive run.

Also, when pg_rewind fails mid-flight, there is likely no way to be able
to recover the target data folder anyway, in which case a new base
backup is the best option.  A note is added in the documentation as
well about.

Reported-by: Christian H.
Author: Michael Paquier
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/20180104200633.17004.16377%40wrigleys.postgresql.org

6 years agoAllow replication slots to be dropped in single-user mode
Alvaro Herrera [Fri, 6 Jul 2018 20:38:29 +0000 (16:38 -0400)]
Allow replication slots to be dropped in single-user mode

Starting with commit 9915de6c1cb2, replication slot drop uses a
condition variable sleep to wait until the current user of the slot goes
away.  This is more user friendly than the previous behavior of erroring
out if the slot is in use, but it fails with a not-for-user-consumption
error message in single-user mode; plus, if you're using single-user
mode because you don't want to start the server in the regular mode
(say, disk is full and WAL won't recycle because of the slot), it's
inconvenient.

Fix by skipping the cond variable sleep in single-user mode, since
there can't be anybody to wait for anyway.

Reported-by: tushar
Author: Álvaro Herrera 
Discussion: https://postgr.es/m/3b2f809f-326c-38dd-7a9e-897f957a4eb1@enterprisedb.com

6 years agological decoding: beware of an unset specinsert change
Alvaro Herrera [Thu, 5 Jul 2018 21:42:37 +0000 (17:42 -0400)]
logical decoding: beware of an unset specinsert change

Coverity complains that there is no protection in the code (at least in
non-assertion-enabled builds) against speculative insertion failing to
follow the expected protocol.  Add an elog(ERROR) for the case.

6 years agoReduce cost of test_decoding's new oldest_xmin test
Alvaro Herrera [Thu, 5 Jul 2018 20:37:32 +0000 (16:37 -0400)]
Reduce cost of test_decoding's new oldest_xmin test

Change a whole-database VACUUM into doing just pg_attribute, which is
the portion that verifies what we want it to do.  The original
formulation wastes a lot of CPU time, which leads the test to fail when
runtime exceeds isolationtester timeout when it's super-slow, such as
under CLOBBER_CACHE_ALWAYS.  Per buildfarm member friarbird.

It turns out that the previous shape of the test doesn't always detect
the condition it is supposed to detect (on unpatched reorderbuffer
code): the reason is that there is a good chance of encountering a
xl_running_xacts record (logged every 15 seconds) before the checkpoint
-- and because we advance the xmin when we receive that WAL record, and
we *don't* advance the xmin twice consecutively without receiving a
client message in between, that means the xmin is not advanced enough
for the tuple to be pruned from pg_attribute by VACUUM.  So the test
would spuriously pass.

The reason this test deficiency wasn't detected earlier is that HOT
pruning removes the tuple anyway, even if vacuum leaves it in place, so
the test correctly fails (detecting the coding mistake), but for the
wrong reason.

To fix this mess, run the s0_get_changes step twice before vacuum
instead of once: this seems to cause the xmin to be advanced reliably,
wreaking havoc with more certainty.

Author: Arseny Sher
Discussion: https://postgr.es/m/87h8lkuxoa.fsf@ars-thinkpad

6 years agodoc: Fix typos
Peter Eisentraut [Thu, 5 Jul 2018 20:52:19 +0000 (22:52 +0200)]
doc: Fix typos

Author: Justin Pryzby 

6 years agoFix typo
Peter Eisentraut [Tue, 3 Jul 2018 18:18:56 +0000 (20:18 +0200)]
Fix typo

6 years agodoc: Improve wording and fix whitespace
Peter Eisentraut [Wed, 27 Jun 2018 05:51:20 +0000 (07:51 +0200)]
doc: Improve wording and fix whitespace

6 years agoPrevent references to invalid relation pages after fresh promotion
Michael Paquier [Thu, 5 Jul 2018 01:47:01 +0000 (10:47 +0900)]
Prevent references to invalid relation pages after fresh promotion

If a standby crashes after promotion before having completed its first
post-recovery checkpoint, then the minimal recovery point which marks
the LSN position where the cluster is able to reach consistency may be
set to a position older than the first end-of-recovery checkpoint while
all the WAL available should be replayed.  This leads to the instance
thinking that it contains inconsistent pages, causing a PANIC and a hard
instance crash even if all the WAL available has not been replayed for
certain sets of records replayed.  When in crash recovery,
minRecoveryPoint is expected to always be set to InvalidXLogRecPtr,
which forces the recovery to replay all the WAL available, so this
commit makes sure that the local copy of minRecoveryPoint from the
control file is initialized properly and stays as it is while crash
recovery is performed.  Once switching to archive recovery or if crash
recovery finishes, then the local copy minRecoveryPoint can be safely
updated.

Pavan Deolasee has reported and diagnosed the failure in the first
place, and the base fix idea to rely on the local copy of
minRecoveryPoint comes from Kyotaro Horiguchi, which has been expanded
into a full-fledged patch by me.  The test included in this commit has
been written by Álvaro Herrera and Pavan Deolasee, which I have modified
to make it faster and more reliable with sleep phases.

Backpatch down to all supported versions where the bug appears, aka 9.3
which is where the end-of-recovery checkpoint is not run by the startup
process anymore.  The test gets easily supported down to 10, still it
has been tested on all branches.

Reported-by: Pavan Deolasee
Diagnosed-by: Pavan Deolasee
Reviewed-by: Pavan Deolasee, Kyotaro Horiguchi
Author: Michael Paquier, Kyotaro Horiguchi, Pavan Deolasee, Álvaro
Herrera
Discussion: https://postgr.es/m/CABOikdPOewjNL=05K5CbNMxnNtXnQjhTx2F--4p4ruorCjukbA@mail.gmail.com

6 years agoCheck for interrupts inside the nbtree page deletion code.
Andres Freund [Wed, 4 Jul 2018 21:53:30 +0000 (14:53 -0700)]
Check for interrupts inside the nbtree page deletion code.

When deleting pages the nbtree code has to walk through siblings of a
tree node. When those sibling links are corrupted that can lead to
endless loops - which are currently not interruptible.  This is
especially problematic if autovacuum is repeatedly blocked on such
indexes, as it can be hard to get out of that situation without
resorting to single user mode.

Thus add interrupt checks to appropriate places in such
loops. Unfortunately in one of the cases it's it's not easy to do so.

Between 9.3 and 9.4 the page deletion (and page split) code changed
significantly. Before it was significantly less robust against
interruptions. Therefore don't backpatch to 9.3.

Author: Andres Freund
Discussion: https://postgr.es/m/20180627191629[email protected]
Backpatch: 9.4-

6 years agoImprove the performance of relation deletes during recovery.
Fujii Masao [Wed, 4 Jul 2018 17:21:15 +0000 (02:21 +0900)]
Improve the performance of relation deletes during recovery.

When multiple relations are deleted at the same transaction,
the files of those relations are deleted by one call to smgrdounlinkall(),
which leads to scan whole shared_buffers only one time. OTOH,
previously, during recovery, smgrdounlink() (not smgrdounlinkall()) was
called for each file to delete, which led to scan shared_buffers
multiple times. Obviously this could cause to increase the WAL replay
time very much especially when shared_buffers was huge.

To alleviate this situation, this commit changes the recovery so that
it also calls smgrdounlinkall() only one time to delete multiple
relation files.

This is just fix for oversight of commit 279628a0a7, not new feature.
So, per discussion on pgsql-hackers, we concluded to backpatch this
to all supported versions.

Author: Fujii Masao
Reviewed-by: Michael Paquier, Andres Freund, Thomas Munro, Kyotaro Horiguchi, Takayuki Tsunakawa
Discussion: https://postgr.es/m/CAHGQGwHVQkdfDqtvGVkty+19cQakAydXn1etGND3X0PHbZ3+6w@mail.gmail.com

6 years agoFix libpq example programs
Peter Eisentraut [Sun, 1 Jul 2018 12:06:40 +0000 (14:06 +0200)]
Fix libpq example programs

When these programs call pg_catalog.set_config, they need to check for
PGRES_TUPLES_OK instead of PGRES_COMMAND_OK.  Fix for
5770172cb0c9df9e6ce27c507b449557e5b45124.

Reported-by: Ideriha, Takeshi
6 years agoReplace search.cpan.org with metacpan.org
Michael Paquier [Fri, 29 Jun 2018 13:17:37 +0000 (22:17 +0900)]
Replace search.cpan.org with metacpan.org

search.cpan.org has been EOL'd, with metacpan.org being the official
replacement to which URLs now redirect.  Update links to match the new
URL. Also update links to CPAN to use https as it will redirect from
http.

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/B74C0219-6BA9-46E1-A524-5B9E8CD3BDB3@yesql.se

7 years agodoc: backpatch mention use of cross platform logical replication
Bruce Momjian [Thu, 28 Jun 2018 13:12:07 +0000 (09:12 -0400)]
doc: backpatch mention use of cross platform logical replication

Backpatch 21c1f0c607f0344ae8f71ecaae1fe6f58cf7ff9a to PG 10 docs.

Reported-by: Haribabu Kommi
Discussion: https://postgr.es/m/CAJrrPGfdknoqZcMipPy8XnH3hO3uRic6JTD=jv35oj1DWqL07g@mail.gmail.com

Backpatch-through: only 10

7 years agoFix thinko in comments.
Amit Kapila [Wed, 27 Jun 2018 12:59:42 +0000 (18:29 +0530)]
Fix thinko in comments.

A slot can not be stored in a tuple but it's vice versa.

Reported-by: Ashutosh Bapat
Author: Ashutosh Bapat
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAFjFpRcHhNhXdegyJv3KKDWrwO1_NB_KYZM_ZSDeMOZaL1A5jQ@mail.gmail.com