Andrew Dunstan [Sat, 20 Nov 2021 22:54:43 +0000 (17:54 -0500)]
Require version 0.98 of Test::More for TAP tests
This means that the subtest feature will be available for use.
We expect that this change will make prairiedog go red until it is
updated, but other buildfarm animals should be fine.
Discussion: https://postgr.es/m/
f5e1d308-4e33-37a7-bdf1-
f6e0c75119de@dunslane.net
Tom Lane [Sat, 20 Nov 2021 19:29:56 +0000 (14:29 -0500)]
Fix SP-GiST scan initialization logic for binary-compatible cases.
Commit
ac9099fc1 rearranged the logic in spgGetCache() that determines
the index's attType (nominal input data type) and leafType (actual
type stored in leaf index tuples). Turns out this broke things for
the case where (a) the actual input data type is different from the
nominal type, (b) the opclass's config function leaves leafType
defaulted, and (c) the opclass has no "compress" function. (b) caused
us to assign the actual input data type as leafType, and then since
that's not attType, we complained that a "compress" function is
required. For non-polymorphic opclasses, condition (a) arises in
binary-compatible cases, such as using SP-GiST text_ops for a varchar
column, or using any opclass on a domain over its nominal input type.
To fix, use attType for leafType when the index's declared column type
is different from but binary-compatible with attType. Do this only in
the defaulted-leafType case, to avoid overriding any explicit
selection made by the opclass.
Per bug #17294 from Ilya Anfimov. Back-patch to v14.
Discussion: https://postgr.es/m/17294-
8f6c7962ce877edc@postgresql.org
Tom Lane [Fri, 19 Nov 2021 17:11:38 +0000 (12:11 -0500)]
Allow psql's other uses of simple_prompt() to be interrupted by ^C.
This fills in the work left un-done by
5f1148224. \prompt can
be canceled out of now, and so can password prompts issued during
\connect. (We don't need to do anything for password prompts
issued during startup, because we aren't yet trapping SIGINT
at that point.)
Nathan Bossart
Discussion: https://postgr.es/m/747443.
1635536754@sss.pgh.pa.us
Andres Freund [Fri, 19 Nov 2021 16:43:12 +0000 (08:43 -0800)]
Initialize backend status reporting during bootstrap.
This allows a later commit to reduce the number of branches in performance
sensitive functions during normal running, compared to a very minor saving
during bootstrapping.
Author: Melanie Plageman <
[email protected]>
Reviewed-By: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CAAKRu_Yeg+vh6SHNEo1+=O7e-BPX35cU0XQM=YwQRnkFyv_y+w@mail.gmail.com
Amit Kapila [Fri, 19 Nov 2021 03:34:40 +0000 (09:04 +0530)]
Fix parallel operations that prevent oldest xmin from advancing.
While determining xid horizons, we skip over backends that are running
Vacuum. We also ignore Create Index Concurrently, or Reindex Concurrently
for the purposes of computing Xmin for Vacuum. But we were not setting the
flags corresponding to these operations when they are performed in
parallel which was preventing Xid horizon from advancing.
The optimization related to skipping Create Index Concurrently, or Reindex
Concurrently operations was implemented in PG-14 but the fix is the same
for the Parallel Vacuum as well so back-patched till PG-13.
Author: Masahiko Sawada
Reviewed-by: Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/CAD21AoCLQqgM1sXh9BrDFq0uzd3RBFKi=Vfo6cjjKODm0Onr5w@mail.gmail.com
Michael Paquier [Fri, 19 Nov 2021 02:02:15 +0000 (11:02 +0900)]
Improve psql tab completion for transforms, domains and sequences
The following improvements are done:
- Addition of some tab completion for CREATE DOMAIN.
- Addition of some tab completion for CREATE TRANSFORM.
- Addition of type completion for CREATE SEQUENCE AS.
Author: Ken Kato
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/
8d370135aef066659eef8e8fbfa6315b@oss.nttdata.com
Tom Lane [Thu, 18 Nov 2021 19:50:13 +0000 (14:50 -0500)]
Use appropriate -Wno-warning switches when compiling bitcode.
We use "clang" to compile bitcode files for LLVM inlining. That might
be different from the build's main C compiler, so it needs its own set
of compiler flags. To simplify configure, we don't bother adding any
-W switches to that flag set; there's little need since the main build
will show us any warnings. However, if we don't want to see unwanted
warnings, we still have to add any -Wno-warning switches we'd normally
use with clang.
This escaped notice before commit
9ff47ea41, which tried to add
-Wno-compound-token-split-by-macro; buildfarm animals using mismatched
CC and CLANG still showed those warnings. I'm not sure why we never
saw any effects from the lack of -Wno-unused-command-line-argument
(maybe that's only activated by -Wall?). clang does not currently
support -Wno-format-truncation or -Wno-stringop-truncation, although
in the interests of future-proofing and consistency I included tests
for those.
Back-patch to v11 where we started building bitcode files.
Discussion: https://postgr.es/m/
2921539.
1637254619@sss.pgh.pa.us
Michael Paquier [Thu, 18 Nov 2021 03:52:49 +0000 (12:52 +0900)]
Fix quoting of ACL item in table for upgrade binary compatibility checks
Per buildfarm member prion, that runs the regression tests under a role
name that uses a hyphen. Issue introduced by
835bcba.
Discussion: https://postgr.es/m/
[email protected]
Backpatch-through: 12
Michael Paquier [Thu, 18 Nov 2021 01:37:15 +0000 (10:37 +0900)]
Add table to regression tests for binary-compatibility checks in pg_upgrade
This commit adds to the main regression test suite a table with all
the in-core data types (some exceptions apply). This table is not
dropped, so as pg_upgrade would be able to check the binary
compatibility of the types tracked in the table. If a new type is added
in core, this part of the tests would need a refresh but the tests are
designed to fail if that were to happen.
As this is useful for upgrades and that these rely on the objects
created in the regression test suite of the old version upgraded from,
a backpatch down to 12 is done, which is the last point where a binary
incompatible change has been done (
7c15cef). This will hopefully be
enough to find out if something gets broken during the development of a
new version of Postgres, so as it is possible to take actions in
pg_upgrade itself in this case (like
0ccfc28 for sql_identifier).
An area that is not covered yet is related to external modules, which
may create their own types. The testing infrastructure of pg_upgrade is
not integrated yet with the external modules stored in core
(src/test/modules/ or contrib/, all use the same database name for their
tests so there would be an overlap). This could be improved in the
future.
Author: Justin Pryzby
Reviewed-by: Jacob Champion, Peter Eisentraut, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/
20201206180248[email protected]
Backpatch-through: 12
Tom Lane [Thu, 18 Nov 2021 00:09:54 +0000 (19:09 -0500)]
Provide a variant of simple_prompt() that can be interrupted by ^C.
Up to now, you couldn't escape out of psql's \password command
by typing control-C (or other local spelling of SIGINT). This
is pretty user-unfriendly, so improve it. To do so, we have to
modify the functions provided by pg_get_line.c; but we don't
want to mess with psql's SIGINT handler setup, so provide an
API that lets that handler cause the cancel to occur.
This relies on the assumption that we won't do any major harm by
longjmp'ing out of fgets(). While that's obviously a little shaky,
we've long had the same assumption in the main input loop, and few
issues have been reported.
psql has some other simple_prompt() calls that could usefully
be improved the same way; for now, just deal with \password.
Nathan Bossart, minor tweaks by me
Discussion: https://postgr.es/m/747443.
1635536754@sss.pgh.pa.us
Tom Lane [Wed, 17 Nov 2021 21:54:12 +0000 (16:54 -0500)]
Add a planner support function for starts_with().
This fills in some gaps in planner support for starts_with() and
the equivalent ^@ operator:
* A condition such as "textcol ^@ constant" can now use a regular
btree index, not only an SP-GiST index, so long as the index's
collation is C. (This works just like "textcol LIKE 'foo%'".)
* "starts_with(textcol, constant)" can be optimized the same as
"textcol ^@ constant".
* Fixed-prefix LIKE and regex patterns are now more like starts_with()
in another way: if you apply one to an SPGiST-indexed column, you'll
get an index condition using ^@ rather than two index conditions with
>= and <.
Per a complaint from Shay Rojansky. Patch by me; thanks to
Nathan Bossart for review.
Discussion: https://postgr.es/m/232599.
1633800229@sss.pgh.pa.us
Tom Lane [Wed, 17 Nov 2021 19:16:34 +0000 (14:16 -0500)]
Clean up error handling in pg_basebackup's walmethods.c.
The error handling here was a mess, as a result of a fundamentally
bad design (relying on errno to keep its value much longer than is
safe to assume) as well as a lot of just plain sloppiness, both as
to noticing errors at all and as to reporting the correct errno.
Moreover, the recent addition of LZ4 compression broke things
completely, because liblz4 doesn't use errno to report errors.
To improve matters, keep the error state in the DirectoryMethodData or
TarMethodData struct, and add a string field so we can handle cases
that don't set errno. (The tar methods already had a version of this,
but it can be done more efficiently since all these cases use a
constant error string.) Make the dir and tar methods handle errors
in basically identical ways, which they didn't before.
This requires copying errno into the state struct in a lot of places,
which is a bit tedious, but it has the virtue that we can get rid of
ad-hoc code to save and restore errno in a number of places ... not
to mention that it fixes other places that should've saved/restored
errno but neglected to.
In passing, fix some pointlessly static buffers to be ordinary
local variables.
There remains an issue about exactly how to handle errors from
fsync(), but that seems like material for its own patch.
While the LZ4 problems are new, all the rest of this is fixes for
old bugs, so backpatch to v10 where walmethods.c was introduced.
Patch by me; thanks to Michael Paquier for review.
Discussion: https://postgr.es/m/
1343113.
1636489231@sss.pgh.pa.us
Tom Lane [Wed, 17 Nov 2021 18:08:25 +0000 (13:08 -0500)]
Handle close() failures more robustly in pg_dump and pg_basebackup.
Coverity complained that applying get_gz_error after a failed gzclose,
as we did in one place in pg_basebackup, is unsafe. I think it's
right: it's entirely likely that the call is touching freed memory.
Change that to inspect errno, as we do for other gzclose calls.
Also, be careful to initialize errno to zero immediately before any
gzclose() call where we care about the error status. (There are
some calls where we don't, because we already failed at some previous
step.) This ensures that we don't get a misleadingly irrelevant
error code if gzclose() fails in a way that doesn't set errno.
We could work harder at that, but it looks to me like all such cases
are basically can't-happen if we're not misusing zlib, so it's
not worth the extra notational cruft that would be required.
Also, fix several places that simply failed to check for close-time
errors at all, mostly at some remove from the close or gzclose itself;
and one place that did check but didn't bother to report the errno.
Back-patch to v12. These mistakes are older than that, but between
the frontend logging API changes that happened in v12 and the fact
that frontend code can't rely on %m before that, the patch would need
substantial revision to work in older branches. It doesn't quite
seem worth the trouble given the lack of related field complaints.
Patch by me; thanks to Michael Paquier for review.
Discussion: https://postgr.es/m/
1343113.
1636489231@sss.pgh.pa.us
Tom Lane [Wed, 17 Nov 2021 16:31:31 +0000 (11:31 -0500)]
Fix display of SQL-standard function's arguments in INSERT/SELECT.
If a SQL-standard function body contains an INSERT ... SELECT statement,
any function parameters referenced within the SELECT were always printed
in $N style, rather than using the parameter name if any. While not
strictly incorrect, this wasn't the intention, and it's inconsistent
with the way that such parameters would be printed in any other kind
of statement.
The cause is that the recursion to get_query_def from
get_insert_query_def neglected to pass down the context->namespaces
list, passing constant NIL instead. This is a very ancient oversight,
but AFAICT it had no visible consequences before commit
e717a9a18
added an outermost namespace with function parameters. We don't allow
INSERT ... SELECT as a sub-query, except in a top-level WITH clause,
where it couldn't contain any outer references that might need to access
upper namespaces. So although that's arguably a bug, I don't see any
point in changing it before v14.
In passing, harden the code added to get_parameter by
e717a9a18 so that
it won't crash if a PARAM_EXTERN Param appears in an unexpected place.
Per report from Erki Eessaar. Code fix by me, regression test case
by Masahiko Sawada.
Discussion: https://postgr.es/m/AM9PR01MB8268347BED344848555167FAFE949@AM9PR01MB8268.eurprd01.prod.exchangelabs.com
Daniel Gustafsson [Wed, 17 Nov 2021 13:40:38 +0000 (14:40 +0100)]
Improve publication error messages
Commit
81d5995b4b introduced more fine-grained errormessages for
incorrect relkinds for publication, while unlogged and temporary
tables were reported with using the same message. This provides
separate error messages for these types of relpersistence.
Author: Bharath Rupireddy <
[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Reviewed-by: Jeevan Ladhe <[email protected]>
Reviewed-by: Euler Taveira <[email protected]>
Discussion: https://postgr.es/m/CALj2ACW9S=AswyQHjtO6WMcsergMkCBTtzXGrM8DX26DzfeTLQ@mail.gmail.com
Daniel Gustafsson [Wed, 17 Nov 2021 12:34:41 +0000 (13:34 +0100)]
Doc: add see-also references to CREATE PUBLICATION.
The "See also" section on the reference page for CREATE PUBLICATION
didn't match the cross references on CREATE SUBSCRIPTION and their
ALTER counterparts. Fixed by adding an xref to the CREATE and ALTER
SUBSCRIPTION pages. Backpatch down to v10 where CREATE PUBLICATION
was introduced.
Author: Peter Smith <
[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Discussion: https://postgr.es/m/CAHut+PvGWd3-Ktn96c-z6uq-8TGVVP=TPOkEovkEfntoo2mRhw@mail.gmail.com
Backpatch-through: 10
Peter Eisentraut [Wed, 17 Nov 2021 06:30:30 +0000 (07:30 +0100)]
Fix incorrect format placeholders
Michael Paquier [Wed, 17 Nov 2021 02:04:18 +0000 (11:04 +0900)]
Remove global variable "LastRec" in xlog.c
This variable is used only by StartupXLOG() now, so let's make it local
to simplify the code.
Author: Amul Sul
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAAJ_b96Qd023itERBRN9Z7P2saNDT3CYvGuMO8RXwndVNN6z7g@mail.gmail.com
Alvaro Herrera [Tue, 16 Nov 2021 16:30:37 +0000 (13:30 -0300)]
Fix headerscheck failure in replication/worker_internal.h
Broken by
31c389d8de91
Robert Haas [Tue, 16 Nov 2021 14:43:17 +0000 (09:43 -0500)]
Move InitXLogInsert() call from InitXLOGAccess() to BaseInit().
At present, there is an undocumented coding rule that you must call
RecoveryInProgress(), or do something else that results in a call
to InitXLogInsert(), before trying to write WAL. Otherwise, the
WAL construction buffers won't be initialized, resulting in
failures.
Since it's not good to rely on a status inquiry function like
RecoveryInProgress() having the side effect of initializing
critical data structures, instead do the initialization eariler,
when the backend first starts up.
Patch by me. Reviewed by Nathan Bossart and Michael Paquier.
Discussion: http://postgr.es/m/CA+TgmoY7b65qRjzHN_tWUk8B4sJqk1vj1d31uepVzmgPnZKeLg@mail.gmail.com
Amit Kapila [Tue, 16 Nov 2021 02:40:13 +0000 (08:10 +0530)]
Invalidate relcache when changing REPLICA IDENTITY index.
When changing REPLICA IDENTITY INDEX to another one, the target table's
relcache was not being invalidated. This leads to skipping update/delete
operations during apply on the subscriber side as the columns required to
search corresponding rows won't get logged.
Author: Tang Haiying, Hou Zhijie
Reviewed-by: Euler Taveira, Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/OS0PR01MB61133CA11630DAE45BC6AD95FB939@OS0PR01MB6113.jpnprd01.prod.outlook.com
Robert Haas [Mon, 15 Nov 2021 19:22:13 +0000 (14:22 -0500)]
Fix thinko in bbsink_throttle_manifest_contents.
Report and diagnosis by Dmitry Dolgov.
Discussion: http://postgr.es/m/
20211115162641.dmo6l32fklh64gnw@localhost
Peter Geoghegan [Sat, 13 Nov 2021 03:45:58 +0000 (19:45 -0800)]
Explain pruning pgstats accounting subtleties.
Add a comment explaining why the pgstats accounting used during
opportunistic heap pruning operations (to maintain the current number of
dead tuples in the relation) needs to compensate by subtracting away the
number of new LP_DEAD items. This is needed so it can avoid completely
forgetting about tuples that become LP_DEAD items during pruning -- they
should still count.
It seems more natural to discuss this issue at the only relevant call
site (opportunistic pruning), since the same issue does not apply to the
only other caller (the VACUUM call site). Move everything there too.
Author: Peter Geoghegan <
[email protected]>
Discussion: https://postgr.es/m/CAH2-Wzm7f+A6ej650gi_ifTgbhsadVW5cujAL3punpupHff5Yg@mail.gmail.com
Daniel Gustafsson [Fri, 12 Nov 2021 20:38:10 +0000 (21:38 +0100)]
Document PG_TEST_NOCLEAN in TAP test README
Commit
90627cf98 added support for retaining the data directory even on
successful tests, but failed to document the environment variable which
controls retention. This adds a small note to the TAP test README about
PG_TEST_NOCLEAN which when set skips removing the data directories from
successful tests.
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/
2B02C1B3-3F41-4E14-92B9-
005D83623A0B@yesql.se
Tom Lane [Fri, 12 Nov 2021 19:55:32 +0000 (14:55 -0500)]
Make psql's \password default to CURRENT_USER, not PQuser(conn).
The documentation says plainly that \password acts on "the current user"
by default. What it actually acted on, or tried to, was the username
used to log into the current session. This is not the same thing if
one has since done SET ROLE or SET SESSION AUTHENTICATION. Aside from
the possible surprise factor, it's quite likely that the current role
doesn't have permissions to set the password of the original role.
To fix, use "SELECT CURRENT_USER" to get the role name to act on.
(This syntax works with servers at least back to 7.0.) Also, in
hopes of reducing confusion, include the role name that will be
acted on in the password prompt.
The discrepancy from the documentation makes this a bug, so
back-patch to all supported branches.
Patch by me; thanks to Nathan Bossart for review.
Discussion: https://postgr.es/m/747443.
1635536754@sss.pgh.pa.us
Tom Lane [Fri, 12 Nov 2021 16:50:40 +0000 (11:50 -0500)]
postgres_fdw: suppress casts on constants in limited cases.
When deparsing an expression of the form "remote_var OP constant",
we'd normally apply a cast to the constant to make sure that the
remote parser thinks it's of the same type we do. However, doing
so is often not necessary, and it causes problems if the user has
intentionally declared the local column as being of a different
type than the remote column. A plausible use-case for that is
using text to represent a type that's an enum on the remote side.
A comparison on such a column will get shipped as "var = 'foo'::text",
which blows up on the remote side because there's no enum = text
operator. But if we simply leave off the explicit cast, the
comparison will do exactly what the user wants.
It's possible to do this without major risk of semantic problems, by
relying on the longstanding parser heuristic that "if one operand of
an operator is of type unknown, while the other one has a known type,
assume that the unknown operand is also of that type". Hence, this
patch leaves off the cast only if (a) the operator inputs have the same
type locally; (b) the constant will print as a string literal or NULL,
both of which are initially taken as type unknown; and (c) the non-Const
input is a plain foreign Var. Rule (c) guarantees that the remote
parser will know the type of the non-Const input; moreover, it means
that if this cast-omission does cause any semantic surprises, that can
only happen in cases where the local column has a different type than
the remote column. That wasn't guaranteed to work anyway, and this
patch should represent a net usability gain for such cases.
One point that I (tgl) remain slightly uncomfortable with is that we
will ignore an implicit RelabelType when deciding if the non-Const input
is a plain Var. That makes it a little squishy to argue that the remote
should resolve the Const as being of the same type as its Var, because
then our Const is not the same type as our Var. However, if we don't do
that, then this hack won't work as desired if the user chooses to use
varchar rather than text to represent some remote column. That seems
useful, so do it like this for now. We might have to give up the
RelabelType-ignoring bit if any problems surface.
Dian Fay, with review and kibitzing by me
Discussion: https://postgr.es/m/C9LU294V7K4F.34LRRDU449O45@lamia
Andrew Dunstan [Fri, 12 Nov 2021 15:36:30 +0000 (10:36 -0500)]
Report found versions of required perl modules
Configure tests for the presence of perl modules required for TAP tests,
and that they meet specified minimum version requirements. This patch
makes it report the version of the module that's actually found rather
than just an 'ok' message. This will help in deciding if we can upgrade
minimum requirements for these modules.
Discussion: https://postgr.es/m/
f5e1d308-4e33-37a7-bdf1-
f6e0c75119de@dunslane.net
Michael Paquier [Fri, 12 Nov 2021 12:49:21 +0000 (21:49 +0900)]
Fix memory overrun when querying pg_stat_slru
pg_stat_get_slru() in pgstatfuncs.c would point to one element after the
end of the array PgStat_SLRUStats when finishing to scan its entries.
This had no direct consequences as no data from the extra memory area
was read, but static analyzers would rightfully complain here. So let's
be clean.
While on it, this adds one regression test in the area reserved for
system views.
Reported-by: Alexander Kozhemyakin, via AddressSanitizer
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/17280-
37da556e86032070@postgresql.org
Backpatch-through: 13
Noah Misch [Fri, 12 Nov 2021 01:10:18 +0000 (17:10 -0800)]
Report any XLogReadRecord() error in XlogReadTwoPhaseData().
Buildfarm members kittiwake and tadarida have witnessed errors at this
site. The site discarded key facts. Back-patch to v10 (all supported
versions).
Reviewed by Michael Paquier and Tom Lane.
Discussion: https://postgr.es/m/
20211107013157[email protected]
Peter Geoghegan [Thu, 11 Nov 2021 21:42:17 +0000 (13:42 -0800)]
Update heap_page_prune() free space map comments.
It is up to the heap_page_prune() caller to decide what to do about
updating the FSM for a page following pruning. Update old comments that
address what we might want to do as if it was the responsibility of
heap_page_prune() itself. heap_page_prune() doesn't have enough
high-level context to make a sensible choice.
Peter Geoghegan [Thu, 11 Nov 2021 21:13:08 +0000 (13:13 -0800)]
Update another obsolete reference in vacuumlazy.c.
Addresses an oversight in commit
7ab96cf6.
Robert Haas [Thu, 11 Nov 2021 20:02:53 +0000 (15:02 -0500)]
Improve performance of pgarch_readyXlog() with many status files.
Presently, the archive_status directory was scanned for each file to
archive. When there are many status files, say because archive_command
has been failing for a long time, these directory scans can get very
slow. With this change, the archiver remembers several files to archive
during each directory scan, speeding things up.
To ensure timeline history files are archived as quickly as possible,
XLogArchiveNotify() forces the archiver to do a new directory scan as
soon as the .ready file for one is created.
Nathan Bossart, per a long discussion involving many people. It is
not clear to me exactly who out of all those people reviewed this
particular patch.
Discussion: http://postgr.es/m/CA+TgmobhAbs2yabTuTRkJTq_kkC80-+jw=pfpypdOJ7+gAbQbw@mail.gmail.com
Discussion: http://postgr.es/m/
620F3CE1-0255-4D66-9D87-
0EADE866985A@amazon.com
Tom Lane [Thu, 11 Nov 2021 15:36:39 +0000 (10:36 -0500)]
Fall back to unsigned int, not int, for socklen_t.
It's a coin toss which of these is a better default assumption.
However, of the machines we have in the buildfarm, the only ones
relying on the fallback socklen_t definition are ancient HPUX,
and on that platform unsigned int is the right choice. Minor
tweak to
ee3a1a5b6.
Discussion: https://postgr.es/m/
1440792.
1636558888@sss.pgh.pa.us
Alvaro Herrera [Thu, 11 Nov 2021 14:03:29 +0000 (11:03 -0300)]
Restore lock level to set vacuum flags
Commit
27838981be9d mistakenly reduced the lock level from exclusive to
shared that is acquired to set PGPROC->statusFlags; this was reverted
by
dcfff74fb166, but failed to do so in one spot. Fix it.
Backpatch to 14.
Noted by Andres Freund.
Discussion: https://postgr.es/m/
20211111020724[email protected]
Peter Eisentraut [Thu, 11 Nov 2021 09:49:44 +0000 (10:49 +0100)]
doc: Add referential actions to CREATE/ALTER TABLE synopsis
The general constraint synopsis references "referential_action", but
this was not further defined in the synopsis section. Compared to the
level of detail that the synopsis gives to other subclauses, this
should surely be there.
extracted from a patch by Paul Martinez <
[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CACqFVBZQyMYJV=njbSMxf+rbDHpx=W=B7AEaMKn8dWn9OZJY7w@mail.gmail.com
Michael Paquier [Thu, 11 Nov 2021 06:00:59 +0000 (15:00 +0900)]
Fix buffer overrun in unicode string normalization with empty input
PostgreSQL 13 and newer versions are directly impacted by that through
the SQL function normalize(), which would cause a call of this function
to write one byte past its allocation if using in input an empty
string after recomposing the string with NFC and NFKC. Older versions
(v10~v12) are not directly affected by this problem as the only code
path using normalization is SASLprep in SCRAM authentication that
forbids the case of an empty string, but let's make the code more robust
anyway there so as any out-of-core callers of this function are covered.
The solution chosen to fix this issue is simple, with the addition of a
fast-exit path if the decomposed string is found as empty. This would
only happen for an empty string as at its lowest level a codepoint would
be decomposed as itself if it has no entry in the decomposition table or
if it has a decomposition size of 0.
Some tests are added to cover this issue in v13~. Note that an empty
string has always been considered as normalized (grammar "IS NF[K]{C,D}
NORMALIZED", through the SQL function is_normalized()) for all the
operations allowed (NFC, NFD, NFKC and NFKD) since this feature has been
introduced as of
2991ac5. This behavior is unchanged but some tests are
added in v13~ to check after that.
I have also checked "make normalization-check" in src/common/unicode/,
while on it (works in 13~, and breaks in older stable branches
independently of this commit).
The release notes should just mention this commit for v13~.
Reported-by: Matthijs van der Vleuten
Discussion: https://postgr.es/m/17277-
0c527a373794e802@postgresql.org
Backpatch-through: 10
Michael Paquier [Thu, 11 Nov 2021 01:51:00 +0000 (10:51 +0900)]
Clean up compilation warnings coming from PL/Perl with clang-12~
clang-12 has introduced -Wcompound-token-split-by-macro, that is causing
a large amount of warnings when building PL/Perl because of its
interactions with upstream Perl. This commit adds one -Wno to CFLAGS at
./configure time if the flag is supported by the compiler to silence all
those warnings.
Upstream perl has fixed this issue, but it is going to take some time
before this is spread across the buildfarm, and we have noticed that
some animals would be useful with an extra -Werror to help with the
detection of incorrect placeholders (see
b0cf544), dangomushi being
one.
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/YYr3qYa/
[email protected]
Backpatch-through: 10
Tom Lane [Wed, 10 Nov 2021 18:12:58 +0000 (13:12 -0500)]
Doc: improve protocol spec for logical replication Type messages.
protocol.sgml documented the layout for Type messages, but completely
dropped the ball otherwise, failing to explain what they are, when
they are sent, or what they're good for. While at it, do a little
copy-editing on the description of Relation messages.
In passing, adjust the comment for apply_handle_type() to make it
clearer that we choose not to do anything when receiving a Type
message, not that we think it has no use whatsoever.
Per question from Stefen Hillman.
Discussion: https://postgr.es/m/CAPgW8pMknK5pup6=T4a_UG=Cz80Rgp=KONqJmTdHfaZb0RvnFg@mail.gmail.com
Robert Haas [Wed, 10 Nov 2021 15:12:20 +0000 (10:12 -0500)]
Fix thinko in assertion in basebackup.c.
Commit
5a1007a5088cd6ddf892f7422ea8dbaef362372f tried to introduce
an assertion that the block size was at least twice the size of a
tar block, but I got the math wrong. My error was reported to me
off-list.
Robert Haas [Wed, 10 Nov 2021 14:36:57 +0000 (09:36 -0500)]
More cleanup of 'ThisTimeLineID'.
In XLogCtlData, rename the structure member ThisTimeLineID to
InsertTimeLineID and update the comments to make clear that it's only
expected to be set after recovery is complete.
In StartupXLOG, replace the local variables ThisTimeLineID and
PrevTimeLineID with new local variables replayTLI and newTLI. In the
old scheme, ThisTimeLineID was the replay TLI until we created a new
timeline, and after that the replay TLI was in PrevTimeLineID. Now,
replayTLI is the TLI from which we last replayed WAL throughout the
entire function, and newTLI is either that, or the new timeline created
upon promotion.
Remove some misleading comments from the comment block just above where
recoveryTargetTimeLineGoal and friends are declared. It's become
incorrect, not only because ThisTimeLineID as a variable is now gone,
but also because the rmgr code does not care about ThisTimeLineID and
has not since what used to be the TLI field in the page header was
repurposed to store the page checksum.
Add a comment GetFlushRecPtr that it's only supposed to be used in
normal running, and an assertion to verify that this is so.
Per some ideas from Michael Paquier and some of my own. Review by
Michael Paquier also.
Discussion: http://postgr.es/m/CA+TgmoY1a2d1AnVR3tJcKmGGkhj7GGrwiNwjtKr21dxOuLBzCQ@mail.gmail.com
Peter Eisentraut [Wed, 10 Nov 2021 07:13:12 +0000 (08:13 +0100)]
Fix incorrect format placeholders
Michael Paquier [Wed, 10 Nov 2021 03:00:33 +0000 (12:00 +0900)]
Improve error messages for some callers of XLogReadRecord()
A couple of code paths related to logical decoding (WAL sender, slot
advancing, etc.) use XLogReadRecord(), feeding on error messages
generated by walreader.c on a failure. All those messages have no
context, making it harder to spot from where an error could come even if
these should not happen. All the other callers of XLogReadRecord() do
that already.
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
[email protected]
Jeff Davis [Tue, 9 Nov 2021 18:59:08 +0000 (10:59 -0800)]
Add pg_checkpointer predefined role for CHECKPOINT command.
Any user with the privileges of pg_checkpointer can issue a CHECKPOINT
command.
Reviewed-by: Stephen Frost
Discussion: https://postgr.es/m/
67a1d667e8ec228b5e07f232184c80348c5d93f4.camel%40j-davis.com
Tom Lane [Tue, 9 Nov 2021 23:40:19 +0000 (18:40 -0500)]
Fix instability in 026_overwrite_contrecord.pl test.
We've seen intermittent failures in this test on slower buildfarm
machines, which I think can be explained by assuming that autovacuum
emitted some additional WAL. Disable autovacuum to stabilize it.
In passing, use stringwise not numeric comparison to compare
WAL file names. Doesn't matter at present, but they are
hex strings not decimal ...
Discussion: https://postgr.es/m/
1372189.
1636499287@sss.pgh.pa.us
Robert Haas [Tue, 9 Nov 2021 19:21:35 +0000 (14:21 -0500)]
Have the server properly terminate tar archives.
Earlier versions of PostgreSQL featured a version of pg_basebackup
that wanted to edit tar archives but was too dumb to parse them
properly. The server made things easier for the client by failing
to add the two blocks of zero bytes that ought to end a tar file,
leaving it up to the client to do that.
But since commit
23a1c6578c87fca0e361c4f5f9a07df5ae1f9858, we
don't need this hack any more, because pg_basebackup is now smarter
and can parse tar files even if they are properly terminated! So
change the server to always properly terminate the tar files. Older
versions of pg_basebackup can't talk to new servers anyway, so
there's no compatibility break.
On the pg_basebackup side, we see still need to add the terminating
zero bytes if we're talking to an older server, but not when the
server is v15+. Hopefully at some point we'll be able to remove
some of this compatibility cruft, but it seems best to hang on to
it for now.
In passing, add a file header comment to bbstreamer_tar.c, to make
it clearer what's going on here.
Discussion: http://postgr.es/m/CA+TgmoZbNzsWwM4BE5Jb_qHncY817DYZwGf+2-7hkMQ27ZwsMQ@mail.gmail.com
Peter Eisentraut [Tue, 9 Nov 2021 14:20:47 +0000 (15:20 +0100)]
Remove check for accept() argument types
This check was used to accommodate a staggering variety in particular
in the type of the third argument of accept(). This is no longer of
concern on currently supported systems. We can just use socklen_t in
the code and put in a simple check that substitutes int for socklen_t
if it's missing, to cover the few stragglers.
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://www.postgresql.org/message-id/
3538f4c4-1886-64f2-dcff-
aaad8267fb82@enterprisedb.com
Michael Paquier [Tue, 9 Nov 2021 03:56:34 +0000 (12:56 +0900)]
Make some comments use the term "ProcSignal" for consistency
The surroundings in procsignal.c prefer using "ProcSignal" rather than
"procsignal".
Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACX99ghPmm1M_O4r4g+YsXFjCn=qF7PeDXntLwMpht_Gdg@mail.gmail.com
Fujii Masao [Tue, 9 Nov 2021 03:39:47 +0000 (12:39 +0900)]
doc: Add index entries for pg_stat_statements configuration parameters.
Author: Ken Kato
Reviewed-by: Julien Rouhaud, Fujii Masao
Discussion: https://postgr.es/m/
699cfd8170178db087e54c954b21ece4@oss.nttdata.com
Amit Kapila [Tue, 9 Nov 2021 03:09:33 +0000 (08:39 +0530)]
Rename some enums to use TABLE instead of REL.
Commit
5a2832465f introduced some enums to represent all tables in schema
publications and used REL in their names. Use TABLE instead of REL in
those enums to avoid confusion with other objects like SEQUENCES that can
be part of a publication in the future.
In the passing, (a) Change one of the newly introduced error messages to
make it consistent for Create and Alter commands, (b) add missing alias in
one of the SQL Statements that is used to print publications associated
with the table.
Reported-by: Tomas Vondra, Peter Smith
Author: Vignesh C
Reviewed-by: Hou Zhijie, Peter Smith
Discussion: https://www.postgresql.org/message-id/CALDaNm0OANxuJ6RXqwZsM1MSY4s19nuH3734j4a72etDwvBETQ%40mail.gmail.com
Robert Haas [Mon, 8 Nov 2021 21:36:06 +0000 (16:36 -0500)]
Minimal fix for unterminated tar archive problem.
Commit
23a1c6578c87fca0e361c4f5f9a07df5ae1f9858 improved
pg_basebackup's ability to parse tar archives, but also arranged
to parse them only when we need to make some modification to the
contents of the archive. That's a problem, because the server
doesn't actually terminate tar archives. When the new parsing
logic was engaged, pg_basebackup would properly terminate the
tar file, but when it was skipped, pg_basebackup would just write
whatever it got from the server, meaning that the terminator
was missing.
Most versions of tar are willing to overlook the missing terminator, but
the AIX buildfarm animals were not. Fix by inventing a new kind of
bbstreamer that just blindly adds a terminator, and using it whenever we
don't parse the tar archive.
Discussion: http://postgr.es/m/CA+TgmoZbNzsWwM4BE5Jb_qHncY817DYZwGf+2-7hkMQ27ZwsMQ@mail.gmail.com
Tom Lane [Mon, 8 Nov 2021 19:32:29 +0000 (14:32 -0500)]
Fix incorrect format placeholder.
Per buildfarm warnings.
Tom Lane [Mon, 8 Nov 2021 16:14:56 +0000 (11:14 -0500)]
libpq: reject extraneous data after SSL or GSS encryption handshake.
libpq collects up to a bufferload of data whenever it reads data from
the socket. When SSL or GSS encryption is requested during startup,
any additional data received with the server's yes-or-no reply
remained in the buffer, and would be treated as already-decrypted data
once the encryption handshake completed. Thus, a man-in-the-middle
with the ability to inject data into the TCP connection could stuff
some cleartext data into the start of a supposedly encryption-protected
database session.
This could probably be abused to inject faked responses to the
client's first few queries, although other details of libpq's behavior
make that harder than it sounds. A different line of attack is to
exfiltrate the client's password, or other sensitive data that might
be sent early in the session. That has been shown to be possible with
a server vulnerable to CVE-2021-23214.
To fix, throw a protocol-violation error if the internal buffer
is not empty after the encryption handshake.
Our thanks to Jacob Champion for reporting this problem.
Security: CVE-2021-23222
Tom Lane [Mon, 8 Nov 2021 16:01:43 +0000 (11:01 -0500)]
Reject extraneous data after SSL or GSS encryption handshake.
The server collects up to a bufferload of data whenever it reads data
from the client socket. When SSL or GSS encryption is requested
during startup, any additional data received with the initial
request message remained in the buffer, and would be treated as
already-decrypted data once the encryption handshake completed.
Thus, a man-in-the-middle with the ability to inject data into the
TCP connection could stuff some cleartext data into the start of
a supposedly encryption-protected database session.
This could be abused to send faked SQL commands to the server,
although that would only work if the server did not demand any
authentication data. (However, a server relying on SSL certificate
authentication might well not do so.)
To fix, throw a protocol-violation error if the internal buffer
is not empty after the encryption handshake.
Our thanks to Jacob Champion for reporting this problem.
Security: CVE-2021-23214
David Rowley [Mon, 8 Nov 2021 01:40:33 +0000 (14:40 +1300)]
Fix incorrect hash equality operator bug in Memoize
In v14, because we don't have a field in RestrictInfo to cache both the
left and right type's hash equality operator, we just restrict the scope
of Memoize to only when the left and right types of a RestrictInfo are the
same.
In master we add another field to RestrictInfo and cache both hash
equality operators.
Reported-by: Jaime Casanova
Author: David Rowley
Discussion: https://postgr.es/m/
20210929185544.GB24346%40ahch-to
Backpatch-through: 14
Tomas Vondra [Mon, 8 Nov 2021 00:14:55 +0000 (01:14 +0100)]
Fix gist_bool_ops to use gbtreekey2
Commit
57e3c5160b added a new GiST bool opclass, but it used gbtreekey4
to store the data, which left two bytes undefined, as reported by skink,
our valgrind animal. There was a bit more confusion, because the opclass
also used gbtreekey8 in the definition.
Fix by defining a new gbtreekey2 struct, and using it in all the places.
Discussion: https://postgr.es/m/CAE2gYzyDKJBZngssR84VGZEN=Ux=V9FV23QfPgo+7-yYnKKg4g@mail.gmail.com
Robert Haas [Sun, 7 Nov 2021 20:32:32 +0000 (15:32 -0500)]
Remove tests added by
bd807be6935929bdefe74d1258ca08048f0aafa3.
The buildfarm is unhappy. It's not obvious why it doesn't like
these tests, but let's remove them until we figure it out.
Discussion: http://postgr.es/m/462618.
1636171009@sss.pgh.pa.us
Tom Lane [Sun, 7 Nov 2021 17:18:18 +0000 (12:18 -0500)]
Silence uninitialized-variable warning.
Quite a few buildfarm animals are warning about this, and lapwing
is actually failing (because -Werror). It's a false positive AFAICS,
so no need to do more than zero the variable to start with.
Discussion: https://postgr.es/m/
[email protected]
Tom Lane [Sun, 7 Nov 2021 16:33:53 +0000 (11:33 -0500)]
contrib/sslinfo needs a fix too to make hamerkop happy.
Re-ordering the #include's is a bit problematic here because
libpq/libpq-be.h needs to include <openssl/ssl.h>. Instead,
let's #undef the unwanted macro after all the #includes.
This is definitely uglier than the other way, but it should
work despite possible future header rearrangements.
(A look at the openssl headers indicates that X509_NAME is the
only conflicting symbol that we use.)
In passing, remove a related but long-incorrect comment in
pg_backup_archiver.h.
Discussion: https://postgr.es/m/
1051867.
1635720347@sss.pgh.pa.us
Tom Lane [Sat, 6 Nov 2021 23:12:51 +0000 (19:12 -0400)]
Doc: add some notes about performance of the List functions.
Per suggestion from Andres Freund.
Discussion: https://postgr.es/m/
20211104221248[email protected]
Andres Freund [Sat, 6 Nov 2021 22:43:22 +0000 (15:43 -0700)]
windows: Remove use of WIN32_LEAN_AND_MEAN from crashdump.c.
Since
8162464a25e we do so in win32_port.h. But it likely didn't do much
before that either, because at that point windows.h was already included via
win32_port.h.
Reported-By: Tom Lane
Discussion: https://postgr.es/m/612842.
1636237461@sss.pgh.pa.us
Tom Lane [Sat, 6 Nov 2021 22:02:27 +0000 (18:02 -0400)]
Blind attempt to fix MSVC pgcrypto build.
Commit
db7d1a7b0 pulled out Mkvcbuild.pm's custom support for building
contrib/pgcrypto, but neglected to inform it that that module can now
be built normally. Or at least I guess it can now be built normally.
But this is definitely causing bowerbird to fail, since it's trying to
test a module it hasn't built.
Tom Lane [Sat, 6 Nov 2021 17:28:53 +0000 (13:28 -0400)]
Disallow making an empty lexeme via array_to_tsvector().
The tsvector data type has always forbidden lexemes to be empty.
However, array_to_tsvector() didn't get that memo, and would
allow an empty-string array element to become an empty lexeme.
This could result in dump/restore failures later, not to mention
whatever semantic issues might be behind the original prohibition.
However, other functions that take a plain text input directly as
a lexeme value do not need a similar restriction, because they only
match the string against existing tsvector entries. In particular
it'd be a bad idea to make ts_delete() reject empty strings, since
that is the most convenient way to clean up any bad data that might
have gotten into a tsvector column via this bug.
Reflecting on that, let's also remove the prohibition against NULL
array elements in tsvector_delete_arr and tsvector_setweight_by_filter.
It seems more consistent to ignore them, as an empty-string element
would be ignored.
There's a case for back-patching this, since it's clearly a bug fix.
On balance though, it doesn't seem like something to change in a
minor release.
Jean-Christophe Arnu
Discussion: https://postgr.es/m/CAHZmTm1YVndPgUVRoag2WL0w900XcoiivDDj-gTTYBsG25c65A@mail.gmail.com
Tom Lane [Sat, 6 Nov 2021 16:43:18 +0000 (12:43 -0400)]
Second attempt to silence SSL compile failures on hamerkop.
After further investigation, it seems the cause of the problem
is our recent decision to start defining WIN32_LEAN_AND_MEAN.
That causes <windows.h> to no longer include <wincrypt.h>, which
means that the OpenSSL headers are unable to prevent conflicts
with that header by #undef'ing the conflicting macros. Apparently,
some other system header that be-secure-openssl.c #includes after
the OpenSSL headers is pulling in <wincrypt.h>. It's obscure just
where that happens and why we're not seeing it on other Windows
buildfarm animals. However, it should work to move the OpenSSL
#includes to the end of the list. For the sake of future-proofing,
do likewise in fe-secure-openssl.c. In passing, remove useless
double inclusions of <openssl/ssl.h>.
Thanks to Thomas Munro for running down the relevant information.
Discussion: https://postgr.es/m/
1051867.
1635720347@sss.pgh.pa.us
Alexander Korotkov [Sat, 6 Nov 2021 15:31:21 +0000 (18:31 +0300)]
Reset lastOverflowedXid on standby when needed
Currently, lastOverflowedXid is never reset. It's just adjusted on new
transactions known to be overflowed. But if there are no overflowed
transactions for a long time, snapshots could be mistakenly marked as
suboverflowed due to wraparound.
This commit fixes this issue by resetting lastOverflowedXid when needed
altogether with KnownAssignedXids.
Backpatch to all supported versions.
Reported-by: Stan Hu
Discussion: https://postgr.es/m/CAMBWrQ%3DFp5UAsU_nATY7EMY7NHczG4-DTDU%3DmCvBQZAQ6wa2xQ%40mail.gmail.com
Author: Kyotaro Horiguchi, Alexander Korotkov
Reviewed-by: Stan Hu, Simon Riggs, Nikolay Samokhvalov, Andrey Borodin, Dmitry Dolgov
Tom Lane [Sat, 6 Nov 2021 16:12:32 +0000 (12:12 -0400)]
Un-break pg_basebackup's MSVC build.
Commit
23a1c6578 thought it'd be cute to refactor
pg_basebackup/Makefile with a new variable BBOBJS,
but our MSVC build system knows nothing of that.
Per buildfarm.
Tomas Vondra [Sat, 6 Nov 2021 16:00:43 +0000 (17:00 +0100)]
Add bool GiST opclass to btree_gist
Adds bool opclass to btree_gist extension, to allow creating GiST
indexes on bool columns. GiST indexes on a single bool column don't seem
particularly useful, but this allows defining exclusion constraings
involving a bool column, for example.
Author: Emre Hasegeli
Reviewed-by: Andrey Borodin
Discussion: https://postgr.es/m/CAE2gYzyDKJBZngssR84VGZEN=Ux=V9FV23QfPgo+7-yYnKKg4g@mail.gmail.com
Tomas Vondra [Sat, 6 Nov 2021 15:32:11 +0000 (16:32 +0100)]
Mark mystreamer variable as PG_USED_FOR_ASSERTS_ONLY
Silences warnings about unused variable, when built without asserts.
Peter Geoghegan [Sat, 6 Nov 2021 06:38:07 +0000 (23:38 -0700)]
Update obsolete reference in vacuumlazy.c.
Oversight in commit
7ab96cf6.
Tomas Vondra [Sat, 6 Nov 2021 00:25:31 +0000 (01:25 +0100)]
Fix handling of NaN values in BRIN minmax multi
When calculating distance between float4/float8 values, we need to be a
bit more careful about NaN values in order not to trigger assert. We
consider NaN values to be equal (distace 0.0) and in infinite distance
from all other values.
On builds without asserts, this issue is mostly harmless - the ranges
may be merged in less efficient order, but the index is still correct.
Per report from Andreas Seltenreich. Backpatch to 14, where this new
BRIN opclass was introduced.
Reported-by: Andreas Seltenreich
Discussion: https://postgr.es/m/
[email protected]
Peter Geoghegan [Fri, 5 Nov 2021 21:08:47 +0000 (14:08 -0700)]
Update obsolete heap pruning comments.
Add new comments that spell out what VACUUM expects from heap pruning:
pruning must never leave behind DEAD tuples that still have tuple
storage. This has at least been the case since commit
8523492d, which
established the principle that vacuumlazy.c doesn't have to deal with
DEAD tuples that still have tuple storage directly, except perhaps by
simply retrying pruning (to handle a rare corner case involving
concurrent transaction abort).
In passing, update some references to old symbol names that were missed
by the snapshot scalability work (specifically commit
dc7420c2c9).
Robert Haas [Fri, 5 Nov 2021 16:53:15 +0000 (12:53 -0400)]
Change ThisTimeLineID from a global variable to a local variable.
StartupXLOG() still has ThisTimeLineID as a local variable, but the
remaining code in xlog.c now needs to the relevant TimeLineID by some
other means. Mostly, this means that we now pass it as a function
parameter to a bunch of functions where we didn't previously.
However, a few cases require special handling:
- In functions that might be called by outside callers who
wouldn't necessarily know what timeline to specify, we get
the timeline ID from shared memory. XLogCtl->ThisTimeLineID
can be used in most cases since recovery is known to have
completed by the time those functions are called. In
xlog_redo(), we can use XLogCtl->replayEndTLI.
- XLogFileClose() needs to know the TLI of the open logfile.
Do that with a new global variable openLogTLI. While
someone could argue that this is just trading one global
variable for another, the new one has a far more narrow
purposes and is referenced in just a few places.
- read_backup_label() now returns the TLI that it obtains
by parsing the backup_label file. Previously, ReadRecord()
could be called to parse the checkpoint record without
ThisTimeLineID having been initialized. Now, the timeline
is passed down, and I didn't want to pass an uninitialized
variable; this change lets us avoid that. The old coding
didn't seem to have any practical consequences that we need
to worry about, but this is cleaner.
- In BootstrapXLOG(), it's just a constant.
Patch by me, reviewed and tested by Michael Paquier, Amul Sul, and
Álvaro Herrera.
Discussion: https://postgr.es/m/CA+TgmobfAAqhfWa1kaFBBFvX+5CjM=7TE=n4r4Q1o2bjbGYBpA@mail.gmail.com
Robert Haas [Fri, 5 Nov 2021 16:50:01 +0000 (12:50 -0400)]
Remove all use of ThisTimeLineID global variable outside of xlog.c
All such code deals with this global variable in one of three ways.
Sometimes the same functions use it in more than one of these ways
at the same time.
First, sometimes it's an implicit argument to one or more functions
being called in xlog.c or elsewhere, and must be set to the
appropriate value before calling those functions lest they
misbehave. In those cases, it is now passed as an explicit argument
instead.
Second, sometimes it's used to obtain the current timeline after
the end of recovery, i.e. the timeline to which WAL is being
written and flushed. Such code now calls GetWALInsertionTimeLine()
or relies on the new out parameter added to GetFlushRecPtr().
Third, sometimes it's used during recovery to store the current
replay timeline. That can change, so such code must generally
update the value before each use. It can still do that, but must
now use a local variable instead.
The net effect of these changes is to reduce by a fair amount the
amount of code that is directly accessing this global variable.
That's good, because history has shown that we don't always think
clearly about which timeline ID it's supposed to contain at any
given point in time, or indeed, whether it has been or needs to
be initialized at any given point in the code.
Patch by me, reviewed and tested by Michael Paquier, Amul Sul, and
Álvaro Herrera.
Discussion: https://postgr.es/m/CA+TgmobfAAqhfWa1kaFBBFvX+5CjM=7TE=n4r4Q1o2bjbGYBpA@mail.gmail.com
Robert Haas [Fri, 5 Nov 2021 16:43:04 +0000 (12:43 -0400)]
Don't set ThisTimeLineID when there's no reason to do so.
In slotfuncs.c, pg_replication_slot_advance() needs to determine
the LSN up to which the slot should be advanced, but that doesn't
require us to update ThisTimeLineID, because none of the code called
from here depends on it. If the replication slot is logical,
pg_logical_replication_slot_advance will call read_local_xlog_page,
which does use ThisTimeLineID, but also takes care of making sure
it's up to date. If the replication slot is physical, the timeline
isn't used for anything at all.
In logicalfuncs.c, pg_logical_slot_get_changes_guts() has the same
issue: the only code we're going to run that cares about timelines
is in or downstream of read_local_xlog_page, which already makes
sure that the correct value gets set. Hence, don't do it here.
Patch by me, reviewed and tested by Michael Paquier, Amul Sul, and
Álvaro Herrera.
Discussion: https://postgr.es/m/CA+TgmobfAAqhfWa1kaFBBFvX+5CjM=7TE=n4r4Q1o2bjbGYBpA@mail.gmail.com
Alvaro Herrera [Fri, 5 Nov 2021 15:29:35 +0000 (12:29 -0300)]
Avoid crash in rare case of concurrent DROP
When a role being dropped contains is referenced by catalog objects that
are concurrently also being dropped, a crash can result while trying to
construct the string that describes the objects. Suppress that by
ignoring objects whose descriptions are returned as NULL.
The majority of relevant codesites were already cautious about this
already; we had just missed a couple.
This is an old bug, so backpatch all the way back.
Reported-by: Alexander Lakhin <[email protected]>
Discussion: https://postgr.es/m/17126-
21887f04508cb5c8@postgresql.org
Alvaro Herrera [Fri, 5 Nov 2021 15:09:31 +0000 (12:09 -0300)]
Document that ALTER TABLE .. TYPE removes statistics
Co-authored-by: Nikolai Berkoff <[email protected]>
Discussion: https://postgr.es/m/vCc8XnwDmlP4ZnHBQLIVxzD405BiYHVC9qZlhIF7IsfxK0gC9mZ4PUUOH0-3y6kv5p-87-3_ljqT1KvQVAnb8OoWhPU3kcqWn2ZpmxRBCQg=@pm.me
Alvaro Herrera [Fri, 5 Nov 2021 14:40:03 +0000 (11:40 -0300)]
Pipeline mode disallows multicommand strings
... so mention that in appropriate places of the libpq docs.
Backpatch to 14.
Reported-by: RekGRpth <[email protected]>
Discussion: https://postgr.es/m/17235-
53bb38fc5be593dc@postgresql.org
Alvaro Herrera [Fri, 5 Nov 2021 14:31:57 +0000 (11:31 -0300)]
Document default and changeability of log_startup_progress_interval
Review for
9ce346eabf35.
Author: Álvaro Herrera <
[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://postgr.es/m/
202110292123[email protected]
Robert Haas [Fri, 5 Nov 2021 14:22:07 +0000 (10:22 -0400)]
Introduce 'bbstreamer' abstraction to modularize pg_basebackup.
pg_basebackup knows how to do quite a few things with a backup that it
gets from the server, like just write out the files, or compress them
first, or even parse the tar format and inject a modified
postgresql.auto.conf file into the archive generated by the server.
Unforatunely, this makes pg_basebackup.c a very large source file, and
also somewhat difficult to enhance, because for example the knowledge
that the server is sending us a 'tar' file rather than some other sort
of archive is spread all over the place rather than centralized.
In an effort to improve this situation, this commit invents a new
'bbstreamer' abstraction. Each archive received from the server is
fed to a bbstreamer which may choose to dispose of it or pass it
along to some other bbstreamer. Chunks may also be "labelled"
according to whether they are part of the payload data of a file
in the archive or part of the archive metadata.
So, for example, if we want to take a tar file, modify the
postgresql.auto.conf file it contains, and the gzip the result
and write it out, we can use a bbstreamer_tar_parser to parse the
tar file received from the server, a bbstreamer_recovery_injector
to modify the contents of postgresql.auto.conf, a
bbstreamer_tar_archiver to replace the tar headers for the file
modified in the previous step with newly-built ones that are
correct for the modified file, and a bbstreamer_gzip_writer to
gzip and write the resulting data. Only the objects with "tar"
in the name know anything about the tar archive format, and in
theory we could re-archive using some other format rather than
"tar" if somebody wanted to write the code.
These chances do add a substantial amount of code, but I think the
result is a lot more maintainable and extensible. pg_basebackup.c
itself shrinks by roughly a third, with a lot of the complexity
previously contained there moving into the newly-added files.
Patch by me. The larger patch series of which this is a part has been
reviewed and tested at various times by Andres Freund, Sumanta
Mukherjee, Dilip Kumar, Suraj Kharage, Dipesh Pandit, Tushar Ahuja,
Mark Dilger, Sergei Kornilov, and Jeevan Ladhe.
Discussion: https://postgr.es/m/CA+TgmoZGwR=ZVWFeecncubEyPdwghnvfkkdBe9BLccLSiqdf9Q@mail.gmail.com
Discussion: https://postgr.es/m/CA+TgmoZvqk7UuzxsX1xjJRmMGkqoUGYTZLDCH8SmU1xTPr1Xig@mail.gmail.com
Alvaro Herrera [Fri, 5 Nov 2021 14:22:30 +0000 (11:22 -0300)]
Reword doc blurb for vacuumdb --analyze-in-stages
Make users aware that using it in a database with existing stats might
cause transient problems.
Author: Nikolai Berkoff <
[email protected]>
Discussion: https://postgr.es/m/s-kSljtWXMWgMfGTztPTPcS80R8FHdOrBxDTnrQI6GMZbT7au1A4b0fzaSFtKwCI8nwN0MhgPLfVOTvJ7DwTjkip4P3d0o4VgrMJs4OLN-o=@pm.me
Robert Haas [Fri, 5 Nov 2021 14:08:30 +0000 (10:08 -0400)]
Introduce 'bbsink' abstraction to modularize base backup code.
The base backup code has accumulated a healthy number of new
features over the years, but it's becoming increasingly difficult
to maintain and further enhance that code because there's no
real separation of concerns. For example, the code that
understands knows the details of how we send data to the client
using the libpq protocol is scattered throughout basebackup.c,
rather than being centralized in one place.
To try to improve this situation, introduce a new 'bbsink' object
which acts as a recipient for archives generated during the base
backup progress and also for the backup manifest. This commit
introduces three types of bbsink: a 'copytblspc' bbsink forwards the
backup to the client using one COPY OUT operation per tablespace and
another for the manifest, a 'progress' bbsink performs command
progress reporting, and a 'throttle' bbsink performs rate-limiting.
The 'progress' and 'throttle' bbsink types also forward the data to a
successor bbsink; at present, the last bbsink in the chain will
always be of type 'copytblspc'. There are plans to add more types
of 'bbsink' in future commits.
This abstraction is a bit leaky in the case of progress reporting,
but this still seems cleaner than what we had before.
Patch by me, reviewed and tested by Andres Freund, Sumanta Mukherjee,
Dilip Kumar, Suraj Kharage, Dipesh Pandit, Tushar Ahuja, Mark Dilger,
and Jeevan Ladhe.
Discussion: https://postgr.es/m/CA+TgmoZGwR=ZVWFeecncubEyPdwghnvfkkdBe9BLccLSiqdf9Q@mail.gmail.com
Discussion: https://postgr.es/m/CA+TgmoZvqk7UuzxsX1xjJRmMGkqoUGYTZLDCH8SmU1xTPr1Xig@mail.gmail.com
Robert Haas [Fri, 5 Nov 2021 13:17:40 +0000 (09:17 -0400)]
amcheck: Add additional TOAST pointer checks.
Expand the checks of toasted attributes to complain if the rawsize is
overlarge. For compressed attributes, also complain if compression
appears to have expanded the attribute or if the compression method is
invalid.
Mark Dilger, reviewed by Justin Pryzby, Alexander Alekseev, Heikki
Linnakangas, Greg Stark, and me.
Discussion: http://postgr.es/m/
8E42250D-586A-4A27-B317-
8B062C3816A8@enterprisedb.com
Peter Eisentraut [Fri, 5 Nov 2021 12:59:42 +0000 (13:59 +0100)]
pgcrypto: Remove non-OpenSSL support
pgcrypto had internal implementations of some encryption algorithms,
as an alternative to calling out to OpenSSL. These were rarely used,
since most production installations are built with OpenSSL. Moreover,
maintaining parallel code paths makes the code more complex and
difficult to maintain.
This patch removes these internal implementations. Now, pgcrypto is
only built if OpenSSL support is configured.
Reviewed-by: Daniel Gustafsson <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
0b42f1df-8cba-6a30-77d7-
acc241cc88c1%40enterprisedb.com
Michael Paquier [Fri, 5 Nov 2021 06:25:36 +0000 (15:25 +0900)]
Improve psql tab completion for COMMENT
Completion is added for more object types, like domain constraints, text
search-ish objects or policies. Moreover, the area is reorganized,
changing the list of objects supported by COMMENT to be in the same
order as the documentation to ease future additions.
Author: Ken Kato
Reviewed-by: Fujii Masao, Shinya Kato, Suraj Khamkar, Michael Paquier
Discussion: https://postgr.es/m/
6e0c2f3f657b229bea32d098d118f307@oss.nttdata.com
Peter Geoghegan [Fri, 5 Nov 2021 02:54:05 +0000 (19:54 -0700)]
Add hardening to catch invalid TIDs in indexes.
Add hardening to the heapam index tuple deletion path to catch TIDs in
index pages that point to a heap item that index tuples should never
point to. The corruption we're trying to catch here is particularly
tricky to detect, since it typically involves "extra" (corrupt) index
tuples, as opposed to the absence of required index tuples in the index.
For example, a heap TID from an index page that turns out to point to an
LP_UNUSED item in the heap page has a good chance of being caught by one
of the new checks. There is a decent chance that the recently fixed
parallel VACUUM bug (see commit
9bacec15) would have been caught had
that particular check been in place for Postgres 14. No backpatch of
this extra hardening for now, though.
Author: Peter Geoghegan <
[email protected]>
Reviewed-By: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CAH2-Wzk-4_raTzawWGaiqNvkpwDXxv3y1AQhQyUeHfkU=tFCeA@mail.gmail.com
Michael Paquier [Fri, 5 Nov 2021 02:33:25 +0000 (11:33 +0900)]
Add support for LZ4 compression in pg_receivewal
pg_receivewal gains a new option, --compression-method=lz4, available
when the code is compiled with --with-lz4. Similarly to gzip, this
gives the possibility to compress archived WAL segments with LZ4. This
option is not compatible with --compress.
The implementation uses LZ4 frames, and is compatible with simple lz4
commands. Like gzip, using --synchronous ensures that any data will be
flushed to disk within the current .partial segment, so as it is
possible to retrieve as much WAL data as possible even from a
non-completed segment (this requires completing the partial file with
zeros up to the WAL segment size supported by the backend after
decompression, but this is the same as gzip).
The calculation of the streaming start LSN is able to transparently find
and check LZ4-compressed segments. Contrary to gzip where the
uncompressed size is directly stored in the object read, the LZ4 chunk
protocol does not store the uncompressed data by default. There is
contentSize that can be used with LZ4 frames by that would not help if
using an archive that includes segments compressed with the defaults of
a "lz4" command, where this is not stored. So, this commit has taken
the most extensible approach by decompressing the already-archived
segment to check its uncompressed size, through a blank output buffer in
chunks of 64kB (no actual performance difference noticed with 8kB, 16kB
or 32kB, and the operation in itself is actually fast).
Tests have been added to verify the creation and correctness of the
generated LZ4 files. The latter is achieved by the use of command
"lz4", if found in the environment.
The tar-based WAL method in walmethods.c, used now only by
pg_basebackup, does not know yet about LZ4. Its code could be extended
for this purpose.
Author: Georgios Kokolatos
Reviewed-by: Michael Paquier, Jian Guo, Magnus Hagander, Dilip Kumar
Discussion: https://postgr.es/m/ZCm1J5vfyQ2E6dYvXz8si39HQ2gwxSZ3IpYaVgYa3lUwY88SLapx9EEnOf5uEwrddhx2twG7zYKjVeuP5MwZXCNPybtsGouDsAD1o2L_I5E=@pm.me
Peter Geoghegan [Fri, 5 Nov 2021 02:07:54 +0000 (19:07 -0700)]
Add various assertions to heap pruning code.
These assertions document (and verify) our high level assumptions about
how pruning can and cannot affect existing items from target heap pages.
For example, one of the new assertions verifies that pruning does not
set a heap-only tuple to LP_DEAD.
Author: Peter Geoghegan <
[email protected]>
Reviewed-By: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CAH2-Wz=vhvBx1GjF+oueHh8YQcHoQYrMi0F0zFMHEr8yc4sCoA@mail.gmail.com
Michael Paquier [Thu, 4 Nov 2021 03:32:37 +0000 (12:32 +0900)]
Fix some thinkos with pg_receivewal --compression-method
The option name was incorrect in one of the error messages, and the
short option 'I' was used in the code but we did not intend things to be
this way. While on it, fix the documentation to refer to a "method",
and not a "level.
Oversights in commit
d62bcc8, that I have detected after more review of
the LZ4 patch for pg_receivewal.
Michael Paquier [Thu, 4 Nov 2021 02:10:31 +0000 (11:10 +0900)]
Rework compression options of pg_receivewal
pg_receivewal includes since
cada1af the option --compress, to allow the
compression of WAL segments using gzip, with a value of 0 (the default)
meaning that no compression can be used.
This commit introduces a new option, called --compression-method, able
to use as values "none", the default, and "gzip", to make things more
extensible. The case of --compress=0 becomes fuzzy with this option
layer, so we have made the choice to make pg_receivewal return an error
when using "none" and a non-zero compression level, meaning that the
authorized values of --compress are now [1,9] instead of [0,9]. Not
specifying --compress with "gzip" as compression method makes
pg_receivewal use the default of zlib instead (Z_DEFAULT_COMPRESSION).
The code in charge of finding the streaming start LSN when scanning the
existing archives is refactored and made more extensible. While on it,
rename "compression" to "compression_level" in walmethods.c, to reduce
the confusion with the introduction of the compression method, even if
the tar method used by pg_basebackup does not rely on the compression
method (yet, at least), but just on the compression level (this area
could be improved more, actually).
This is in preparation for an upcoming patch that adds LZ4 support to
pg_receivewal.
Author: Georgios Kokolatos
Reviewed-by: Michael Paquier, Jian Guo, Magnus Hagander, Dilip Kumar,
Robert Haas
Discussion: https://postgr.es/m/ZCm1J5vfyQ2E6dYvXz8si39HQ2gwxSZ3IpYaVgYa3lUwY88SLapx9EEnOf5uEwrddhx2twG7zYKjVeuP5MwZXCNPybtsGouDsAD1o2L_I5E=@pm.me
Peter Geoghegan [Thu, 4 Nov 2021 00:34:19 +0000 (17:34 -0700)]
Add another old commit to git-blame-ignore-revs.
Add another historic pgindent commit that was missed by the initial work
done in commit
8e638845.
Heikki Linnakangas [Wed, 3 Nov 2021 17:38:17 +0000 (19:38 +0200)]
Update alternative expected output file.
Previous commit added a test to 'largeobject', but neglected the
alternative expected output file 'largeobject_1.source'. Per failure
on buildfarm animal 'hamerkop'.
Discussion: https://www.postgresql.org/message-id/
DBA08346-9962-4706-92D1-
230EE5201C10@yesql.se
Heikki Linnakangas [Wed, 3 Nov 2021 08:28:52 +0000 (10:28 +0200)]
Fix snapshot reference leak if lo_export fails.
If lo_export() fails to open the target file or to write to it, it leaks
the created LargeObjectDesc and its snapshot in the top-transaction
context and resource owner. That's pretty harmless, it's a small leak
after all, but it gives the user a "Snapshot reference leak" warning.
Fix by using a short-lived memory context and no resource owner for
transient LargeObjectDescs that are opened and closed within one function
call. The leak is easiest to reproduce with lo_export() on a directory
that doesn't exist, but in principle the other lo_* functions could also
fail.
Backpatch to all supported versions.
Reported-by: Andrew B
Reviewed-by: Alvaro Herrera
Discussion: https://www.postgresql.org/message-id/
32bf767a-2d65-71c4-f170-
122f416bab7e@iki.fi
Peter Eisentraut [Wed, 3 Nov 2021 06:34:28 +0000 (07:34 +0100)]
Fix incorrect format placeholder
Peter Geoghegan [Wed, 3 Nov 2021 02:52:11 +0000 (19:52 -0700)]
Fix parallel amvacuumcleanup safety bug.
Commit
b4af70cb inverted the return value of the function
parallel_processing_is_safe(), but missed the amvacuumcleanup test.
Index AMs that don't support parallel cleanup at all were affected.
The practical consequences of this bug were not very serious. Hash
indexes are affected, but since they just return the number of blocks
during hashvacuumcleanup anyway, it can't have had much impact.
Author: Masahiko Sawada <
[email protected]>
Discussion: https://postgr.es/m/CAD21AoA-Em+aeVPmBbL_s1V-ghsJQSxYL-i3JP8nTfPiD1wjKw@mail.gmail.com
Backpatch: 14-, where commit
b4af70cb appears.
Tom Lane [Tue, 2 Nov 2021 19:18:00 +0000 (15:18 -0400)]
Blind attempt to silence SSL compile failures on hamerkop.
Buildfarm member hamerkop has been failing for the last few days
with errors that look like OpenSSL's X509-related symbols have
not been imported into be-secure-openssl.c. It's unclear why
this should be, but let's try adding an explicit #include of
<openssl/x509v3.h>, as there has long been in fe-secure-openssl.c.
Discussion: https://postgr.es/m/
1051867.
1635720347@sss.pgh.pa.us
Peter Geoghegan [Tue, 2 Nov 2021 19:06:17 +0000 (12:06 -0700)]
Don't overlook indexes during parallel VACUUM.
Commit
b4af70cb, which simplified state managed by VACUUM, performed
refactoring of parallel VACUUM in passing. Confusion about the exact
details of the tasks that the leader process is responsible for led to
code that made it possible for parallel VACUUM to miss a subset of the
table's indexes entirely. Specifically, indexes that fell under the
min_parallel_index_scan_size size cutoff were missed. These indexes are
supposed to be vacuumed by the leader (alongside any parallel unsafe
indexes), but weren't vacuumed at all. Affected indexes could easily
end up with duplicate heap TIDs, once heap TIDs were recycled for new
heap tuples. This had generic symptoms that might be seen with almost
any index corruption involving structural inconsistencies between an
index and its table.
To fix, make sure that the parallel VACUUM leader process performs any
required index vacuuming for indexes that happen to be below the size
cutoff. Also document the design of parallel VACUUM with these
below-size-cutoff indexes.
It's unclear how many users might be affected by this bug. There had to
be at least three indexes on the table to hit the bug: a smaller index,
plus at least two additional indexes that themselves exceed the size
cutoff. Cases with just one additional index would not run into
trouble, since the parallel VACUUM cost model requires two
larger-than-cutoff indexes on the table to apply any parallel
processing. Note also that autovacuum was not affected, since it never
uses parallel processing.
Test case based on tests from a larger patch to test parallel VACUUM by
Masahiko Sawada.
Many thanks to Kamigishi Rei for her invaluable help with tracking this
problem down.
Author: Peter Geoghegan <
[email protected]>
Author: Masahiko Sawada <
[email protected]>
Reported-By: Kamigishi Rei <[email protected]>
Reported-By: Andrew Gierth <[email protected]>
Diagnosed-By: Andres Freund <[email protected]>
Bug: #17245
Discussion: https://postgr.es/m/17245-
ddf06aaf85735f36@postgresql.org
Discussion: https://postgr.es/m/
20211030023740[email protected]
Backpatch: 14-, where the refactoring commit appears.
Tom Lane [Tue, 2 Nov 2021 18:28:50 +0000 (14:28 -0400)]
Ensure consistent logical replication of datetime and float8 values.
In walreceiver, set the publisher's relevant GUCs (datestyle,
intervalstyle, extra_float_digits) to the same values that pg_dump uses,
and for the same reason: we need the output to be read the same way
regardless of the receiver's settings. Without this, it's possible
for subscribers to misinterpret transmitted values.
Although this is clearly a bug fix, it's not without downsides:
subscribers that are storing values into some other datatype, such as
text, could get different results than before, and perhaps be unhappy
about that. Given the lack of previous complaints, it seems best
to change this only in HEAD, and to call it out as an incompatible
change in v15.
Japin Li, per report from Sadhuprasad Patro
Discussion: https://postgr.es/m/CAFF0-CF=D7pc6st-3A9f1JnOt0qmc+BcBPVzD6fLYisKyAjkGA@mail.gmail.com
Tom Lane [Tue, 2 Nov 2021 17:36:47 +0000 (13:36 -0400)]
Fix variable lifespan in ExecInitCoerceToDomain().
This undoes a mistake in
1ec7679f1: domainval and domainnull were
meant to live across loop iterations, but they were incorrectly
moved inside the loop. The effect was only to emit useless extra
EEOP_MAKE_READONLY steps, so it's not a big deal; nonetheless,
back-patch to v13 where the mistake was introduced.
Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAqXuhbkaAp-sGH6dR6Nsq7v28_0TPexHOm6FiDYqwQD-w@mail.gmail.com
Tom Lane [Tue, 2 Nov 2021 16:54:35 +0000 (12:54 -0400)]
Doc: clean up some places that mentioned template1 but not template0.
Improve old text that wasn't updated when we added template0 to
the standard database set.
Per suggestion from P. Luzanov.
Discussion: https://postgr.es/m/
163583775122.675.
3700595100340939507@wrigleys.postgresql.org
Tom Lane [Tue, 2 Nov 2021 16:12:02 +0000 (12:12 -0400)]
Doc: be more precise about conflicts between relation names.
Use verbiage like "The name of the table must be distinct from the name
of any other relation (table, sequence, index, view, materialized view,
or foreign table) in the same schema." in the reference pages for all
those object types. The main change here is to mention materialized
views explicitly; although a couple of these pages failed to say
anything at all about name conflicts.
Per suggestion from Daniel Westermann.
Discussion: https://postgr.es/m/ZR0P278MB0920D0946509233459AF0DEFD2889@ZR0P278MB0920.CHEP278.PROD.OUTLOOK.COM
Tom Lane [Tue, 2 Nov 2021 15:31:54 +0000 (11:31 -0400)]
Avoid O(N^2) behavior in SyncPostCheckpoint().
As in commits
6301c3ada and
e9d9ba2a4, avoid doing repetitive
list_delete_first() operations, since that would be expensive when
there are many files waiting to be unlinked. This is a slightly
larger change than in those cases. We have to keep the list state
valid for calls to AbsorbSyncRequests(), so it's necessary to invent a
"canceled" field instead of immediately deleting PendingUnlinkEntry
entries. Also, because we might not be able to process all the
entries, we need a new list primitive list_delete_first_n().
list_delete_first_n() is almost list_copy_tail(), but it modifies the
input List instead of making a new copy. I found a couple of existing
uses of the latter that could profitably use the new function. (There
might be more, but the other callers look like they probably shouldn't
overwrite the input List.)
As before, back-patch to v13.
Discussion: https://postgr.es/m/
CD2F0E7F-9822-45EC-A411-
AE56F14DEA9F@amazon.com