Tom Lane [Wed, 20 Jan 2021 16:49:29 +0000 (11:49 -0500)]
Disable vacuum page skipping in selected test cases.
By default VACUUM will skip pages that it can't immediately get
exclusive access to, which means that even activities as harmless
and unpredictable as checkpoint buffer writes might prevent a page
from being processed. Ordinarily this is no big deal, but we have
a small number of test cases that examine the results of VACUUM's
processing and therefore will fail if the page of interest is skipped.
This seems to be the explanation for some rare buildfarm failures.
To fix, add the DISABLE_PAGE_SKIPPING option to the VACUUM commands
in tests where this could be an issue.
In passing, remove a duplicated query in pageinspect/sql/page.sql.
Back-patch as necessary (some of these cases are as old as v10).
Discussion: https://postgr.es/m/413923.
1611006484@sss.pgh.pa.us
Heikki Linnakangas [Wed, 20 Jan 2021 09:58:03 +0000 (11:58 +0200)]
Fix bug in detecting concurrent page splits in GiST insert
In commit
9eb5607e699, I got the condition on checking for split or
deleted page wrong: I used && instead of ||. The comment correctly said
"concurrent split _or_ deletion".
As a result, GiST insertion could miss a concurrent split, and insert to
wrong page. Duncan Sands demonstrated this with a test script that did a
lot of concurrent inserts.
Backpatch to v12, where this was introduced. REINDEX is required to fix
indexes that were affected by this bug.
Backpatch-through: 12
Reported-by: Duncan Sands
Discussion: https://www.postgresql.org/message-id/
a9690483-6c6c-3c82-c8ba-
dc1a40848f11%40deepbluecap.com
Michael Paquier [Wed, 20 Jan 2021 02:39:14 +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
Bruce Momjian [Mon, 18 Jan 2021 23:48:25 +0000 (18:48 -0500)]
doc: adjust alignment of doc file list for "pg_waldump.sgml"
Backpatch-through: 10
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[email protected]
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[email protected]
Tomas Vondra [Fri, 15 Jan 2021 22:24:19 +0000 (23:24 +0100)]
Disallow CREATE STATISTICS on system catalogs
Add a check that CREATE STATISTICS does not add extended statistics on
system catalogs, similarly to indexes etc. It can be overriden using
the allow_system_table_mods GUC.
This bug exists since
7b504eb282c, adding the extended statistics, so
backpatch all the way back to PostgreSQL 10.
Author: Tomas Vondra
Reported-by: Dean Rasheed
Backpatch-through: 10
Discussion: https://postgr.es/m/CAEZATCXAPrrOKwEsyZKQ4uzzJQWBCt6QAvOcgqRGdWwT1zb%2BrQ%40mail.gmail.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
Fujii Masao [Fri, 15 Jan 2021 03:44:17 +0000 (12:44 +0900)]
Fix calculation of how much shared memory is required to store a TOC.
Commit
ac883ac453 refactored shm_toc_estimate() but changed its calculation
of shared memory size for TOC incorrectly. Previously this could cause too
large memory to be allocated.
Back-patch to v11 where the bug was introduced.
Author: Takayuki Tsunakawa
Discussion: https://postgr.es/m/TYAPR01MB2990BFB73170E2C4921E2C4DFEA80@TYAPR01MB2990.jpnprd01.prod.outlook.com
Tom Lane [Thu, 14 Jan 2021 21:19:38 +0000 (16:19 -0500)]
pg_dump: label PUBLICATION TABLE ArchiveEntries with an owner.
This is the same fix as commit
9eabfe300 applied to INDEX ATTACH
entries, but for table-to-publication attachments. As in that
case, even though the backend doesn't record "ownership" of the
attachment, we still ought to label it in the dump archive with
the role name that should run the ALTER PUBLICATION command.
The existing behavior causes the ALTER to be done by the original
role that started the restore; that will usually work fine, but
there may be corner cases where it fails.
The bulk of the patch is concerned with changing struct
PublicationRelInfo to include a pointer to the associated
PublicationInfo object, so that we can get the owner's name
out of that when the time comes. While at it, I rewrote
getPublicationTables() to do just one query of pg_publication_rel,
not one per table.
Back-patch to v10 where this code was introduced.
Discussion: https://postgr.es/m/
1165710.
1610473242@sss.pgh.pa.us
Alvaro Herrera [Thu, 14 Jan 2021 18:32:14 +0000 (15:32 -0300)]
Prevent drop of tablespaces used by partitioned relations
When a tablespace is used in a partitioned relation (per commits
ca4103025dfe in pg12 for tables and
33e6c34c3267 in pg11 for indexes),
it is possible to drop the tablespace, potentially causing various
problems. One such was reported in bug #16577, where a rewriting ALTER
TABLE causes a server crash.
Protect against this by using pg_shdepend to keep track of tablespaces
when used for relations that don't keep physical files; we now abort a
tablespace if we see that the tablespace is referenced from any
partitioned relations.
Backpatch this to 11, where this problem has been latent all along. We
don't try to create pg_shdepend entries for existing partitioned
indexes/tables, but any ones that are modified going forward will be
protected.
Note slight behavior change: when trying to drop a tablespace that
contains both regular tables as well as partitioned ones, you'd
previously get ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE and now you'll
get ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST. Arguably, the latter is more
correct.
It is possible to add protecting pg_shdepend entries for existing
tables/indexes, by doing
ALTER TABLE ONLY some_partitioned_table SET TABLESPACE pg_default;
ALTER TABLE ONLY some_partitioned_table SET TABLESPACE original_tablespace;
for each partitioned table/index that is not in the database default
tablespace. Because these partitioned objects do not have storage, no
file needs to be actually moved, so it shouldn't take more time than
what's required to acquire locks.
This query can be used to search for such relations:
SELECT ... FROM pg_class WHERE relkind IN ('p', 'I') AND reltablespace <> 0
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16577-
881633a9f9894fd5@postgresql.org
Author: Álvaro Herrera
Reviewed-by: Michael Paquier
Fujii Masao [Thu, 14 Jan 2021 05:37:01 +0000 (14:37 +0900)]
Stabilize timeline switch regression test.
Commit
fef5b47f6b added the regression test to check whether a standby is
able to follow a primary on a newer timeline when WAL archiving is enabled.
But the buildfarm member florican reported that this test failed because
the requested WAL segment was removed and replication failed. This is a
timing issue. Since neither replication slot is used nor wal_keep_size is set
in the test, checkpoint could remove the WAL segment that's still necessary
for replication.
This commit stabilizes the test by setting wal_keep_size.
Back-patch to v13 where the regression test that this commit stabilizes
was added.
Author: Fujii Masao
Discussion: https://postgr.es/m/X//
[email protected]
Fujii Masao [Thu, 14 Jan 2021 03:28:47 +0000 (12:28 +0900)]
Ensure that a standby is able to follow a primary on a newer timeline.
Commit
709d003fbd refactored WAL-reading code, but accidentally caused
WalSndSegmentOpen() to fail to follow a timeline switch while reading from
a historic timeline. This issue caused a standby to fail to follow a primary
on a newer timeline when WAL archiving is enabled.
If there is a timeline switch within the segment, WalSndSegmentOpen() should
read from the WAL segment belonging to the new timeline. But previously
since it failed to follow a timeline switch, it tried to read the WAL segment
with old timeline. When WAL archiving is enabled, that WAL segment with
old timeline doesn't exist because it's renamed to .partial. This leads
a primary to have tried to read non-existent WAL segment, and which caused
replication to faill with the error "ERROR: requested WAL segment ... has
already been removed".
This commit fixes WalSndSegmentOpen() so that it's able to follow a timeline
switch, to ensure that a standby is able to follow a primary on a newer
timeline even when WAL archiving is enabled.
This commit also adds the regression test to check whether a standby is
able to follow a primary on a newer timeline when WAL archiving is enabled.
Back-patch to v13 where the bug was introduced.
Reported-by: Kyotaro Horiguchi
Author: Kyotaro Horiguchi, tweaked by Fujii Masao
Reviewed-by: Alvaro Herrera, Fujii Masao
Discussion: https://postgr.es/m/
20201209.174314.
282492377848029776[email protected]
Alvaro Herrera [Wed, 13 Jan 2021 20:55:41 +0000 (17:55 -0300)]
Call out vacuum considerations in create index docs
Backpatch to pg12, which is as far as it goes without conflicts.
Author: James Coleman
Reviewed-by: "David G. Johnston"
Discussion: https://postgr.es/m/CAAaqYe9oEfbz7AxXq7OX+FFVi5w5p1e_Of8ON8ZnKO9QqBfmjg@mail.gmail.com
Tom Lane [Wed, 13 Jan 2021 19:52:49 +0000 (14:52 -0500)]
Disallow a digit as the first character of a variable name in pgbench.
The point of this restriction is to avoid trying to substitute variables
into timestamp literal values, which may contain strings like '12:34'.
There is a good deal more that should be done to reduce pgbench's
tendency to substitute where it shouldn't. But this is sufficient to
solve the case complained of by Jaime Soler, and it's simple enough
to back-patch.
Back-patch to v11; before commit
9d36a3866, pgbench had a slightly
different definition of what a variable name is, and anyway it seems
unwise to change long-stable branches for this.
Fabien Coelho
Discussion: https://postgr.es/m/alpine.DEB.2.22.394.
2006291740420.805678@pseudo
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
Magnus Hagander [Wed, 13 Jan 2021 10:07:37 +0000 (11:07 +0100)]
Remove incorrect markup
Seems
737d69ffc3c made a copy/paste or automation error resulting in two
extra right-parenthesis.
Reported-By: Michael Vastola
Backpatch-through: 13
Discussion: https://postgr.es/m/
161051035421.12224.
1741822783166533529@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: [email protected]
Diagnosed-by: [email protected]
Author: Amit Kapila
Backpatch-through: 9.5
Discussion: https://postgr.es/m/
033ab54c-6393-42ee-8ec9-
2b399b5d8cde[email protected]
Tom Lane [Tue, 12 Jan 2021 18:37:38 +0000 (13:37 -0500)]
pg_dump: label INDEX ATTACH ArchiveEntries with an owner.
Although a partitioned index's attachment to its parent doesn't
have separate ownership, the ArchiveEntry for it needs to be
marked with an owner anyway, to ensure that the ALTER command
is run by the appropriate role when restoring with
--use-set-session-authorization. Without this, the ALTER will
be run by the role that started the restore session, which will
usually work but it's formally the wrong thing.
Back-patch to v11 where this type of ArchiveEntry was added.
In HEAD, add equivalent commentary to the just-added TABLE ATTACH
case, which I'd made do the right thing already.
Discussion: https://postgr.es/m/
1094034.
1610418498@sss.pgh.pa.us
Tom Lane [Tue, 12 Jan 2021 17:52:14 +0000 (12:52 -0500)]
Doc: fix description of privileges needed for ALTER PUBLICATION.
Adding a table to a publication requires ownership of the table
(in addition to ownership of the publication). This was mentioned
nowhere.
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
Discussion: https://postgr.es/m/CAD21AoAzz6qipFJBbGEaHmyWxvvNDp8httbwLR9tUQWaTjUs2Q@mail.gmail.com
Amit Kapila [Tue, 12 Jan 2021 03:00:16 +0000 (08:30 +0530)]
Fix relation descriptor leak.
We missed closing the relation descriptor while sending changes via the
root of partitioned relations during logical replication.
Author: Amit Langote and Mark Zhao
Reviewed-by: Amit Kapila and Ashutosh Bapat
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/
[email protected]
Discussion: https://postgr.es/m/
[email protected]
Bruce Momjian [Sat, 9 Jan 2021 17:11:16 +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: [email protected]
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 [Fri, 8 Jan 2021 01:36:09 +0000 (20:36 -0500)]
Adjust createdb TAP tests to work on recent OpenBSD.
We found last February that the error-case tests added by commit
008cf0409 failed on OpenBSD, because that platform doesn't really
check locale names. At the time it seemed that that was only an issue
for LC_CTYPE, but testing on a more recent version of OpenBSD shows
that it's now equally lax about LC_COLLATE.
Rather than dropping the LC_COLLATE test too, put back LC_CTYPE
(reverting
c4b0edb07), and adjust these tests to accept the different
error message that we get if setlocale() doesn't reject a bogus locale
name. The point of these tests is not really what the backend does
with the locale name, but to show that createdb quotes funny locale
names safely; so we're not losing test reliability this way.
Back-patch as appropriate.
Discussion: https://postgr.es/m/231373.
1610058324@sss.pgh.pa.us
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.
Fujii Masao [Wed, 6 Jan 2021 03:29:43 +0000 (12:29 +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
Fujii Masao [Wed, 6 Jan 2021 02:58:23 +0000 (11:58 +0900)]
doc: Fix description about default behavior of recovery_target_timeline.
The default value of recovery_target_timeline was changed in v12,
but the description about the default behavior of that was not updated.
Back-patch to v12 where the default behavior of recovery_target_timeline
was changed.
Author: Benoit Lobréau
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CAPE8EZ7c3aruEmM24GYkj8y8WmHKD1m9TtPtgCF0nQ3zw4LCkQ@mail.gmail.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:51:21 +0000 (11:51 +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:08:59 +0000 (11:08 +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
Tom Lane [Mon, 4 Jan 2021 23:32:40 +0000 (18:32 -0500)]
Fix integer-overflow corner cases in substring() functions.
If the substring start index and length overflow when added together,
substring() misbehaved, either throwing a bogus "negative substring
length" error on a case that should succeed, or failing to complain that
a negative length is negative (and instead returning the whole string,
in most cases). Unsurprisingly, the text, bytea, and bit variants of
the function all had this issue. Rearrange the logic to ensure that
negative lengths are always rejected, and add an overflow check to
handle the other case.
Also install similar guards into detoast_attr_slice() (nee
heap_tuple_untoast_attr_slice()), since it's far from clear that
no other code paths leading to that function could pass it values
that would overflow.
Patch by myself and Pavel Stehule, per bug #16804 from Rafi Shamim.
Back-patch to v11. While these bugs are old, the common/int.h
infrastructure for overflow-detecting arithmetic didn't exist before
commit
4d6ad3125, and it doesn't seem like these misbehaviors are bad
enough to justify developing a standalone fix for the older branches.
Discussion: https://postgr.es/m/16804-
f4eeeb6c11ba71d4@postgresql.org
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
Peter Geoghegan [Thu, 31 Dec 2020 01:21:41 +0000 (17:21 -0800)]
Get heap page max offset with buffer lock held.
On further reflection it seems better to call PageGetMaxOffsetNumber()
after acquiring a buffer lock on the page. This shouldn't really
matter, but doing it this way is cleaner.
Follow-up to commit
42288174.
Backpatch: 12-, just like commit
42288174
Peter Geoghegan [Thu, 31 Dec 2020 00:29:03 +0000 (16:29 -0800)]
Fix index deletion latestRemovedXid bug.
The logic for determining the latest removed XID for the purposes of
generating recovery conflicts in REDO routines was subtly broken. It
failed to follow links from HOT chains, and so failed to consider all
relevant heap tuple headers in some cases.
To fix, expand the loop that deals with LP_REDIRECT line pointers to
also deal with HOT chains. The new version of the loop is loosely based
on a similar loop from heap_prune_chain().
The impact of this bug is probably quite limited, since the horizon code
necessarily deals with heap tuples that are pointed to by LP_DEAD-set
index tuples. The process of setting LP_DEAD index tuples (e.g. within
the kill_prior_tuple mechanism) is highly correlated with opportunistic
pruning of pointed-to heap tuples. Plus the question of generating a
recovery conflict usually comes up some time after index tuple LP_DEAD
bits were initially set, unlike heap pruning, where a latestRemovedXid
is generated at the point of the pruning operation (heap pruning has no
deferred "would-be page split" style processing that produces conflicts
lazily).
Only backpatch to Postgres 12, the first version where this logic runs
during original execution (following commit
558a9165e08). The index
latestRemovedXid mechanism has had the same bug since it first appeared
over 10 years ago (in commit
a760893d), but backpatching to all
supported versions now seems like a bad idea on balance. Running the
new improved code during recovery seems risky, especially given the lack
of complaints from the field.
Author: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-Wz=Eib393+HHcERK_9MtgNS7Ew1HY=RDC_g6GL46zM5C6Q@mail.gmail.com
Backpatch: 12-
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
Tom Lane [Wed, 30 Dec 2020 16:38:42 +0000 (11:38 -0500)]
Fix up usage of krb_server_keyfile GUC parameter.
secure_open_gssapi() installed the krb_server_keyfile setting as
KRB5_KTNAME unconditionally, so long as it's not empty. However,
pg_GSS_recvauth() only installed it if KRB5_KTNAME wasn't set already,
leading to a troubling inconsistency: in theory, clients could see
different sets of server principal names depending on whether they
use GSSAPI encryption. Always using krb_server_keyfile seems like
the right thing, so make both places do that. Also fix up
secure_open_gssapi()'s lack of a check for setenv() failure ---
it's unlikely, surely, but security-critical actions are no place
to be sloppy.
Also improve the associated documentation.
This patch does nothing about secure_open_gssapi()'s use of setenv(),
and indeed causes pg_GSS_recvauth() to use it too. That's nominally
against project portability rules, but since this code is only built
with --with-gssapi, I do not feel a need to do something about this
in the back branches. A fix will be forthcoming for HEAD though.
Back-patch to v12 where GSSAPI encryption was introduced. The
dubious behavior in pg_GSS_recvauth() goes back further, but it
didn't have anything to be inconsistent with, so let it be.
Discussion: https://postgr.es/m/
2187460.
1609263156@sss.pgh.pa.us
Noah Misch [Wed, 30 Dec 2020 09:43:43 +0000 (01:43 -0800)]
In pg_upgrade cross-version test, handle lack of oldstyle_length().
This suffices for testing v12 -> v13; some other version pairs need more
changes. Back-patch to v10, which removed the function.
Michael Paquier [Tue, 29 Dec 2020 09:18:59 +0000 (18:18 +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[email protected]
Backpatch-through: 9.5
Tom Lane [Mon, 28 Dec 2020 22:58:58 +0000 (17:58 -0500)]
Improve log messages related to pg_hba.conf not matching a connection.
Include details on whether GSS encryption has been activated;
since we added "hostgssenc" type HBA entries, that's relevant info.
Kyotaro Horiguchi and Tom Lane. Back-patch to v12 where
GSS encryption was introduced.
Discussion: https://postgr.es/m/
e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se
Tom Lane [Mon, 28 Dec 2020 22:44:17 +0000 (17:44 -0500)]
Fix assorted issues in backend's GSSAPI encryption support.
Unrecoverable errors detected by GSSAPI encryption can't just be
reported with elog(ERROR) or elog(FATAL), because attempting to
send the error report to the client is likely to lead to infinite
recursion or loss of protocol sync. Instead make this code do what
the SSL encryption code has long done, which is to just report any
such failure to the server log (with elevel COMMERROR), then pretend
we've lost the connection by returning errno = ECONNRESET.
Along the way, fix confusion about whether message translation is done
by pg_GSS_error() or its callers (the latter should do it), and make
the backend version of that function work more like the frontend
version.
Avoid allocating the port->gss struct until it's needed; we surely
don't need to allocate it in the postmaster.
Improve logging of "connection authorized" messages with GSS enabled.
(As part of this, I back-patched the code changes from
dc11f31a1.)
Make BackendStatusShmemSize() account for the GSS-related space that
will be allocated by CreateSharedBackendStatus(). This omission
could possibly cause out-of-shared-memory problems with very high
max_connections settings.
Remove arbitrary, pointless restriction that only GSS authentication
can be used on a GSS-encrypted connection.
Improve documentation; notably, document the fact that libpq now
prefers GSS encryption over SSL encryption if both are possible.
Per report from Mikael Gustavsson. Back-patch to v12 where
this code was introduced.
Discussion: https://postgr.es/m/
e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se
Tom Lane [Mon, 28 Dec 2020 20:43:44 +0000 (15:43 -0500)]
Fix bugs in libpq's GSSAPI encryption support.
The critical issue fixed here is that if a GSSAPI-encrypted connection
is successfully made, pqsecure_open_gss() cleared conn->allow_ssl_try,
as an admittedly-hacky way of preventing us from then trying to tunnel
SSL encryption over the already-encrypted connection. The problem
with that is that if we abandon the GSSAPI connection because of a
failure during authentication, we would not attempt SSL encryption
in the next try with the same server. This can lead to unexpected
connection failure, or silently getting a non-encrypted connection
where an encrypted one is expected.
Fortunately, we'd only manage to make a GSSAPI-encrypted connection
if both client and server hold valid tickets in the same Kerberos
infrastructure, which is a relatively uncommon environment.
Nonetheless this is a very nasty bug with potential security
consequences. To fix, don't reset the flag, instead adding a
check for conn->gssenc being already true when deciding whether
to try to initiate SSL.
While here, fix some lesser issues in libpq's GSSAPI code:
* Use the need_new_connection stanza when dropping an attempted
GSSAPI connection, instead of partially duplicating that code.
The consequences of this are pretty minor: AFAICS it could only
lead to auth_req_received or password_needed remaining set when
they shouldn't, which is not too harmful.
* Fix pg_GSS_error() to not repeat the "mprefix" it's given multiple
times, and to notice any failure return from gss_display_status().
* Avoid gratuitous dependency on NI_MAXHOST in
pg_GSS_load_servicename().
Per report from Mikael Gustavsson. Back-patch to v12 where
this code was introduced.
Discussion: https://postgr.es/m/
e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se
Tom Lane [Mon, 28 Dec 2020 17:13:40 +0000 (12:13 -0500)]
Expose the default for channel_binding in PQconndefaults().
If there's a static default value for a connection option,
it should be shown in the PQconninfoOptions array.
Daniele Varrazzo
Discussion: https://postgr.es/m/CA+mi_8Zo8Rgn7p+6ZRY7QdDu+23ukT9AvoHNyPbgKACxwgGhZA@mail.gmail.com
Tom Lane [Mon, 28 Dec 2020 16:55:23 +0000 (11:55 -0500)]
Further fix thinko in plpgsql memory leak fix.
There's a second call of get_eval_mcontext() that should also be
get_stmt_mcontext(). This is actually dead code, since no
interesting allocations happen before switching back to the
original context, but we should keep it in sync with the other
call to forestall possible future bugs.
Discussion: https://postgr.es/m/
f075f7be-c654-9aa8-3ffc-
e9214622f02a@enterprisedb.com
Tom Lane [Mon, 28 Dec 2020 16:41:25 +0000 (11:41 -0500)]
Fix thinko in plpgsql memory leak fix.
Commit
a6b1f5365 intended to place the transient "target" list of
a CALL statement in the function's statement-lifespan context,
but I fat-fingered that and used get_eval_mcontext() instead of
get_stmt_mcontext(). The eval_mcontext belongs to the "simple
expression" infrastructure, which is destroyed at transaction end.
The net effect is that a CALL in a procedure to another procedure
that has OUT or INOUT parameters would fail if the called procedure
did a COMMIT.
Per report from Peter Eisentraut. Back-patch to v11, like the
prior patch.
Discussion: https://postgr.es/m/
f075f7be-c654-9aa8-3ffc-
e9214622f02a@enterprisedb.com
Michael Paquier [Mon, 28 Dec 2020 13:16:57 +0000 (22:16 +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 10:57:51 +0000 (19:57 +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
Jeff Davis [Sun, 27 Dec 2020 20:09:00 +0000 (12:09 -0800)]
Second attempt to stabilize
05c02589.
Removing the EXPLAIN test to stabilize the buildfarm. The execution
test should still be effective to catch the bug even if the plan is
slightly different on different platforms.
Jeff Davis [Sun, 27 Dec 2020 17:47:23 +0000 (09:47 -0800)]
Stabilize test introduced in
05c02589, per buildfarm.
In passing, make the capitalization match the rest of the file.
Reported-by: Tom Lane
Jeff Davis [Sun, 27 Dec 2020 01:25:30 +0000 (17:25 -0800)]
Fix bug #16784 in Disk-based Hash Aggregation.
Before processing tuples, agg_refill_hash_table() was setting all
pergroup pointers to NULL to signal to advance_aggregates() that it
should not attempt to advance groups that had spilled.
The problem was that it also set the pergroups for sorted grouping
sets to NULL, which caused rescanning to fail.
Instead, change agg_refill_hash_table() to only set the pergroups for
hashed grouping sets to NULL; and when compiling the expression, pass
doSort=false.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16784-
7ff169bf2c3d1588%40postgresql.org
Backpatch-through: 13
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[email protected]
Tom Lane [Fri, 25 Dec 2020 02:37:46 +0000 (21:37 -0500)]
Avoid time-of-day-dependent failure in log rotation test.
Buildfarm members pogona and petalura have shown a failure when
pg_ctl/t/004_logrotate.pl starts just before local midnight.
The default rotate-at-midnight behavior occurs just before the
Perl script examines current_logfiles, so it figures that the
rotation it's already requested has occurred ... but in reality,
that rotation happens just after it looks, so the expected new
log data goes into a different file than the one it's examining.
In HEAD, src/test/kerberos/t/001_auth.pl has acquired similar code
that evidently has a related failure mode. Besides being quite new,
few buildfarm critters run that test, so it's unsurprising that
we've not yet seen a failure there.
Fix both cases by setting log_rotation_age = 0 so that no time-based
rotation can occur. Also absorb 004_logrotate.pl's decision to
set lc_messages = 'C' into the kerberos test, in hopes that it will
work in non-English prevailing locales.
Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pogona&dt=2020-12-24%2022%3A10%3A04
Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=petalura&dt=2020-02-01%2022%3A20%3A04
Tom Lane [Thu, 24 Dec 2020 22:00:43 +0000 (17:00 -0500)]
Fix race condition between shutdown and unstarted background workers.
If a database shutdown (smart or fast) is commanded between the time
some process decides to request a new background worker and the time
that the postmaster can launch that worker, then nothing happens
because the postmaster won't launch any bgworkers once it's exited
PM_RUN state. This is fine ... unless the requesting process is
waiting for that worker to finish (or even for it to start); in that
case the requestor is stuck, and only manual intervention will get us
to the point of being able to shut down.
To fix, cancel pending requests for workers when the postmaster sends
shutdown (SIGTERM) signals, and similarly cancel any new requests that
arrive after that point. (We can optimize things slightly by only
doing the cancellation for workers that have waiters.) To fit within
the existing bgworker APIs, the "cancel" is made to look like the
worker was started and immediately stopped, causing deregistration of
the bgworker entry. Waiting processes would have to deal with
premature worker exit anyway, so this should introduce no bugs that
weren't there before. We do have a side effect that registration
records for restartable bgworkers might disappear when theoretically
they should have remained in place; but since we're shutting down,
that shouldn't matter.
Back-patch to v10. There might be value in putting this into 9.6
as well, but the management of bgworkers is a bit different there
(notably see
8ff518699) and I'm not convinced it's worth the effort
to validate the patch for that branch.
Discussion: https://postgr.es/m/661570.
1608673226@sss.pgh.pa.us
Bruce Momjian [Wed, 23 Dec 2020 14:37:38 +0000 (09:37 -0500)]
docs: document which server-side languages can create procs
This was missed when the feature was added.
Reported-by: Daniel Westermann
Discussion: https://postgr.es/m/
160624532969.25818.
4767632047905006142@wrigleys.postgresql.org
Backpatch-through: 11
Michael Paquier [Wed, 23 Dec 2020 03:51:35 +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 [Tue, 22 Dec 2020 18:23:49 +0000 (13:23 -0500)]
Improve autoprewarm's handling of early-shutdown scenarios.
Bad things happen if the DBA issues "pg_ctl stop -m fast" before
autoprewarm finishes loading its list of blocks to prewarm.
The current worker process successfully terminates early, but
(if this wasn't the last database with blocks to prewarm) the
leader process will just try to launch another worker for the
next database. Since the postmaster is now in PM_WAIT_BACKENDS
state, it ignores the launch request, and the leader just sits
until it's killed manually.
This is mostly the fault of our half-baked design for launching
background workers, but a proper fix for that is likely to be
too invasive to be back-patchable. To ameliorate the situation,
fix apw_load_buffers() to check whether SIGTERM has arrived
just before trying to launch another worker. That leaves us with
only a very narrow window in each worker launch where SIGTERM
could occur between the launch request and successful worker start.
Another issue is that if the leader process does manage to exit,
it unconditionally rewrites autoprewarm.blocks with only the
blocks currently in shared buffers, thus forgetting any blocks
that we hadn't reached yet while prewarming. This seems quite
unhelpful, since the next database start will then not have the
expected prewarming benefit. Fix it to not modify the file if
we shut down before the initial load attempt is complete.
Per bug #16785 from John Thompson. Back-patch to v11 where
the autoprewarm code was introduced.
Discussion: https://postgr.es/m/16785-
c0207d8c67fb5f25@postgresql.org
Tomas Vondra [Tue, 22 Dec 2020 01:00:39 +0000 (02:00 +0100)]
Improve find_em_expr_usable_for_sorting_rel comment
Clarify the relationship between find_em_expr_usable_for_sorting_rel and
prepare_sort_from_pathkeys, i.e. what restrictions need to be shared
between those two places.
Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
Tomas Vondra [Mon, 21 Dec 2020 18:37:14 +0000 (19:37 +0100)]
Don't search for volatile expr in find_em_expr_usable_for_sorting_rel
While prepare_sort_from_pathkeys has to be concerned about matching up
a volatile expression to the proper tlist entry, we don't need to do
that in find_em_expr_usable_for_sorting_rel becausee such a sort will
have to be postponed anyway.
Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
Tomas Vondra [Mon, 21 Dec 2020 17:58:32 +0000 (18:58 +0100)]
Disallow SRFs when considering sorts below Gather Merge
While we do allow SRFs in ORDER BY, scan/join processing should not
consider such cases - such sorts should only happen via final Sort atop
a ProjectSet. So make sure we don't try adding such sorts below Gather
Merge, just like we do for expressions that are volatile and/or not
parallel safe.
Backpatch to PostgreSQL 13, where this code was introduced as part of
the Incremental Sort patch.
Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
Discussion: https://postgr.es/m/295524.
1606246314%40sss.pgh.pa.us
Tom Lane [Mon, 21 Dec 2020 18:11:29 +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
Tomas Vondra [Mon, 21 Dec 2020 17:29:46 +0000 (18:29 +0100)]
Check parallel safety in generate_useful_gather_paths
Commit
ebb7ae839d ensured we ignore pathkeys with volatile expressions
when considering adding a sort below a Gather Merge. Turns out we need
to care about parallel safety of the pathkeys too, otherwise we might
try sorting e.g. on results of a correlated subquery (as demonstrated
by a report from Luis Roberto).
Initial investigation by Tom Lane, patch by James Coleman. Backpatch
to 13, where the code was instroduced (as part of Incremental Sort).
Reported-by: Luis Roberto
Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/
622580997.
37108180.
1604080457319.JavaMail.zimbra%40siscobra.com.br
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
Tomas Vondra [Mon, 21 Dec 2020 17:16:16 +0000 (18:16 +0100)]
Consider unsorted paths in generate_useful_gather_paths
generate_useful_gather_paths used to skip unsorted paths (without any
pathkeys), but that is unnecessary - the later code actually can handle
such paths just fine by adding a Sort node. This is clearly a thinko,
preventing construction of useful plans.
Backpatch to 13, where Incremental Sort was introduced.
Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
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
Michael Paquier [Wed, 16 Dec 2020 01:39:29 +0000 (10:39 +0900)]
doc: Fix explanation related to pg_shmem_allocations
Offsets are shown as NULL only for anonymous allocations.
Author: Benoit Lobréau
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CAPE8EZ5Lnoyqoz7aZpvQM0E8sW+hw+k6G2NULe+m4arFRrA1aA@mail.gmail.com
Backpatch-through: 13
Bruce Momjian [Wed, 16 Dec 2020 00:20:15 +0000 (19:20 -0500)]
doc: clarify COPY TO for partitioning/inheritance
It was not clear how COPY TO behaved with partitioning/inheritance
because the paragraphs were so far apart. Also reword to simplify.
Discussion: https://postgr.es/m/
20201203211723[email protected]
Author: Justin Pryzby
Backpatch-through: 10
Jeff Davis [Tue, 15 Dec 2020 07:48:44 +0000 (23:48 -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:47 +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:38 +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
Andres Freund [Tue, 8 Dec 2020 02:21:06 +0000 (18:21 -0800)]
jit: Correct parameter type for generated expression evaluation functions.
clang only uses the 'i1' type for scalar booleans, not for pointers to
booleans (as the pointer might be pointing into a larger memory
allocation). Therefore a pointer-to-bool needs to the "storage" boolean.
There's no known case of wrong code generation due to this, but it seems quite
possible that it could cause problems (see e.g.
72559438f92).
Author: Andres Freund
Discussion: https://postgr.es/m/
20201207212142[email protected]
Backpatch: 11-, where jit support was added
Andres Freund [Tue, 8 Dec 2020 02:12:23 +0000 (18:12 -0800)]
jit: configure: Explicitly reference 'native' component.
Until recently 'native' was implicitly included via 'orcjit', but a change
included in LLVM 11 (not yet released) removed a number of such indirect
component references.
Reported-By: Fabien COELHO
Reported-By: Andres Freund
Reported-By: Thomas Munro
Author: Andres Freund
Discussion: https://postgr.es/m/20201201064949[email protected]
Backpatch: 11-, where jit support was added
Andres Freund [Tue, 10 Nov 2020 04:01:33 +0000 (20:01 -0800)]
backpatch "jit: Add support for LLVM 12."
As there haven't been problem on the buildfarm due to this change,
backpatch
6c57f2ed16e now.
Author: Andres Freund
Discussion: https://postgr.es/m/
20201016011244[email protected]
Backpatch: 11-, where jit support was added
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
Tom Lane [Sat, 5 Dec 2020 21:16:13 +0000 (16:16 -0500)]
Fix missed step in removal of useless RESULT RTEs in the planner.
Commit
4be058fe9 forgot that the append_rel_list would already be
populated at the time we remove useless result RTEs, and it might contain
PlaceHolderVars that need to be adjusted like the ones in the main parse
tree. This could lead to "no relation entry for relid N" failures later
on, when the planner tries to do something with an unadjusted PHV.
Per report from Tom Ellis. Back-patch to v12 where the bug came in.
Discussion: https://postgr.es/m/
20201205173056.GF30712@cloudinit-builder
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 16:33:24 +0000 (11:33 -0500)]
doc: remove unnecessary blank before command option text
Backpatch-through: 11
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
Bruce Momjian [Wed, 2 Dec 2020 01:27:06 +0000 (20:27 -0500)]
pg_checksums: data_checksum_version is unsigned so use %u not %d
While the previous behavior didn't generate a warning, we might as well
use an accurate *printf specification.
Backpatch-through: 12
Tom Lane [Tue, 1 Dec 2020 19:02:27 +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
Alvaro Herrera [Tue, 1 Dec 2020 14:46:56 +0000 (11:46 -0300)]
Avoid memcpy() with a NULL source pointer and count == 0
When memcpy() is called on a pointer, the compiler is entitled to assume
that the pointer is not null, which can lead to optimizing nearby code
in potentially undesirable ways. We still want such optimizations
(gcc's -fdelete-null-pointer-checks) in cases where they're valid.
Related: commit
13bba02271dc.
Backpatch to pg11, where this particular instance appeared.
Reported-by: Ranier Vilela
Reported-by: Zhihong Yu
Discussion: https://postgr.es/m/CAEudQApUndmQkr5fLrCKXQ7+ib44i7S+Kk93pyVThS85PnG3bQ@mail.gmail.com
Discussion: https://postgr.es/m/CALNJ-vSdhwSM5f4tnNn1cdLHvXMVe_S+V3nR5GwNrmCPNB2VtQ@mail.gmail.com
Thomas Munro [Tue, 1 Dec 2020 00:21:03 +0000 (13:21 +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
Reviewed-by: Pavel Borisov
Reviewed-by: Neil Chen
Reviewed-by: David Zhang
Discussion: https://postgr.es/m/16663-
fe97ccf9932fc800%40postgresql.org
Tom Lane [Mon, 30 Nov 2020 21:32:56 +0000 (16:32 -0500)]
Fix missing outfuncs.c support for IncrementalSortPath.
For debugging purposes, Path nodes are supposed to have outfuncs
support, but this was overlooked in the original incremental sort patch.
While at it, clean up a couple other minor oversights, as well as
bizarre choice of return type for create_incremental_sort_path().
(All the existing callers just cast it to "Path *" immediately, so
they don't care, but some future caller might care.)
outfuncs.c fix by Zhijie Hou, the rest by me
Discussion: https://postgr.es/m/
324c4d81d8134117972a5b1f6cdf9560@G08CNEXMBPEKD05.g08.fujitsu.local
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
Reviewed-by: David Johnston
Discussion: https://postgr.es/m/CAAaqYe994=PUrn8CJZ4UEo_S-FfRr_3ogERyhtdgHAb2WG_Ufg@mail.gmail.com
Tom Lane [Mon, 30 Nov 2020 20:24:13 +0000 (15:24 -0500)]
Remove configure-time probe for DocBook DTD.
Checking for DocBook being installed was valuable when we were on the
OpenSP docs toolchain, because that was rather hard to get installed
fully. Nowadays, as long as you have xmllint and xsltproc installed,
you're good, because those programs will fetch the DocBook files off
the net at need. Moreover, testing this at configure time means that
a network access may well occur whether or not you have any interest
in building the docs later. That can be slow (typically 2 or 3
seconds, though much higher delays have been reported), and it seems
not very nice to be doing an off-machine access without warning, too.
Hence, drop the PGAC_CHECK_DOCBOOK probe, and adjust related
documentation. Without that macro, there's not much left of
config/docbook.m4 at all, so I just removed it.
Back-patch to v11, where we started to use xmllint in the
PGAC_CHECK_DOCBOOK probe.
Discussion: https://postgr.es/m/
E2EE6B76-2D96-408A-B961-
CAE47D1A86F0@yesql.se
Discussion: https://postgr.es/m/
A55A7FC9-FA60-47FE-98B5-
139CDC57CE6E@gmail.com
Tom Lane [Mon, 30 Nov 2020 19:38:00 +0000 (14:38 -0500)]
Prevent parallel index build in a standalone backend.
This can't work if there's no postmaster, and indeed the code got an
assertion failure trying. There should be a check on IsUnderPostmaster
gating the use of parallelism, as the planner has for ordinary
parallel queries.
Commit
40d964ec9 got this right, so follow its model of checking
IsUnderPostmaster at the same place where we check for
max_parallel_maintenance_workers == 0. In general, new code
implementing parallel utility operations should do the same.
Report and patch by Yulin Pei, cosmetically adjusted by me.
Back-patch to v11 where this code came in.
Discussion: https://postgr.es/m/HK0PR01MB22747D839F77142D7E76A45DF4F50@HK0PR01MB2274.apcprd01.prod.exchangelabs.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/
[email protected]
Heikki Linnakangas [Mon, 30 Nov 2020 08:26:43 +0000 (10:26 +0200)]
Remove leftover comments, left behind by removal of WITH OIDS.
Author: Amit Langote
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGaRoF3XrhPW-Y7P%2BG7bKo84Z_h%3DkQHvMh-80%3Dav3wmOw%40mail.gmail.com
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
Tom Lane [Sun, 29 Nov 2020 18:58:30 +0000 (13:58 -0500)]
Doc: clarify behavior of PQconnectdbParams().
The documentation omitted the critical tidbit that a keyword-array entry
is simply ignored if its corresponding value-array entry is NULL or an
empty string; it will *not* override any previously-obtained value for
the parameter. (See conninfo_array_parse().) I'd supposed that would
force the setting back to default, which is what led me into bug #16746;
but it doesn't.
While here, I couldn't resist the temptation to do some copy-editing,
both in the description of PQconnectdbParams() and in the section
about connection URI syntax.
Discussion: https://postgr.es/m/931505.
1606618746@sss.pgh.pa.us
Noah Misch [Sun, 29 Nov 2020 05:52:27 +0000 (21:52 -0800)]
Retry initial slurp_file("current_logfiles"), in test 004_logrotate.pl.
Buildfarm member topminnow failed when the test script attempted this
before the syslogger would have created the file. Back-patch to v12,
which introduced the test.
Tom Lane [Sat, 28 Nov 2020 19:03:40 +0000 (14:03 -0500)]
Fix a recently-introduced race condition in LISTEN/NOTIFY handling.
Commit
566372b3d fixed some race conditions involving concurrent
SimpleLruTruncate calls, but it introduced new ones in async.c.
A newly-listening backend could attempt to read Notify SLRU pages that
were in process of being truncated, possibly causing an error. Also,
the QUEUE_TAIL pointer could become set to a value that's not equal to
the queue position of any backend. While that's fairly harmless in
v13 and up (thanks to commit
51004c717), in older branches it resulted
in near-permanent disabling of the queue truncation logic, so that
continued use of NOTIFY led to queue-fill warnings and eventual
inability to send any more notifies. (A server restart is enough to
make that go away, but it's still pretty unpleasant.)
The core of the problem is confusion about whether QUEUE_TAIL
represents the "logical" tail of the queue (i.e., the oldest
still-interesting data) or the "physical" tail (the oldest data we've
not yet truncated away). To fix, split that into two variables.
QUEUE_TAIL regains its definition as the logical tail, and we
introduce a new variable to track the oldest un-truncated page.
Per report from Mikael Gustavsson. Like the previous patch,
back-patch to all supported branches.
Discussion: https://postgr.es/m/
1b8561412e8a4f038d7a491c8b922788@smhi.se