Noah Misch [Tue, 13 Apr 2021 02:24:41 +0000 (19:24 -0700)]
Use "-I." in directories holding Bison parsers, for Oracle compilers.
With the Oracle Developer Studio 12.6 compiler, #line directives alter
the current source file location for purposes of #include "..."
directives. Hence, a VPATH build failed with 'cannot find include file:
"specscanner.c"'. With two exceptions, parser-containing directories
already add "-I. -I$(srcdir)"; eliminate the exceptions. Back-patch to
9.6 (all supported versions).
Noah Misch [Tue, 13 Apr 2021 02:24:21 +0000 (19:24 -0700)]
Port regress-python3-mangle.mk to Solaris "sed".
It doesn't support "\(foo\)*" like a POSIX "sed" implementation does;
see the Autoconf manual. Back-patch to 9.6 (all supported versions).
Tom Lane [Mon, 12 Apr 2021 18:37:22 +0000 (14:37 -0400)]
Fix old bug with coercing the result of a COLLATE expression.
There are hacks in parse_coerce.c to push down a requested coercion
to below any CollateExpr that may appear. However, we did that even
if the requested data type is non-collatable, leading to an invalid
expression tree in which CollateExpr is applied to a non-collatable
type. The fix is just to drop the CollateExpr altogether, reasoning
that it's useless.
This bug is ten years old, dating to the original addition of
COLLATE support. The lack of field complaints suggests that there
aren't a lot of user-visible consequences. We noticed the problem
because it would trigger an assertion in DefineVirtualRelation if
the invalid structure appears as an output column of a view; however,
in a non-assert build, you don't see a crash just a (subtly incorrect)
complaint about applying collation to a non-collatable type. I found
that by putting the incorrect structure further down in a view, I could
make a view definition that would fail dump/reload, per the added
regression test case. But CollateExpr doesn't do anything at run-time,
so this likely doesn't lead to any really exciting consequences.
Per report from Yulin Pei. Back-patch to all supported branches.
Discussion: https://postgr.es/m/HK0PR01MB22744393C474D503E16C8509F4709@HK0PR01MB2274.apcprd01.prod.exchangelabs.com
Michael Paquier [Mon, 12 Apr 2021 02:31:46 +0000 (11:31 +0900)]
Fix out-of-bound memory access for interval -> char conversion
Using Roman numbers (via "RM" or "rm") for a conversion to calculate a
number of months has never considered the case of negative numbers,
where a conversion could easily cause out-of-bound memory accesses. The
conversions in themselves were not completely consistent either, as
specifying 12 would result in NULL, but it should mean XII.
This commit reworks the conversion calculation to have a more
consistent behavior:
- If the number of months and years is 0, return NULL.
- If the number of months is positive, return the exact month number.
- If the number of months is negative, do a backward calculation, with
-1 meaning December, -2 November, etc.
Reported-by: Theodor Arsenij Larionov-Trichkin
Author: Julien Rouhaud
Discussion: https://postgr.es/m/16953-
f255a18f8c51f1d5@postgresql.org
backpatch-through: 9.6
Magnus Hagander [Fri, 9 Apr 2021 10:40:14 +0000 (12:40 +0200)]
Fix typo
Author: Daniel Westermann
Backpatch-through: 9.6
Discussion: https://postgr.es/m/GV0P278MB0483A7AA85BAFCC06D90F453D2739@GV0P278MB0483.CHEP278.PROD.OUTLOOK.COM
Michael Paquier [Fri, 9 Apr 2021 04:53:38 +0000 (13:53 +0900)]
Fix typos and grammar in documentation and code comments
Comment fixes are applied on HEAD, and documentation improvements are
applied on back-branches where needed.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20210408164008.GJ6592@telsasoft.com
Backpatch-through: 9.6
Tomas Vondra [Wed, 7 Apr 2021 13:58:35 +0000 (15:58 +0200)]
Don't add non-existent pages to bitmap from BRIN
The code in bringetbitmap() simply added the whole matching page range
to the TID bitmap, as determined by pages_per_range, even if some of the
pages were beyond the end of the heap. The query then might fail with
an error like this:
ERROR: could not open file "base/20176/20228.2" (target block
262144): previous segment is only 131021 blocks
In this case, the relation has 262093 pages (131072 and 131021 pages),
but we're trying to acess block 262144, i.e. first block of the 3rd
segment. At that point _mdfd_getseg() notices the preceding segment is
incomplete, and fails.
Hitting this in practice is rather unlikely, because:
* Most indexes use power-of-two ranges, so segments and page ranges
align perfectly (segment end is also a page range end).
* The table size has to be just right, with the last segment being
almost full - less than one page range from full segment, so that the
last page range actually crosses the segment boundary.
* Prefetch has to be enabled. The regular page access checks that
pages are not beyond heap end, but prefetch does not. On older
releases (before 12) the execution stops after hitting the first
non-existent page, so the prefetch distance has to be sufficient
to reach the first page in the next segment to trigger the issue.
Since 12 it's enough to just have prefetch enabled, the prefetch
distance does not matter.
Fixed by not adding non-existent pages to the TID bitmap. Backpatch
all the way back to 9.6 (BRIN indexes were introduced in 9.5, but that
release is EOL).
Backpatch-through: 9.6
Fujii Masao [Mon, 5 Apr 2021 17:25:37 +0000 (02:25 +0900)]
Shut down transaction tracking at startup process exit.
Maxim Orlov reported that the shutdown of standby server could result in
the following assertion failure. The cause of this issue was that,
when the shutdown caused the startup process to exit, recovery-time
transaction tracking was not shut down even if it's already initialized,
and some locks the tracked transactions were holding could not be released.
At this situation, if other process was invoked and the PGPROC entry that
the startup process used was assigned to it, it found such unreleased locks
and caused the assertion failure, during the initialization of it.
TRAP: FailedAssertion("SHMQueueEmpty(&(MyProc->myProcLocks[i]))"
This commit fixes this issue by making the startup process shut down
transaction tracking and release all locks, at the exit of it.
Back-patch to all supported branches.
Reported-by: Maxim Orlov
Author: Fujii Masao
Reviewed-by: Maxim Orlov
Discussion: https://postgr.es/m/
ad4ce692cc1d89a093b471ab1d969b0b@postgrespro.ru
Bruce Momjian [Fri, 2 Apr 2021 20:42:29 +0000 (16:42 -0400)]
Use macro MONTHS_PER_YEAR instead of '12' in /ecpg/pgtypeslib
All other places already use MONTHS_PER_YEAR appropriately.
Backpatch-through: 9.6
Joe Conway [Fri, 2 Apr 2021 17:48:56 +0000 (13:48 -0400)]
Clarify documentation of RESET ROLE
Command-line options, or previous "ALTER (ROLE|DATABASE) ...
SET ROLE ..." commands, can change the value of the default role
for a session. In the presence of one of these, RESET ROLE will
change the current user identifier to the default role rather
than the session user identifier. Fix the documentation to
reflect this reality. Backpatch to all supported versions.
Author: Nathan Bossart
Reviewed-By: Laurenz Albe, David G. Johnston, Joe Conway
Reported by: Nathan Bossart
Discussion: https://postgr.es/m/flat/
925134DB-8212-4F60-8AB1-
B1231D750CB4%40amazon.com
Backpatch-through: 9.6
Michael Paquier [Fri, 2 Apr 2021 07:37:28 +0000 (16:37 +0900)]
doc: Clarify how to generate backup files with non-exclusive backups
The current instructions describing how to write the backup_label and
tablespace_map files are confusing. For example, opening a file in text
mode on Windows and copy-pasting the file's contents would result in a
failure at recovery because of the extra CRLF characters generated. The
documentation was not stating that clearly, and per discussion this is
not considered as a supported scenario.
This commit extends a bit the documentation to mention that it may be
required to open the file in binary mode before writing its data.
Reported-by: Wang Shenhao
Author: David Steele
Reviewed-by: Andrew Dunstan, Magnus Hagander
Discussion: https://postgr.es/m/
8373f61426074f2cb6be92e02f838389@G08CNEXMBPEKD06.g08.fujitsu.local
Backpatch-through: 9.6
Bruce Momjian [Fri, 2 Apr 2021 01:17:24 +0000 (21:17 -0400)]
doc: mention that intervening major releases can be skipped
Also mention that you should read the intervening major releases notes.
This change was also applied to the website.
Discussion: https://postgr.es/m/
20210330144949.GA8259@momjian.us
Backpatch-through: 9.6
Tom Lane [Thu, 1 Apr 2021 17:34:16 +0000 (13:34 -0400)]
Fix pg_restore's misdesigned code for detecting archive file format.
Despite the clear comments pointing out that the duplicative code
segments in ReadHead() and _discoverArchiveFormat() needed to be
in sync, they were not: the latter did not bother to apply any of
the sanity checks in the former. We'd missed noticing this partly
because none of those checks would fail in scenarios we customarily
test, and partly because the oversight would be masked if both
segments execute, which they would in cases other than needing to
autodetect the format of a non-seekable stdin source. However,
in a case meeting all these requirements --- for example, trying
to read a newer-than-supported archive format from non-seekable
stdin --- pg_restore missed applying the version check and would
likely dump core or otherwise misbehave.
The whole thing is silly anyway, because there seems little reason
to duplicate the logic beyond the one-line verification that the
file starts with "PGDMP". There seems to have been an undocumented
assumption that multiple major formats (major enough to require
separate reader modules) would nonetheless share the first half-dozen
fields of the custom-format header. This seems unlikely, so let's
fix it by just nuking the duplicate logic in _discoverArchiveFormat().
Also get rid of the pointless attempt to seek back to the start of
the file after successful autodetection. That wastes cycles and
it means we have four behaviors to verify not two.
Per bug #16951 from Sergey Koposov. This has been broken for
decades, so back-patch to all supported versions.
Discussion: https://postgr.es/m/16951-
a4dd68cf0de23048@postgresql.org
Michael Paquier [Thu, 1 Apr 2021 06:29:12 +0000 (15:29 +0900)]
doc: Clarify use of ACCESS EXCLUSIVE lock in various sections
Some sections of the documentation used "exclusive lock" to describe
that an ACCESS EXCLUSIVE lock is taken during a given operation. This
can be confusing to the reader as ACCESS SHARE is allowed with an
EXCLUSIVE lock is used, but that would not be the case with what is
described on those parts of the documentation.
Author: Greg Rychlewski
Discussion: https://postgr.es/m/CAKemG7VptD=7fNWckFMsMVZL_zzvgDO6v2yVmQ+ZiBfc_06kCQ@mail.gmail.com
Backpatch-through: 9.6
Etsuro Fujita [Tue, 30 Mar 2021 04:00:08 +0000 (13:00 +0900)]
Update obsolete comment.
Back-patch to all supported branches.
Author: Etsuro Fujita
Discussion: https://postgr.es/m/CAPmGK17DwzaSf%2BB71dhL2apXdtG-OmD6u2AL9Cq2ZmAR0%2BzapQ%40mail.gmail.com
Stephen Frost [Sun, 28 Mar 2021 15:28:22 +0000 (11:28 -0400)]
doc: Define TLS as an acronym
Commit
c6763156589 added an acronym reference for "TLS" but the definition
was never added.
Author: Daniel Gustafsson
Reviewed-by: Michael Paquier
Backpatch-through: 9.6
Discussion: https://postgr.es/m/
27109504-82DB-41A8-8E63-
C0498314F5B0@yesql.se
Fujii Masao [Thu, 25 Mar 2021 02:23:30 +0000 (11:23 +0900)]
Fix bug in WAL replay of COMMIT_TS_SETTS record.
Previously the WAL replay of COMMIT_TS_SETTS record called
TransactionTreeSetCommitTsData() with the argument write_xlog=true,
which generated and wrote new COMMIT_TS_SETTS record.
This should not be acceptable because it's during recovery.
This commit fixes the WAL replay of COMMIT_TS_SETTS record
so that it calls TransactionTreeSetCommitTsData() with write_xlog=false
and doesn't generate new WAL during recovery.
Back-patch to all supported branches.
Reported-by: lx zou <zoulx1982@163.com>
Author: Fujii Masao
Reviewed-by: Alvaro Herrera
Discussion: https://postgr.es/m/16931-
620d0f2fdc6108f1@postgresql.org
Tom Lane [Tue, 23 Mar 2021 18:27:50 +0000 (14:27 -0400)]
Fix psql's \connect command some more.
Jasen Betts reported yet another unintended side effect of commit
85c54287a: reconnecting with "\c service=whatever" did not have the
expected results. The reason is that starting from the output of
PQconndefaults() effectively allows environment variables (such
as PGPORT) to override entries in the service file, whereas the
normal priority is the other way around.
Not using PQconndefaults at all would require yet a third main code
path in do_connect's parameter setup, so I don't really want to fix
it that way. But we can have the logic effectively ignore all the
default values for just a couple more lines of code.
This patch doesn't change the behavior for "\c -reuse-previous=on
service=whatever". That remains significantly different from before
85c54287a, because many more parameters will be re-used, and thus
not be possible for service entries to replace. But I think this
is (mostly?) intentional. In any case, since libpq does not report
where it got parameter values from, it's hard to do differently.
Per bug #16936 from Jasen Betts. As with the previous patches,
back-patch to all supported branches. (9.5 is unfortunately now
out of support, so this won't get fixed there.)
Discussion: https://postgr.es/m/16936-
3f524322a53a29f0@postgresql.org
Fujii Masao [Tue, 23 Mar 2021 00:53:08 +0000 (09:53 +0900)]
pg_waldump: Fix bug in per-record statistics.
pg_waldump --stats=record identifies a record by a combination
of the RmgrId and the four bits of the xl_info field of the record.
But XACT records use the first bit of those four bits for an optional
flag variable, and the following three bits for the opcode to
identify a record. So previously the same type of XACT record
could have different four bits (three bits are the same but the
first one bit is different), and which could cause
pg_waldump --stats=record to show two lines of per-record statistics
for the same XACT record. This is a bug.
This commit changes pg_waldump --stats=record so that it processes
only XACT record differently, i.e., filters the opcode out of xl_info
and uses a combination of the RmgrId and those three bits as
the identifier of a record, only for XACT record. For other records,
the four bits of the xl_info field are still used.
Back-patch to all supported branches.
Author: Kyotaro Horiguchi
Reviewed-by: Shinya Kato, Fujii Masao
Discussion: https://postgr.es/m/
2020100913412132258847@highgo.ca
Tom Lane [Fri, 19 Mar 2021 02:09:41 +0000 (22:09 -0400)]
Don't leak malloc'd strings when a GUC setting is rejected.
Because guc.c prefers to keep all its string values in malloc'd
not palloc'd storage, it has to be more careful than usual to
avoid leaks. Error exits out of string GUC hook checks failed
to clear the proposed value string, and error exits out of
ProcessGUCArray() failed to clear the malloc'd results of
ParseLongOption().
Found via valgrind testing.
This problem is ancient, so back-patch to all supported branches.
Discussion: https://postgr.es/m/
3816764.
1616104288@sss.pgh.pa.us
Tom Lane [Fri, 19 Mar 2021 01:44:43 +0000 (21:44 -0400)]
Don't leak compiled regex(es) when an ispell cache entry is dropped.
The text search cache mechanisms assume that we can clean up
an invalidated dictionary cache entry simply by resetting the
associated long-lived memory context. However, that does not work
for ispell affixes that make use of regular expressions, because
the regex library deals in plain old malloc. Hence, we leaked
compiled regex(es) any time we dropped such a cache entry. That
could quickly add up, since even a fairly trivial regex can use up
tens of kB, and a large one can eat megabytes. Add a memory context
callback to ensure that a regex gets freed when its owning cache
entry is cleared.
Found via valgrind testing.
This problem is ancient, so back-patch to all supported branches.
Discussion: https://postgr.es/m/
3816764.
1616104288@sss.pgh.pa.us
Tom Lane [Wed, 17 Mar 2021 20:10:38 +0000 (16:10 -0400)]
Prevent buffer overrun in read_tablespace_map().
Robert Foggia of Trustwave reported that read_tablespace_map()
fails to prevent an overrun of its on-stack input buffer.
Since the tablespace map file is presumed trustworthy, this does
not seem like an interesting security vulnerability, but still
we should fix it just in the name of robustness.
While here, document that pg_basebackup's --tablespace-mapping option
doesn't work with tar-format output, because it doesn't. To make it
work, we'd have to modify the tablespace_map file within the tarball
sent by the server, which might be possible but I'm not volunteering.
(Less-painful solutions would require changing the basebackup protocol
so that the source server could adjust the map. That's not very
appetizing either.)
Tom Lane [Fri, 12 Mar 2021 17:20:15 +0000 (12:20 -0500)]
Fix race condition in psql \e's detection of file modification.
psql's editing commands decide whether the user has edited the file
by checking for change of modification timestamp. This is probably
fine for a pre-existing file, but with a temporary file that is
created within the command, it's possible for a fast typist to
save-and-exit in less than the one-second granularity of stat(2)
timestamps. On Windows FAT filesystems the granularity is even
worse, 2 seconds, making the race a bit easier to hit.
To fix, try to set the temp file's mod time to be two seconds ago.
It's unlikely this would fail, but then again the race condition
itself is unlikely, so just ignore any error.
Also, we might as well check the file size as well as its mod time.
While this is a difficult bug to hit, it still seems worth
back-patching, to ensure that users' edits aren't lost.
Laurenz Albe, per gripe from Jacob Champion; based on fix suggestions
from Jacob and myself
Discussion: https://postgr.es/m/
0ba3f2a658bac6546d9934ab6ba63a805d46a49b.camel@cybertec.at
Tom Lane [Thu, 11 Mar 2021 19:43:45 +0000 (14:43 -0500)]
Re-simplify management of inStart in pqParseInput3's subroutines.
Commit
92785dac2 copied some logic related to advancement of inStart
from pqParseInput3 into getRowDescriptions and getAnotherTuple,
because it wanted to allow user-defined row processor callbacks to
potentially longjmp out of the library, and inStart would have to be
updated before that happened to avoid an infinite loop. We later
decided that that API was impossibly fragile and reverted it, but
we didn't undo all of the related code changes, and this bit of
messiness survived. Undo it now so that there's just one place in
pqParseInput3's processing where inStart is advanced; this will
simplify addition of better tracing support.
getParamDescriptions had grown similar processing somewhere along
the way (not in
92785dac2; I didn't track down just when), but it's
actually buggy because its handling of corrupt-message cases seems to
have been copied from the v2 logic where we lacked a known message
length. The cases where we "goto not_enough_data" should not simply
return EOF, because then we won't consume the message, potentially
creating an infinite loop. That situation now represents a
definitively corrupt message, and we should report it as such.
Although no field reports of getParamDescriptions getting stuck in
a loop have been seen, it seems appropriate to back-patch that fix.
I chose to back-patch all of this to keep the logic looking more alike
in supported branches.
Discussion: https://postgr.es/m/
2217283.
1615411989@sss.pgh.pa.us
Bruce Momjian [Thu, 11 Mar 2021 01:25:18 +0000 (20:25 -0500)]
tutorial: land height is "elevation", not "altitude"
This is a follow-on patch to
92c12e46d5. In that patch, we renamed
"altitude" to "elevation" in the docs, based on these details:
https://mapscaping.com/blogs/geo-candy/what-is-the-difference-between-elevation-relief-and-altitude
This renames the tutorial SQL files to match the documentation.
Reported-by: max1@inbox.ru
Discussion: https://postgr.es/m/
161512392887.1046.
3137472627109459518@wrigleys.postgresql.org
Backpatch-through: 9.6
Michael Paquier [Wed, 24 Feb 2021 07:14:13 +0000 (16:14 +0900)]
Fix some typos, grammar and style in docs and comments
The portions fixing the documentation are backpatched where needed.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20210210235557.GQ20012@telsasoft.com
backpatch-through: 9.6
Alvaro Herrera [Tue, 23 Feb 2021 20:30:21 +0000 (17:30 -0300)]
Reinstate HEAP_XMAX_LOCK_ONLY|HEAP_KEYS_UPDATED as allowed
Commit
866e24d47db1 added an assert that HEAP_XMAX_LOCK_ONLY and
HEAP_KEYS_UPDATED cannot appear together, on the faulty assumption that
the latter necessarily referred to an update and not a tuple lock; but
that's wrong, because SELECT FOR UPDATE can use precisely that
combination, as evidenced by the amcheck test case added here.
Remove the Assert(), and also patch amcheck's verify_heapam.c to not
complain if the combination is found. Also, out of overabundance of
caution, update (across all branches) README.tuplock to be more explicit
about this.
Author: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Mahendra Singh Thalor <mahi6run@gmail.com>
Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com>
Discussion: https://postgr.es/m/
20210124061758.GA11756@nol
Tom Lane [Fri, 19 Feb 2021 03:38:55 +0000 (22:38 -0500)]
Fix another ancient bug in parsing of BRE-mode regular expressions.
While poking at the regex code, I happened to notice that the bug
squashed in commit
afcc8772e had a sibling: next() failed to return
a specific value associated with the '}' token for a "\{m,n\}"
quantifier when parsing in basic RE mode. Again, this could result
in treating the quantifier as non-greedy, which it never should be in
basic mode. For that to happen, the last character before "\}" that
sets "nextvalue" would have to set it to zero, or it'd have to have
accidentally been zero from the start. The failure can be provoked
repeatably with, for example, a bound ending in digit "0".
Like the previous patch, back-patch all the way.
Thomas Munro [Tue, 16 Feb 2021 00:10:37 +0000 (13:10 +1300)]
Fix compiler warning in back branches (9.6, 10).
Back-patch a tiny bit of commit
fbb2e9a0 into 9.6 and 10, to silence an
uninitialized variable warning from GCC 10.2. Seen on buildfarm member
handfish, and my own development workflow where I like to use -Werror.
Discussion: https://postgr.es/m/CA%2BhUKGJRcwvK86Uf5t-FrTekZjqHtpv3u%3D3MuBg8Zw8R933Mqg%40mail.gmail.com
Thomas Munro [Mon, 15 Feb 2021 02:43:39 +0000 (15:43 +1300)]
Default to wal_sync_method=fdatasync on FreeBSD.
FreeBSD 13 gained O_DSYNC, which would normally cause wal_sync_method to
choose open_datasync as its default value. That may not be a good
choice for all systems, and performs worse than fdatasync in some
scenarios. Let's preserve the existing default behavior for now.
Like commit
576477e73c4, which did the same for Linux, back-patch to all
supported releases.
Discussion: https://postgr.es/m/CA%2BhUKGLsAMXBQrCxCXoW-JsUYmdOL8ALYvaX%3DCrHqWxm-nWbGA%40mail.gmail.com
Thomas Munro [Mon, 15 Feb 2021 00:32:58 +0000 (13:32 +1300)]
Hold interrupts while running dsm_detach() callbacks.
While cleaning up after a parallel query or parallel index creation that
created temporary files, we could be interrupted by a statement timeout.
The error handling path would then fail to clean up the files when it
ran dsm_detach() again, because the callback was already popped off the
list. Prevent this hazard by holding interrupts while the cleanup code
runs.
Thanks to Heikki Linnakangas for this suggestion, and also to Kyotaro
Horiguchi, Masahiko Sawada, Justin Pryzby and Tom Lane for discussion of
this and earlier ideas on how to fix the problem.
Back-patch to all supported releases.
Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/
20191212180506.GR2082@telsasoft.com
Tom Lane [Sat, 13 Feb 2021 22:49:08 +0000 (17:49 -0500)]
pg_attribute_no_sanitize_alignment() macro
Modern gcc and clang compilers offer alignment sanitizers, which help to detect
pointer misalignment. However, our codebase already contains x86-specific
crc32 computation code, which uses unalignment access. Thankfully, those
compilers also support the attribute, which disables alignment sanitizers at
the function level. This commit adds pg_attribute_no_sanitize_alignment(),
which wraps this attribute, and applies it to pg_comp_crc32c_sse42() function.
Back-patch of commits
993bdb9f9 and
ad2ad698a, to enable doing
alignment testing in all supported branches.
Discussion: https://postgr.es/m/CAPpHfdsne3%3DT%3DfMNU45PtxdhSL_J2PjLTeS8rwKnJzUR4YNd4w%40mail.gmail.com
Discussion: https://postgr.es/m/475514.
1612745257%40sss.pgh.pa.us
Author: Alexander Korotkov, revised by Tom Lane
Reviewed-by: Tom Lane
Tom Lane [Fri, 12 Feb 2021 21:26:47 +0000 (16:26 -0500)]
Avoid divide-by-zero in regex_selectivity() with long fixed prefix.
Given a regex pattern with a very long fixed prefix (approaching 500
characters), the result of pow(FIXED_CHAR_SEL, fixed_prefix_len) can
underflow to zero. Typically the preceding selectivity calculation
would have underflowed as well, so that we compute 0/0 and get NaN.
In released branches this leads to an assertion failure later on.
That doesn't happen in HEAD, for reasons I've not explored yet,
but it's surely still a bug.
To fix, just skip the division when the pow() result is zero, so
that we'll (most likely) return a zero selectivity estimate. In
the edge cases where "sel" didn't yet underflow, perhaps this
isn't desirable, but I'm not sure that the case is worth spending
a lot of effort on. The results of regex_selectivity_sub() are
barely worth the electrons they're written on anyway :-(
Per report from Alexander Lakhin. Back-patch to all supported versions.
Discussion: https://postgr.es/m/
6de0a0c3-ada9-cd0c-3e4e-
2fa9964b41e3@gmail.com
Tom Lane [Mon, 8 Feb 2021 22:01:30 +0000 (17:01 -0500)]
Stamp 9.6.21.
Peter Eisentraut [Mon, 8 Feb 2021 17:10:23 +0000 (18:10 +0100)]
Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash:
c1022f89a3531073349e8954e4f4fae64fe34552
Tom Lane [Sun, 7 Feb 2021 20:46:38 +0000 (15:46 -0500)]
Release notes for 13.2, 12.6, 11.11, 10.16, 9.6.21, 9.5.25.
Tom Lane [Sun, 7 Feb 2021 17:54:08 +0000 (12:54 -0500)]
Revert "Propagate CTE property flags when copying a CTE list into a rule."
This reverts commit
ed290896335414c6c069b9ccae1f3dcdd2fac6ba and
equivalent back-branch commits. The issue is subtler than I thought,
and it's far from new, so just before a release deadline is no time
to be fooling with it. We'll consider what to do at a bit more
leisure.
Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com
Tom Lane [Sun, 7 Feb 2021 00:28:39 +0000 (19:28 -0500)]
Propagate CTE property flags when copying a CTE list into a rule.
rewriteRuleAction() neglected this step, although it was careful to
propagate other similar flags such as hasSubLinks or hasRowSecurity.
Omitting to transfer hasRecursive is just cosmetic at the moment,
but omitting hasModifyingCTE is a live bug, since the executor
certainly looks at that.
The proposed test case only fails back to v10, but since the executor
examines hasModifyingCTE in 9.x as well, I suspect that a test case
could be devised that fails in older branches. Given the nearness
of the release deadline, though, I'm not going to spend time looking
for a better test.
Report and patch by Greg Nancarrow, cosmetic changes by me
Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com
Tom Lane [Sat, 6 Feb 2021 20:17:02 +0000 (15:17 -0500)]
Disallow converting an inheritance child table to a view.
Generally, members of inheritance trees must be plain tables (or,
in more recent versions, foreign tables). ALTER TABLE INHERIT
rejects creating an inheritance relationship that has a view at
either end. When DefineQueryRewrite attempts to convert a relation
to a view, it already had checks prohibiting doing so for partitioning
parents or children as well as traditional-inheritance parents ...
but it neglected to check that a traditional-inheritance child wasn't
being converted. Since the planner assumes that any inheritance
child is a table, this led to making plans that tried to do a physical
scan on a view, causing failures (or even crashes, in recent versions).
One could imagine trying to support such a case by expanding the view
normally, but since the rewriter runs before the planner does
inheritance expansion, it would take some very fundamental refactoring
to make that possible. There are probably a lot of other parts of the
system that don't cope well with such a situation, too. For now,
just forbid it.
Per bug #16856 from Yang Lin. Back-patch to all supported branches.
(In versions before v10, this includes back-patching the portion of
commit
501ed02cf that added has_superclass(). Perhaps the lack of
that infrastructure partially explains the missing check.)
Discussion: https://postgr.es/m/16856-
0363e05c6e1612fd@postgresql.org
Heikki Linnakangas [Fri, 5 Feb 2021 09:14:56 +0000 (11:14 +0200)]
Fix backslash-escaping multibyte chars in COPY FROM.
If a multi-byte character is escaped with a backslash in TEXT mode input,
and the encoding is one of the client-only encodings where the bytes after
the first one can have an ASCII byte "embedded" in the char, we didn't
skip the character correctly. After a backslash, we only skipped the first
byte of the next character, so if it was a multi-byte character, we would
try to process its second byte as if it was a separate character. If it
was one of the characters with special meaning, like '\n', '\r', or
another '\\', that would cause trouble.
One such exmple is the byte sequence '\x5ca45c2e666f6f' in Big5 encoding.
That's supposed to be [backslash][two-byte character][.][f][o][o], but
because the second byte of the two-byte character is 0x5c, we incorrectly
treat it as another backslash. And because the next character is a dot, we
parse it as end-of-copy marker, and throw an "end-of-copy marker corrupt"
error.
Backpatch to all supported versions.
Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/
a897f84f-8dca-8798-3139-
07da5bb38728%40iki.fi
Tom Lane [Tue, 2 Feb 2021 18:49:08 +0000 (13:49 -0500)]
Fix ancient memory leak in contrib/auto_explain.
The ExecutorEnd hook is invoked in a context that could be quite
long-lived, not the executor's own per-query context as I think
we were sort of assuming. Thus, any cruft generated while producing
the EXPLAIN output could accumulate over multiple queries. This can
result in spectacular leakage if log_nested_statements is on, and
even without that I'm surprised nobody complained before.
To fix, just switch into the executor's context so that anything we
allocate will be released when standard_ExecutorEnd frees the executor
state. We might as well nuke the code's retail pfree of the explain
output string, too; that's laughably inadequate to the need.
Japin Li, per report from Jeff Janes. This bug is old, so
back-patch to all supported branches.
Discussion: https://postgr.es/m/CAMkU=1wCVtbeRn0s9gt12KwQ7PLXovbpM8eg25SYocKW3BT4hg@mail.gmail.com
Noah Misch [Sat, 30 Jan 2021 08:00:27 +0000 (00:00 -0800)]
Fix CREATE INDEX CONCURRENTLY for simultaneous prepared transactions.
In a cluster having used CREATE INDEX CONCURRENTLY while having enabled
prepared transactions, queries that use the resulting index can silently
fail to find rows. Fix this for future CREATE INDEX CONCURRENTLY by
making it wait for prepared transactions like it waits for ordinary
transactions. This expands the VirtualTransactionId structure domain to
admit prepared transactions. It may be necessary to reindex to recover
from past occurrences. Back-patch to 9.5 (all supported versions).
Andrey Borodin, reviewed (in earlier versions) by Tom Lane and Michael
Paquier.
Discussion: https://postgr.es/m/
2E712143-97F7-4890-B470-
4A35142ABC82@yandex-team.ru
Tom Lane [Thu, 28 Jan 2021 22:18:23 +0000 (17:18 -0500)]
Silence another gcc 11 warning.
Per buildfarm and local experimentation, bleeding-edge gcc isn't
convinced that the MemSet in reorder_function_arguments() is safe.
Shut it up by adding an explicit check that pronargs isn't negative,
and by changing MemSet to memset. (It appears that either change is
enough to quiet the warning at -O2, but let's do both to be sure.)
Tom Lane [Thu, 28 Jan 2021 16:17:13 +0000 (11:17 -0500)]
Make ecpg's rjulmdy() and rmdyjul() agree with their declarations.
We had "short *mdy" in the extern declarations, but "short mdy[3]"
in the actual function definitions. Per C99 these are equivalent,
but recent versions of gcc have started to issue warnings about
the inconsistency. Clean it up before the warnings get any more
widespread.
Back-patch, in case anyone wants to build older PG versions with
bleeding-edge compilers.
Discussion: https://postgr.es/m/
2401575.
1611764534@sss.pgh.pa.us
Alvaro Herrera [Thu, 28 Jan 2021 15:50:40 +0000 (12:50 -0300)]
pgbench: Remove dead code
doConnect() never returns connections in state CONNECTION_BAD, so
checking for that is pointless. Remove the code that does.
This code has been dead since
ba708ea3dc84, 20 years ago.
Discussion: https://postgr.es/m/
20210126195224.GA20361@alvherre.pgsql
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Alvaro Herrera [Tue, 26 Jan 2021 19:42:13 +0000 (16:42 -0300)]
Report the true database name on connection errors
When reporting connection errors, we might show a database name in the
message that's not the one we actually tried to connect to, if the
database was taken from libpq defaults instead of from user parameters.
Fix such error messages to use PQdb(), which reports the correct name.
(But, per commit
2930c05634bc, make sure not to try to print NULL.)
Apply to branches 9.5 through 13. Branch master has already been
changed differently by commit
58cd8dca3de0.
Reported-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA+TgmobssJ6rS22dspWnu-oDxXevGmhMD8VcRBjmj-b9UDqRjw@mail.gmail.com
Tom Lane [Tue, 26 Jan 2021 18:04:52 +0000 (13:04 -0500)]
Code review for psql's helpSQL() function.
The loops to identify word boundaries could access past the end of
the input string. Likely that would never result in an actual
crash, but it makes valgrind unhappy.
The logic to try different numbers of words didn't work when the
input has two words but we only have a match to the first, eg
"\h with select". (We must "continue" the pass loop, not "break".)
The logic to compute nl_count was bizarrely managed, and in at
least two code paths could end up calling PageOutput with
nl_count = 0, resulting in failing to paginate output that should
have been fed to the pager. Also, in v12 and up, the nl_count
calculation hadn't been updated to account for the addition of a URL.
The PQExpBuffer holding the command syntax details wasn't freed,
resulting in a session-lifespan memory leak.
While here, improve some comments, choose a more descriptive name
for a variable, fix inconsistent datatype choice for another variable.
Per bug #16837 from Alexander Lakhin. This code is very old,
so back-patch to all supported branches.
Kyotaro Horiguchi and Tom Lane
Discussion: https://postgr.es/m/16837-
479bcd56040c71b3@postgresql.org
Tom Lane [Mon, 25 Jan 2021 18:03:12 +0000 (13:03 -0500)]
Fix broken ruleutils support for function TRANSFORM clauses.
I chanced to notice that this dumped core due to a faulty Assert.
To add insult to injury, the output has been misformatted since v11.
Obviously we need some regression testing here.
Discussion: https://postgr.es/m/
d1cc628c-3953-4209-957b-
29427acc38c8@www.fastmail.com
David Rowley [Mon, 25 Jan 2021 07:01:00 +0000 (20:01 +1300)]
Fix hypothetical bug in heap backward scans
Both heapgettup() and heapgettup_pagemode() incorrectly set the first page
to scan in a backward scan in which the number of pages to scan was
specified by heap_setscanlimits(). The code incorrectly started the scan
at the end of the relation when startBlk was 0, or otherwise at
startBlk - 1, neither of which is correct when only scanning a subset of
pages.
The fix here checks if heap_setscanlimits() has changed the number of
pages to scan and if so we set the first page to scan as the final page in
the specified range during backward scans.
Proper adjustment of this code was forgotten when heap_setscanlimits() was
added in
7516f5259 back in 9.5. However, practice, nowhere in core code
performs backward scans after having used heap_setscanlimits(), yet, it is
possible an extension uses the heap functions in this way, hence
backpatch.
An upcoming patch does use heap_setscanlimits() with backward scans, so
this must be fixed before that can go in.
Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvpGc9h0_oVD2CtgBcxCS1N-qDYZSeBRnUh+0CWJA9cMaA@mail.gmail.com
Backpatch-through: 9.5, all supported versions
Tom Lane [Sun, 24 Jan 2021 21:29:48 +0000 (16:29 -0500)]
Update time zone data files to tzdata release 2021a.
DST law changes in Russia (Volgograd zone) and South Sudan.
Historical corrections for Australia, Bahamas, Belize, Bermuda,
Ghana, Israel, Kenya, Nigeria, Palestine, Seychelles, and Vanuatu.
Notably, the Australia/Currie zone has been corrected to the point
where it is identical to Australia/Hobart.
Noah Misch [Sat, 23 Jan 2021 22:09:59 +0000 (14:09 -0800)]
Make check-prepared-txns use temp-install binaries.
It tested the installed binaries. This fixes 9.6 and 9.5 to follow
later versions and "make -C src/test/isolation check" in this respect.
Tom Lane [Fri, 22 Jan 2021 23:58:25 +0000 (18:58 -0500)]
Doc: improve directions for building on macOS.
In light of recent discussions, we should instruct people to
install Apple's command line tools; installing Xcode is secondary.
Also, fix sample command for finding out the default sysroot,
as we now know that the command originally recommended can give
a result that doesn't match your OS version.
Also document the workaround to use if you really don't want
configure to select a sysroot at all.
Discussion: https://postgr.es/m/
20210119111625.20435-1-james.hilliard1@gmail.com
Tom Lane [Wed, 20 Jan 2021 17:07:23 +0000 (12:07 -0500)]
Further tweaking of PG_SYSROOT heuristics for macOS.
It emerges that in some phases of the moon (perhaps to do with
directory entry order?), xcrun will report that the SDK path is
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk
which is normally a symlink to a version-numbered sibling directory.
Our heuristic to skip non-version-numbered pathnames was rejecting
that, which is the wrong thing to do. We'd still like to end up
with a version-numbered PG_SYSROOT value, but we can have that by
dereferencing the symlink.
Like the previous fix, back-patch to all supported versions.
Discussion: https://postgr.es/m/522433.
1611089678@sss.pgh.pa.us
Michael Paquier [Wed, 20 Jan 2021 02:39:31 +0000 (11:39 +0900)]
Fix ALTER DEFAULT PRIVILEGES with duplicated objects
Specifying duplicated objects in this command would lead to unique
constraint violations in pg_default_acl or "tuple already updated by
self" errors. Similarly to GRANT/REVOKE, increment the command ID after
each subcommand processing to allow this case to work transparently.
A regression test is added by tweaking one of the existing queries of
privileges.sql to stress this case.
Reported-by: Andrus
Author: Michael Paquier
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/
ae2a7dc1-9d71-8cba-3bb9-
e4cb7eb1f44e@hot.ee
Backpatch-through: 9.5
Tom Lane [Tue, 19 Jan 2021 18:25:33 +0000 (13:25 -0500)]
Remove faulty support for MergeAppend plan with WHERE CURRENT OF.
Somebody extended search_plan_tree() to treat MergeAppend exactly
like Append, which is 100% wrong, because unlike Append we can't
assume that only one input node is actively returning tuples.
Hence a cursor using a MergeAppend across a UNION ALL or inheritance
tree could falsely match a WHERE CURRENT OF query at a row that
isn't actually the cursor's current output row, but coincidentally
has the same TID (in a different table) as the current output row.
Delete the faulty code; this means that such a case will now return
an error like 'cursor "foo" is not a simply updatable scan of table
"bar"', instead of silently misbehaving. Users should not find that
surprising though, as the same cursor query could have failed that way
already depending on the chosen plan. (It would fail like that if the
sort were done with an explicit Sort node instead of MergeAppend.)
Expand the clearly-inadequate commentary to be more explicit about
what this code is doing, in hopes of forestalling future mistakes.
It's been like this for awhile, so back-patch to all supported
branches.
Discussion: https://postgr.es/m/482865.
1611075182@sss.pgh.pa.us
Tom Lane [Mon, 18 Jan 2021 23:32:30 +0000 (18:32 -0500)]
Avoid crash with WHERE CURRENT OF and a custom scan plan.
execCurrent.c's search_plan_tree() assumed that ForeignScanStates
and CustomScanStates necessarily have a valid ss_currentRelation.
This is demonstrably untrue for postgres_fdw's remote join and
remote aggregation plans, and non-leaf custom scans might not have
an identifiable scan relation either. Avoid crashing by ignoring
such nodes when the field is null.
This solution will lead to errors like 'cursor "foo" is not a
simply updatable scan of table "bar"' in cases where maybe we
could have allowed WHERE CURRENT OF to work. That's not an issue
for postgres_fdw's usages, since joins or aggregations would render
WHERE CURRENT OF invalid anyway. But an otherwise-transparent
upper level custom scan node might find this annoying. When and if
someone cares to expend work on such a scenario, we could invent a
custom-scan-provider callback to determine what's safe.
Report and patch by David Geier, commentary by me. It's been like
this for awhile, so back-patch to all supported branches.
Discussion: https://postgr.es/m/
0253344d-9bdd-11c4-7f0d-
d88c02cd7991@swarm64.com
Noah Misch [Sat, 16 Jan 2021 20:21:35 +0000 (12:21 -0800)]
Fix pg_dump for GRANT OPTION among initial privileges.
The context is an object that no longer bears some aclitem that it bore
initially. (A user issued REVOKE or GRANT statements upon the object.)
pg_dump is forming SQL to reproduce the object ACL. Since initdb
creates no ACL bearing GRANT OPTION, reaching this bug requires an
extension where the creation script establishes such an ACL. No PGXN
extension does that. If an installation did reach the bug, pg_dump
would have omitted a semicolon, causing a REVOKE and the next SQL
statement to fail. Separately, since the affected code exists to
eliminate an entire aclitem, it wants plain REVOKE, not REVOKE GRANT
OPTION FOR. Back-patch to 9.6, where commit
23f34fa4ba358671adab16773e79c17c92cbc870 first appeared.
Discussion: https://postgr.es/m/
20210109102423.GA160022@rfd.leadboat.com
Noah Misch [Sat, 16 Jan 2021 20:21:35 +0000 (12:21 -0800)]
Prevent excess SimpleLruTruncate() deletion.
Every core SLRU wraps around. With the exception of pg_notify, the wrap
point can fall in the middle of a page. Account for this in the
PagePrecedes callback specification and in SimpleLruTruncate()'s use of
said callback. Update each callback implementation to fit the new
specification. This changes SerialPagePrecedesLogically() from the
style of asyncQueuePagePrecedes() to the style of CLOGPagePrecedes().
(Whereas pg_clog and pg_serial share a key space, pg_serial is nothing
like pg_notify.) The bug fixed here has the same symptoms and user
followup steps as
592a589a04bd456410b853d86bd05faa9432cbbb. Back-patch
to 9.5 (all supported versions).
Reviewed by Andrey Borodin and (in earlier versions) by Tom Lane.
Discussion: https://postgr.es/m/
20190202083822.GC32531@gust.leadboat.com
Tom Lane [Fri, 15 Jan 2021 16:28:51 +0000 (11:28 -0500)]
Improve our heuristic for selecting PG_SYSROOT on macOS.
In cases where Xcode is newer than the underlying macOS version,
asking xcodebuild for the SDK path will produce a pointer to the
SDK shipped with Xcode, which may end up building code that does
not work on the underlying macOS version. It appears that in
such cases, xcodebuild's answer also fails to match the default
behavior of Apple's compiler: assuming one has installed Xcode's
"command line tools", there will be an SDK for the OS's own version
in /Library/Developer/CommandLineTools, and the compiler will
default to using that. This is all pretty poorly documented,
but experimentation suggests that "xcrun --show-sdk-path" gives
the sysroot path that the compiler is actually using, at least
in some cases. Hence, try that first, but revert to xcodebuild
if xcrun fails (in very old Xcode, it is missing or lacks the
--show-sdk-path switch).
Also, "xcrun --show-sdk-path" may give a path that is valid but lacks
any OS version identifier. We don't really want that, since most
of the motivation for wiring -isysroot into the build flags at all
is to ensure that all parts of a PG installation are built against
the same SDK, even when considering extensions built later and/or on
a different machine. Insist on finding "N.N" in the directory name
before accepting the result. (Adding "--sdk macosx" to the xcrun
call seems to produce the same answer as xcodebuild, but usually
more quickly because it's cached, so we also try that as a fallback.)
The core reason why we don't want to use Xcode's default SDK in cases
like this is that Apple's technology for introducing new syscalls
does not play nice with Autoconf: for example, configure will think
that preadv/pwritev exist when using a Big Sur SDK, even when building
on an older macOS version where they don't exist. It'd be nice to
have a better solution to that problem, but this patch doesn't attempt
to fix that.
Per report from Sergey Shinderuk. Back-patch to all supported versions.
Discussion: https://postgr.es/m/
ed3b8e5d-0da8-6ebd-fd1c-
e0ac80a4b204@postgrespro.ru
Tom Lane [Wed, 13 Jan 2021 18:30:04 +0000 (13:30 -0500)]
Doc: clarify behavior of back-half options in pg_dump.
Options that change how the archive data is converted to SQL text
are ignored when dumping to archive formats. The documentation
previously said "not meaningful", which is not helpful.
Discussion: https://postgr.es/m/
161052021249.12228.
9598689907884726185@wrigleys.postgresql.org
Amit Kapila [Wed, 13 Jan 2021 03:01:45 +0000 (08:31 +0530)]
Fix memory leak in SnapBuildSerialize.
The memory for the snapshot was leaked while serializing it to disk during
logical decoding. This memory will be freed only once walsender stops
streaming the changes. This can lead to a huge memory increase when master
logs Standby Snapshot too frequently say when the user is trying to create
many replication slots.
Reported-by: funnyxj.fxj@alibaba-inc.com
Diagnosed-by: funnyxj.fxj@alibaba-inc.com
Author: Amit Kapila
Backpatch-through: 9.5
Discussion: https://postgr.es/m/
033ab54c-6393-42ee-8ec9-
2b399b5d8cde.funnyxj.fxj@alibaba-inc.com
Alvaro Herrera [Tue, 12 Jan 2021 14:48:45 +0000 (11:48 -0300)]
Fix thinko in comment
This comment has been wrong since its introduction in commit
2c03216d8311.
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoAzz6qipFJBbGEaHmyWxvvNDp8httbwLR9tUQWaTjUs2Q@mail.gmail.com
Bruce Momjian [Sat, 9 Jan 2021 17:11:15 +0000 (12:11 -0500)]
doc: expand description of how non-SELECT queries are processed
The previous description of how the executor processes non-SELECT
queries was very dense, causing lack of clarity. This expanded text
spells it out more simply.
Reported-by: fotis.koutoupas@gmail.com
Discussion: https://postgr.es/m/
160912275508.676.
17469511338925622905@wrigleys.postgresql.org
Backpatch-through: 9.5
Tom Lane [Fri, 8 Jan 2021 17:16:00 +0000 (12:16 -0500)]
Fix ancient bug in parsing of BRE-mode regular expressions.
brenext(), when parsing a '*' quantifier, forgot to return any "value"
for the token; per the equivalent case in next(), it should return
value 1 to indicate that greedy rather than non-greedy behavior is
wanted. The result is that the compiled regexp could behave like 'x*?'
rather than the intended 'x*', if we were unlucky enough to have
a zero in v->nextvalue at this point. That seems to happen with some
reliability if we have '.*' at the beginning of a BRE-mode regexp,
although that depends on the initial contents of a stack-allocated
struct, so it's not guaranteed to fail.
Found by Alexander Lakhin using valgrind testing. This bug seems
to be aboriginal in Spencer's code, so back-patch all the way.
Discussion: https://postgr.es/m/16814-
6c5e3edd2bdf0d50@postgresql.org
Tom Lane [Thu, 7 Jan 2021 16:45:09 +0000 (11:45 -0500)]
Further second thoughts about idle_session_timeout patch.
On reflection, the order of operations in PostgresMain() is wrong.
These timeouts ought to be shut down before, not after, we do the
post-command-read CHECK_FOR_INTERRUPTS, to guarantee that any
timeout error will be detected there rather than at some ill-defined
later point (possibly after having wasted a lot of work).
This is really an error in the original idle_in_transaction_timeout
patch, so back-patch to 9.6 where that was introduced.
Michael Paquier [Thu, 7 Jan 2021 11:50:56 +0000 (20:50 +0900)]
doc: Remove reference to recovery params for divergence lookup in pg_rewind
The documentation of pg_rewind mentioned the use of restore_command and
primary_conninfo as options available at startup to fetch missing WAL
segments that could be used to find the point of divergence for the
rewind. This is confusing because when finding the point of divergence
the target cluster is offline, so this option is not available.
Issue introduced by
878bd9a, so backpatch down to 9.6. The
documentation of 13 and HEAD was already right as this sentence has been
changed by
a7e8ece when introducing -c/--restore-target-wal.
Reported-by: Amine Tengilimoglu
Discussion: https://postgr.es/m/CADTdw-w_0MP=iQrfizeU4QU5fcZb+w8P_oPeLL+WznWf0kbn3w@mail.gmail.com
Backpatch-through: 9.6
Fujii Masao [Wed, 6 Jan 2021 03:33:28 +0000 (12:33 +0900)]
Detect the deadlocks between backends and the startup process.
The deadlocks that the recovery conflict on lock is involved in can
happen between hot-standby backends and the startup process.
If a backend takes an access exclusive lock on the table and which
finally triggers the deadlock, that deadlock can be detected
as expected. On the other hand, previously, if the startup process
took an access exclusive lock and which finally triggered the deadlock,
that deadlock could not be detected and could remain even after
deadlock_timeout passed. This is a bug.
The cause of this bug was that the code for handling the recovery
conflict on lock didn't take care of deadlock case at all. It assumed
that deadlocks involving the startup process and backends were able
to be detected by the deadlock detector invoked within backends.
But this assumption was incorrect. The startup process also should
have invoked the deadlock detector if necessary.
To fix this bug, this commit makes the startup process invoke
the deadlock detector if deadlock_timeout is reached while handling
the recovery conflict on lock. Specifically, in that case, the startup
process requests all the backends holding the conflicting locks to
check themselves for deadlocks.
Back-patch to v9.6. v9.5 has also this bug, but per discussion we decided
not to back-patch the fix to v9.5. Because v9.5 doesn't have some
infrastructure codes (e.g.,
37c54863cf) that this bug fix patch depends on.
We can apply those codes for the back-patch, but since the next minor
version release is the final one for v9.5, it's risky to do that. If we
unexpectedly introduce new bug to v9.5 by the back-patch, there is no
chance to fix that. We determined that the back-patch to v9.5 would give
more risk than gain.
Author: Fujii Masao
Reviewed-by: Bertrand Drouvot, Masahiko Sawada, Kyotaro Horiguchi
Discussion: https://postgr.es/m/
4041d6b6-cf24-a120-36fa-
1294220f8243@oss.nttdata.com
Bruce Momjian [Tue, 5 Jan 2021 19:26:37 +0000 (14:26 -0500)]
doc: improve NLS instruction wording
Reported-by: "Tang, Haiying"
Discussion: https://postgr.es/m/
bbbccf7a3c2d436e85d45869d612fd6b@G08CNEXMBPEKD05.g08.fujitsu.local
Author: "Tang, Haiying"
Backpatch-through: 9.5
Dean Rasheed [Tue, 5 Jan 2021 11:45:17 +0000 (11:45 +0000)]
Add an explicit cast to double when using fabs().
Commit
bc43b7c2c0 used fabs() directly on an int variable, which
apparently requires an explicit cast on some platforms.
Per buildfarm.
Dean Rasheed [Tue, 5 Jan 2021 11:02:46 +0000 (11:02 +0000)]
Fix numeric_power() when the exponent is INT_MIN.
In power_var_int(), the computation of the number of significant
digits to use in the computation used log(Abs(exp)), which isn't safe
because Abs(exp) returns INT_MIN when exp is INT_MIN. Use fabs()
instead of Abs(), so that the exponent is cast to a double before the
absolute value is taken.
Back-patch to 9.6, where this was introduced (by
7d9a4737c2).
Discussion: https://postgr.es/m/CAEZATCVd6pMkz=BrZEgBKyqqJrt2xghr=fNc8+Z=5xC6cgWrWA@mail.gmail.com
Bruce Momjian [Sat, 2 Jan 2021 18:06:24 +0000 (13:06 -0500)]
Update copyright for 2021
Backpatch-through: 9.5
Tom Lane [Fri, 1 Jan 2021 20:51:09 +0000 (15:51 -0500)]
Doc: improve explanation of EXTRACT(EPOCH) for timestamp without tz.
Try to be clearer about what computation is actually happening here.
Per bug #16797 from Dana Burd.
Discussion: https://postgr.es/m/16797-
f264b0b980b53b8b@postgresql.org
Tom Lane [Wed, 30 Dec 2020 22:48:43 +0000 (17:48 -0500)]
Doc: spell out comparison behaviors for the date/time types.
The behavior of cross-type comparisons among date/time data types was
not really explained anywhere. You could probably infer it if you
recognized the applicability of comments elsewhere about datatype
conversions, but it seems worthy of explicit documentation.
Per bug #16797 from Dana Burd.
Discussion: https://postgr.es/m/16797-
f264b0b980b53b8b@postgresql.org
Michael Paquier [Tue, 29 Dec 2020 09:19:25 +0000 (18:19 +0900)]
doc: Improve some grammar and sentences
90fbf7c has taken care of that for HEAD. This includes the portion of
the fixes that applies to the documentation, where needed depending on
the branch.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20201227202604.GC26311@telsasoft.com
Backpatch-through: 9.5
Michael Paquier [Mon, 28 Dec 2020 13:17:16 +0000 (22:17 +0900)]
Fix inconsistent code with shared invalidations of snapshots
The code in charge of processing a single invalidation message has been
using since
568d413 the structure for relation mapping messages. This
had fortunately no consequence as both locate the database ID at the
same location, but it could become a problem in the future if this area
of the code changes.
Author: Konstantin Knizhnik
Discussion: https://postgr.es/m/
8044c223-4d3a-2cdb-42bf-
29940840ce94@postgrespro.ru
Backpatch-through: 9.5
Fujii Masao [Mon, 28 Dec 2020 11:00:15 +0000 (20:00 +0900)]
postgres_fdw: Fix connection leak.
In postgres_fdw, the cached connections to foreign servers will not be
closed until the local session exits if the user mappings or foreign servers
that those connections depend on are dropped. Those connections can be
leaked.
To fix that connection leak issue, after a change to a pg_foreign_server
or pg_user_mapping catalog entry, this commit makes postgres_fdw close
the connections depending on that entry immediately if current
transaction has not used those connections yet. Otherwise, mark those
connections as invalid and then close them at the end of current transaction,
since they cannot be closed in the midst of the transaction using them.
Closed connections will be remade at the next opportunity if necessary.
Back-patch to all supported branches.
Author: Bharath Rupireddy
Reviewed-by: Zhihong Yu, Zhijie Hou, Fujii Masao
Discussion: https://postgr.es/m/CALj2ACVNcGH_6qLY-4_tXz8JLvA+4yeBThRfxMz7Oxbk1aHcpQ@mail.gmail.com
Noah Misch [Fri, 25 Dec 2020 19:02:56 +0000 (11:02 -0800)]
Fix back-patch of "Invalidate acl.c caches when pg_authid changes."
Test script role names and error messages differed in v10, 9.6 and 9.5.
Discussion: https://postgr.es/m/
20201221095028.GB3777719@rfd.leadboat.com
Noah Misch [Fri, 25 Dec 2020 18:41:59 +0000 (10:41 -0800)]
Invalidate acl.c caches when pg_authid changes.
This makes existing sessions reflect "ALTER ROLE ... [NO]INHERIT" as
quickly as they have been reflecting "GRANT role_name". Back-patch to
9.5 (all supported versions).
Reviewed by Nathan Bossart.
Discussion: https://postgr.es/m/
20201221095028.GB3777719@rfd.leadboat.com
Michael Paquier [Wed, 23 Dec 2020 03:51:51 +0000 (12:51 +0900)]
Fix portability issues with parsing of recovery_target_xid
The parsing of this parameter has been using strtoul(), which is not
portable across platforms. On most Unix platforms, unsigned long has a
size of 64 bits, while on Windows it is 32 bits. It is common in
recovery scenarios to rely on the output of txid_current() or even the
newer pg_current_xact_id() to get a transaction ID for setting up
recovery_target_xid. The value returned by those functions includes the
epoch in the computed result, which would cause strtoul() to fail where
unsigned long has a size of 32 bits once the epoch is incremented.
WAL records and 2PC data include only information about 32-bit XIDs and
it is not possible to have XIDs across more than one epoch, so
discarding the high bits from the transaction ID set has no impact on
recovery. On the contrary, the use of strtoul() prevents a consistent
behavior across platforms depending on the size of unsigned long.
This commit changes the parsing of recovery_target_xid to use
pg_strtouint64() instead, available down to 9.6. There is one TAP test
stressing recovery with recovery_target_xid, where a tweak based on
pg_reset{xlog,wal} is added to bump the XID epoch so as this change gets
tested, as per an idea from Alexander Lakhin.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16780-
107fd0c0385b1035@postgresql.org
Backpatch-through: 9.6
Tom Lane [Mon, 21 Dec 2020 18:11:30 +0000 (13:11 -0500)]
Remove "invalid concatenation of jsonb objects" error case.
The jsonb || jsonb operator arbitrarily rejected certain combinations
of scalar and non-scalar inputs, while being willing to concatenate
other combinations. This was of course quite undocumented. Rather
than trying to document it, let's just remove the restriction,
creating a uniform rule that unless we are handling an object-to-object
concatenation, non-array inputs are converted to one-element arrays,
resulting in an array-to-array concatenation. (This does not change
the behavior for any case that didn't throw an error before.)
Per complaint from Joel Jacobson. Back-patch to all supported branches.
Discussion: https://postgr.es/m/163099.
1608312033@sss.pgh.pa.us
Tom Lane [Sun, 20 Dec 2020 20:28:22 +0000 (15:28 -0500)]
Doc: fix description of how to use src/tutorial files.
The separate "cd" command before invoking psql made sense (or at least
I thought so) when it was added in commit
ed1939332. But
4e3a61635
removed the supporting text that explained when to use it, making it
just confusing. So drop it.
Also switch from four-dot to three-dot filler for the unsupplied
part of the path, since at least one person has read the four-dot
filler as a typo for "../..". And fix these/those inconsistency.
Discussion: https://postgr.es/m/
160837647714.673.
5195186835607800484@wrigleys.postgresql.org
Tom Lane [Sun, 20 Dec 2020 18:37:25 +0000 (13:37 -0500)]
Doc: improve description of pgbench script weights.
Point out the workaround to be used if you want to write a script
file name that includes "@". Clean up the text a little.
Fabien Coelho, additional wordsmithing by me
Discussion: https://postgr.es/m/
1c4e81550d214741827a03292222db8d@G08CNEXMBPEKD06.g08.fujitsu.local
Tom Lane [Fri, 18 Dec 2020 20:46:44 +0000 (15:46 -0500)]
Avoid memcpy() with same source and destination during relmapper init.
A narrow reading of the C standard says that memcpy(x,x,n) is undefined,
although it's hard to envision an implementation that would really
misbehave. However, analysis tools such as valgrind might whine about
this; accordingly, let's band-aid relmapper.c to not do it.
See also
5b630501e,
d3f4e8a8a,
ad7b48ea0, and other similar fixes.
Apparently, none of those folk tried valgrinding initdb? This has been
like this for long enough that I'm surprised it hasn't been reported
before.
Back-patch, just in case anybody wants to use a back branch on a platform
that complains about this; we back-patched those earlier fixes too.
Discussion: https://postgr.es/m/161790.
1608310142@sss.pgh.pa.us
Andrew Dunstan [Tue, 15 Dec 2020 15:00:18 +0000 (10:00 -0500)]
Use native methods to open input in TestLib::slurp_file on Windows.
This is a backport of commits
114541d58e and
6f59826f0 to the remaining
live branches.
Jeff Davis [Tue, 15 Dec 2020 07:49:57 +0000 (23:49 -0800)]
Revert "Cannot use WL_SOCKET_WRITEABLE without WL_SOCKET_READABLE."
This reverts commit
3a9e64aa0d96c8ffb6c682b082d0f72b1d373327.
Commit
4bad60e3 fixed the root of the problem that
3a9e64aa worked
around.
This enables proper pipelining of commands after terminating
replication, eliminating an undocumented limitation.
Discussion: https://postgr.es/m/
3d57bc29-4459-578b-79cb-
7641baf53c57%40iki.fi
Backpatch-through: 9.5
Bruce Momjian [Sat, 12 Dec 2020 17:59:09 +0000 (12:59 -0500)]
initdb: complete getopt_long alphabetization
Backpatch-through: 9.5
Bruce Momjian [Sat, 12 Dec 2020 17:51:16 +0000 (12:51 -0500)]
initdb: properly alphabetize getopt_long options in C string
Backpatch-through: 9.5
Tom Lane [Tue, 8 Dec 2020 22:50:54 +0000 (17:50 -0500)]
Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.
array_get_element and array_get_slice qualify as leakproof, since
they will silently return NULL for bogus subscripts. But
array_set_element and array_set_slice throw errors for such cases,
making them clearly not leakproof. contain_leaked_vars was evidently
written with only the former case in mind, as it gave the wrong answer
for assignment SubscriptingRefs (nee ArrayRefs).
This would be a live security bug, were it not that assignment
SubscriptingRefs can only occur in INSERT and UPDATE target lists,
while we only care about leakproofness for qual expressions; so the
wrong answer can't occur in practice. Still, that's a rather shaky
answer for a security-related question; and maybe in future somebody
will want to ask about leakproofness of a tlist. So it seems wise to
fix and even back-patch this correction.
(We would need some change here anyway for the upcoming
generic-subscripting patch, since extensions might make different
tradeoffs about whether to throw errors. Commit
558d77f20 attempted
to lay groundwork for that by asking check_functions_in_node whether a
SubscriptingRef contains leaky functions; but that idea fails now that
the implementation methods of a SubscriptingRef are not SQL-visible
functions that could be marked leakproof or not.)
Back-patch to 9.6. While 9.5 has the same issue, the code's a bit
different. It seems quite unlikely that we'd introduce any actual bug
in the short time 9.5 has left to live, so the work/risk/reward balance
isn't attractive for changing 9.5.
Discussion: https://postgr.es/m/
3143742.
1607368115@sss.pgh.pa.us
Tom Lane [Tue, 8 Dec 2020 18:09:48 +0000 (13:09 -0500)]
Doc: clarify that CREATE TABLE discards redundant unique constraints.
The SQL standard says that redundant unique constraints are disallowed,
but we long ago decided that throwing an error would be too
user-unfriendly, so we just drop redundant ones. The docs weren't very
clear about that though, as this behavior was only explained for PRIMARY
KEY vs UNIQUE, not UNIQUE vs UNIQUE.
While here, I couldn't resist doing some copy-editing and markup-fixing
on the adjacent text about INCLUDE options.
Per bug #16767 from Matthias vd Meent.
Discussion: https://postgr.es/m/16767-
1714a2056ca516d0@postgresql.org
Tom Lane [Tue, 8 Dec 2020 17:06:19 +0000 (12:06 -0500)]
Doc: explain that the string types can't store \0 (ASCII NUL).
This restriction was mentioned in connection with string literals,
but it wasn't made clear that it's a general restriction not just
a syntactic limitation in query strings.
Per unsigned documentation comment.
Discussion: https://postgr.es/m/
160720552914.710.
16625261471128631268@wrigleys.postgresql.org
Michael Paquier [Tue, 8 Dec 2020 06:22:59 +0000 (15:22 +0900)]
pgcrypto: Detect errors with EVP calls from OpenSSL
The following routines are called within pgcrypto when handling digests
but there were no checks for failures:
- EVP_MD_CTX_size (can fail with -1 as of 3.0.0)
- EVP_MD_CTX_block_size (can fail with -1 as of 3.0.0)
- EVP_DigestInit_ex
- EVP_DigestUpdate
- EVP_DigestFinal_ex
A set of elog(ERROR) is added by this commit to detect such failures,
that should never happen except in the event of a processing failure
internal to OpenSSL.
Note that it would be possible to use ERR_reason_error_string() to get
more context about such errors, but these refer mainly to the internals
of OpenSSL, so it is not really obvious how useful that would be. This
is left out for simplicity.
Per report from Coverity. Thanks to Tom Lane for the discussion.
Backpatch-through: 9.5
Heikki Linnakangas [Mon, 7 Dec 2020 12:44:34 +0000 (14:44 +0200)]
Fix more race conditions in the newly-added pg_rewind test.
pg_rewind looks at the control file to check what timeline a server is on.
But promotion doesn't immediately write a checkpoint, it merely writes
an end-of-recovery WAL record. If pg_rewind runs immediately after
promotion, before the checkpoint has completed, it will think think that
the server is still on the earlier timeline. We ran into this issue a long
time ago already, see commit
484a848a73f.
It's a bit bogus that pg_rewind doesn't determine the timeline correctly
until the end-of-recovery checkpoint has completed. We probably should
fix that. But for now work around it by waiting for the checkpoint
to complete before running pg_rewind, like we did in commit
484a848a73f.
In the passing, tidy up the new test a little bit. Rerder the INSERTs so
that the comments make more sense, remove a spurious CHECKPOINT call after
pg_rewind has already run, and add --debug option, so that if this fails
again, we'll have more data.
Per buildfarm failure at https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=rorqual&dt=2020-12-06%2018%3A32%3A19&stg=pg_rewind-check.
Backpatch to all supported versions.
Discussion: https://www.postgresql.org/message-id/
1713707e-e318-761c-d287-
5b6a4aa807e8@iki.fi
Heikki Linnakangas [Fri, 4 Dec 2020 16:20:18 +0000 (18:20 +0200)]
Fix race conditions in newly-added test.
Buildfarm has been failing sporadically on the new test. I was able to
reproduce this by adding a random 0-10 s delay in the walreceiver, just
before it connects to the primary. There's a race condition where node_3
is promoted before it has fully caught up with node_1, leading to diverged
timelines. When node_1 is later reconfigured as standby following node_3,
it fails to catch up:
LOG: primary server contains no more WAL on requested timeline 1
LOG: new timeline 2 forked off current database system timeline 1 before current recovery point 0/
30000A0
That's the situation where you'd need to use pg_rewind, but in this case
it happens already when we are just setting up the actual pg_rewind
scenario we want to test, so change the test so that it waits until
node_3 is connected and fully caught up before promoting it, so that you
get a clean, controlled failover.
Also rewrite some of the comments, for clarity. The existing comments
detailed what each step in the test did, but didn't give a good overview
of the situation the steps were trying to create.
For reasons I don't understand, the test setup had to be written slightly
differently in 9.6 and 9.5 than in later versions. The 9.5/9.6 version
needed node 1 to be reinitialized from backup, whereas in later versions
it could be shut down and reconfigured to be a standby. But even 9.5 should
support "clean switchover", where primary makes sure that pending WAL is
replicated to standby on shutdown. It would be nice to figure out what's
going on there, but that's independent of pg_rewind and the scenario that
this test tests.
Discussion: https://www.postgresql.org/message-id/
b0a3b95b-82d2-6089-6892-
40570f8c5e60%40iki.fi
Bruce Momjian [Thu, 3 Dec 2020 15:28:58 +0000 (10:28 -0500)]
docs: list single-letter options first in command-line summary
In a few places, the long-version options were listed before the
single-letter ones in the command summary of a few commands. This
didn't match other commands, and didn't match the option ordering later
in the same reference page.
Backpatch-through: 9.5
Heikki Linnakangas [Thu, 3 Dec 2020 13:57:48 +0000 (15:57 +0200)]
Fix pg_rewind bugs when rewinding a standby server.
If the target is a standby server, its WAL doesn't end at the last
checkpoint record, but at minRecoveryPoint. We must scan all the
WAL from the last common checkpoint all the way up to minRecoveryPoint
for modified pages, and also consider that portion when determining
whether the server needs rewinding.
Backpatch to all supported versions.
Author: Ian Barwick and me
Discussion: https://www.postgresql.org/message-id/CABvVfJU-LDWvoz4-Yow3Ay5LZYTuPD7eSjjE4kGyNZpXC6FrVQ%40mail.gmail.com
Tom Lane [Tue, 1 Dec 2020 19:02:28 +0000 (14:02 -0500)]
Ensure that expandTableLikeClause() re-examines the same table.
As it stood, expandTableLikeClause() re-did the same relation_openrv
call that transformTableLikeClause() had done. However there are
scenarios where this would not find the same table as expected.
We hold lock on the LIKE source table, so it can't be renamed or
dropped, but another table could appear before it in the search path.
This explains the odd behavior reported in bug #16758 when cloning a
table as a temp table of the same name. This case worked as expected
before commit
502898192 introduced the need to open the source table
twice, so we should fix it.
To make really sure we get the same table, let's re-open it by OID not
name. That requires adding an OID field to struct TableLikeClause,
which is a little nervous-making from an ABI standpoint, but as long
as it's at the end I don't think there's any serious risk.
Per bug #16758 from Marc Boeren. Like the previous patch,
back-patch to all supported branches.
Discussion: https://postgr.es/m/16758-
840e84a6cfab276d@postgresql.org
Thomas Munro [Tue, 1 Dec 2020 00:46:27 +0000 (13:46 +1300)]
Free disk space for dropped relations on commit.
When committing a transaction that dropped a relation, we previously
truncated only the first segment file to free up disk space (the one
that won't be unlinked until the next checkpoint).
Truncate higher numbered segments too, even though we unlink them on
commit. This frees the disk space immediately, even if other backends
have open file descriptors and might take a long time to get around to
handling shared invalidation events and closing them. Also extend the
same behavior to the first segment, in recovery.
Back-patch to all supported releases.
Bug: #16663
Reported-by: Denis Patron <denis.patron@previnet.it>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Reviewed-by: Neil Chen <carpenter.nail.cz@gmail.com>
Reviewed-by: David Zhang <david.zhang@highgo.ca>
Discussion: https://postgr.es/m/16663-
fe97ccf9932fc800%40postgresql.org
Alvaro Herrera [Mon, 30 Nov 2020 21:24:55 +0000 (18:24 -0300)]
Document concurrent indexes waiting on each other
Because regular CREATE INDEX commands are independent, and there's no
logical data dependency, it's not immediately obvious that transactions
held by concurrent index builds on one table will block the second phase
of concurrent index creation on an unrelated table, so document this
caveat.
Backpatch this all the way back. In branch master, mention that only
some indexes are involved.
Author: James Coleman <jtc331@gmail.com>
Reviewed-by: David Johnston <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CAAaqYe994=PUrn8CJZ4UEo_S-FfRr_3ogERyhtdgHAb2WG_Ufg@mail.gmail.com
Tom Lane [Mon, 30 Nov 2020 17:22:43 +0000 (12:22 -0500)]
Fix miscomputation of direct_lateral_relids for join relations.
If a PlaceHolderVar is to be evaluated at a join relation, but
its value is only needed there and not at higher levels, we neglected
to update the joinrel's direct_lateral_relids to include the PHV's
source rel. This causes problems because join_is_legal() then won't
allow joining the joinrel to the PHV's source rel at all, leading
to "failed to build any N-way joins" planner failures.
Per report from Andreas Seltenreich. Back-patch to 9.5
where the problem originated.
Discussion: https://postgr.es/m/87blfgqa4t.fsf@aurora.ydns.eu
Tom Lane [Sun, 29 Nov 2020 20:22:04 +0000 (15:22 -0500)]
Fix recently-introduced breakage in psql's \connect command.
Through my misreading of what the existing code actually did,
commits
85c54287a et al. broke psql's behavior for the case where
"\c connstring" provides a password in the connstring. We should
use that password in such a case, but as of
85c54287a we ignored it
(and instead, prompted for a password).
Commit
94929f1cf fixed that in HEAD, but since I thought it was
cleaning up a longstanding misbehavior and not one I'd just created,
I didn't back-patch it.
Hence, back-patch the portions of
94929f1cf having to do with
password management. In addition to fixing the introduced bug,
this means that "\c -reuse-previous=on connstring" will allow
re-use of an existing connection's password if the connstring
doesn't change user/host/port. That didn't happen before, but
it seems like a bug fix, and anyway I'm loath to have significant
differences in this code across versions.
Also fix an error with the same root cause about whether or not to
override a connstring's setting of client_encoding. As of
85c54287a
we always did so; restore the previous behavior of overriding only
when stdin/stdout are a terminal and there's no environment setting
of PGCLIENTENCODING. (I find that definition a bit surprising, but
right now doesn't seem like the time to revisit it.)
Per bug #16746 from Krzysztof Gradek. As with the previous patch,
back-patch to all supported branches.
Discussion: https://postgr.es/m/16746-
44b30e2edf4335d4@postgresql.org