Stamp 15.11.
commit : e3cbae2221c8107fa816673adaea4e357c1393df
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 10 Feb 2025 18:24:55 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 10 Feb 2025 18:24:55 -0500
M configure
M configure.ac
Last-minute updates for release notes.
commit : 1c77ff9f9a0278bb23b3629417ed7ce097c1697a
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 10 Feb 2025 18:16:25 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 10 Feb 2025 18:16:25 -0500
Security: CVE-2025-1094
M doc/src/sgml/release-15.sgml
Adapt appendPsqlMetaConnect() to the new fmtId() encoding expectations.
commit : 9862de9176ef161e20a40f327afe18c6a11ecdcc
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 10 Feb 2025 16:30:03 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 10 Feb 2025 16:30:03 -0500
We need to tell fmtId() what encoding to assume, but this function
doesn't know that. Fortunately we can fix that without changing the
function's API, because we can just use SQL_ASCII. That's because
database names in connection requests are effectively binary not text:
no encoding-aware processing will happen on them.
This fixes XversionUpgrade failures seen in the buildfarm. The
alternative of having pg_upgrade use setFmtEncoding() is unappetizing,
given that it's connecting to multiple databases that may have
different encodings.
Andres Freund, Noah Misch, Tom Lane
Security: CVE-2025-1094
M src/fe_utils/string_utils.c
Fix type in test_escape test
commit : a085fa73169d6c0df079cfa4ddfeade812368a64
author : Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 12:09:23 -0500
committer: Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 12:09:23 -0500
On machines where char is unsigned this could lead to option parsing looping
endlessly. It's also too narrow a type on other hardware.
Found via Tom Lane's monitoring of the buildfarm.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Security: CVE-2025-1094
Backpatch-through: 13
M src/test/modules/test_escape/test_escape.c
docs: EUC_TW can be up to four bytes wide, not three
commit : 57bda2508f1c666fd18f2b233823b7c98308226b
author : Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 10:03:39 -0500
committer: Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 10:03:39 -0500
Backpatch-through: 13
Security: CVE-2025-1094
M doc/src/sgml/charset.sgml
Add test of various escape functions
commit : de4b92f3332b7ea3373ae7bad0d790473b3665e6
author : Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 10:03:39 -0500
committer: Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 10:03:39 -0500
As highlighted by the prior commit, writing correct escape functions is less
trivial than one might hope.
This test module tries to verify that different escaping functions behave
reasonably. It e.g. tests:
- Invalidly encoded input to an escape function leads to invalidly encoded
output
- Trailing incomplete multi-byte characters are handled sensibly
- Escaped strings are parsed as single statement by psql's parser (which
derives from the backend parser)
There are further tests that would be good to add. But even in the current
state it was rather useful for writing the fix in the prior commit.
Reviewed-by: Noah Misch <noah@leadboat.com>
Backpatch-through: 13
Security: CVE-2025-1094
M src/test/modules/Makefile
A src/test/modules/test_escape/.gitignore
A src/test/modules/test_escape/Makefile
A src/test/modules/test_escape/t/001_test_escape.pl
A src/test/modules/test_escape/test_escape.c
M src/tools/msvc/Mkvcbuild.pm
M src/tools/pgindent/typedefs.list
Fix handling of invalidly encoded data in escaping functions
commit : 370c94d4cce179736f9a255779eb8e653e1edfb2
author : Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 10:03:39 -0500
committer: Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 10:03:39 -0500
Previously invalidly encoded input to various escaping functions could lead to
the escaped string getting incorrectly parsed by psql. To be safe, escaping
functions need to ensure that neither invalid nor incomplete multi-byte
characters can be used to "escape" from being quoted.
Functions which can report errors now return an error in more cases than
before. Functions that cannot report errors now replace invalid input bytes
with a byte sequence that cannot be used to escape the quotes and that is
guaranteed to error out when a query is sent to the server.
The following functions are fixed by this commit:
- PQescapeLiteral()
- PQescapeIdentifier()
- PQescapeString()
- PQescapeStringConn()
- fmtId()
- appendStringLiteral()
Reported-by: Stephen Fewer <stephen_fewer@rapid7.com>
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Backpatch-through: 13
Security: CVE-2025-1094
M src/fe_utils/string_utils.c
M src/interfaces/libpq/fe-exec.c
Specify the encoding of input to fmtId()
commit : b1756da7541323eae07fe23eaad17ce12f20a56b
author : Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 10:03:39 -0500
committer: Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 10:03:39 -0500
This commit adds fmtIdEnc() and fmtQualifiedIdEnc(), which allow to specify
the encoding as an explicit argument. Additionally setFmtEncoding() is
provided, which defines the encoding when no explicit encoding is provided, to
avoid breaking all code using fmtId().
All users of fmtId()/fmtQualifiedId() are either converted to the explicit
version or a call to setFmtEncoding() has been added.
This commit does not yet utilize the now well-defined encoding, that will
happen in a subsequent commit.
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Backpatch-through: 13
Security: CVE-2025-1094
M src/bin/pg_dump/pg_backup_archiver.c
M src/bin/pg_dump/pg_dump.c
M src/bin/pg_dump/pg_dumpall.c
M src/bin/psql/command.c
M src/bin/scripts/common.c
M src/bin/scripts/createdb.c
M src/bin/scripts/createuser.c
M src/bin/scripts/dropdb.c
M src/bin/scripts/dropuser.c
M src/bin/scripts/reindexdb.c
M src/bin/scripts/vacuumdb.c
M src/fe_utils/string_utils.c
M src/include/fe_utils/string_utils.h
Add pg_encoding_set_invalid()
commit : 703b3fd5de5d7bfe9cb3948601bc165da024256d
author : Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 10:03:39 -0500
committer: Andres Freund <andres@anarazel.de>
date : Mon, 10 Feb 2025 10:03:39 -0500
There are cases where we cannot / do not want to error out for invalidly
encoded input. In such cases it can be useful to replace e.g. an incomplete
multi-byte characters with bytes that will trigger an error when getting
validated as part of a larger string.
Unfortunately, until now, for some encoding no such sequence existed. For
those encodings this commit removes one previously accepted input combination
- we consider that to be ok, as the chosen bytes are outside of the valid
ranges for the encodings, we just previously failed to detect that.
As we cannot add a new field to pg_wchar_table without breaking ABI, this is
implemented "in-line" in the newly added function.
Author: Noah Misch <noah@leadboat.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Backpatch-through: 13
Security: CVE-2025-1094
M src/common/wchar.c
M src/include/mb/pg_wchar.h
M src/test/regress/expected/conversion.out
M src/test/regress/regress.c
M src/test/regress/sql/conversion.sql
Translation updates
commit : 0fb459877a429f7e934bff4acd6c853b2e47515c
author : Peter Eisentraut <peter@eisentraut.org>
date : Mon, 10 Feb 2025 15:12:58 +0100
committer: Peter Eisentraut <peter@eisentraut.org>
date : Mon, 10 Feb 2025 15:12:58 +0100
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: b33709791c59e6c0345a13066663f79a25be7931
M src/backend/po/de.po
M src/backend/po/es.po
M src/backend/po/fr.po
M src/backend/po/ja.po
M src/backend/po/ru.po
M src/bin/pg_basebackup/po/fr.po
M src/bin/pg_basebackup/po/ja.po
M src/bin/pg_controldata/po/ru.po
M src/bin/pg_ctl/po/ru.po
M src/bin/pg_dump/po/ru.po
M src/bin/pg_rewind/po/ru.po
M src/bin/psql/po/ru.po
M src/bin/scripts/po/ru.po
M src/interfaces/ecpg/preproc/po/ru.po
M src/interfaces/libpq/po/de.po
M src/interfaces/libpq/po/es.po
M src/interfaces/libpq/po/ru.po
M src/pl/plpython/po/ru.po
M src/pl/tcl/po/ru.po
Release notes for 17.3, 16.7, 15.11, 14.16, 13.19.
commit : a71b26bd0e900ef0d6068c94583b35d98e7a2185
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Sun, 9 Feb 2025 13:58:53 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Sun, 9 Feb 2025 13:58:53 -0500
M doc/src/sgml/release-15.sgml
Fix pgbench performance issue induced by commit af35fe501.
commit : 499d1cf55d3d07b29bc80b10f8abca4fb0458154
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Fri, 7 Feb 2025 13:41:43 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Fri, 7 Feb 2025 13:41:43 -0500
Commit af35fe501 caused "pgbench -i" to emit a '\r' character
for each data row loaded (when stderr is a terminal).
That's effectively invisible on-screen, but it causes the
connected terminal program to consume a lot of cycles.
It's even worse if you're connected over ssh, as the data
then has to pass through the ssh tunnel.
Simplest fix is to move the added logic inside the if-tests
that check whether to print a progress line. We could do
it another way that avoids duplicating these few lines,
but on the whole this seems the most transparent way to
write it.
Like the previous commit, back-patch to all supported versions.
Reported-by: Andres Freund <andres@anarazel.de>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/4k4drkh7bcmdezq6zbkhp25mnrzpswqi2o75d5uv2eeg3aq6q7@b7kqdmzzwzgb
Backpatch-through: 13
M src/bin/pgbench/pgbench.c
Doc: clarify behavior of timestamptz input some more.
commit : fd467ceceb0374292bd7384319179c516aecf9b1
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Fri, 7 Feb 2025 12:40:41 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Fri, 7 Feb 2025 12:40:41 -0500
Try to make it absolutely plain that we don't retain the
originally specified time zone, only the UTC timestamp.
While at it, make glossary entries for "UTC" and "GMT".
Author: Robert Treat <rob@xzilla.net>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/173796426022.1064.9135167366862649513@wrigleys.postgresql.org
Backpatch-through: 13
M doc/src/sgml/datatype.sgml
M doc/src/sgml/glossary.sgml
doc: Update links which returned 404
commit : 5a874c671f373ecca58139d16d8f453d4d6ad222
author : Daniel Gustafsson <dgustafsson@postgresql.org>
date : Wed, 5 Feb 2025 13:58:40 +0100
committer: Daniel Gustafsson <dgustafsson@postgresql.org>
date : Wed, 5 Feb 2025 13:58:40 +0100
Two links in the isn module documentation were pointing to tools
which had been moved, resulting in 404 error responses. Update
to the new URLs for the tools. The link to the Sequoia 2000 page
in the history section was no longer working, and since the page
is no longer available online update our link to point at the
paper instead which is on a stable URL.
These links exist in all versions of the documentation so backpatch
to all supported branches.
Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: charukiewicz@protonmail.com
Discussion: https://postgr.es/m/173679670185.705.8565555804465055355@wrigleys.postgresql.org
Backpatch-through: 13
M doc/src/sgml/biblio.sgml
M doc/src/sgml/history.sgml
M doc/src/sgml/isn.sgml
pg_controldata: Fix possible errors on corrupted pg_control
commit : f1e0b078b2e3613d28dc10090d35cc3908bbbd09
author : Alexander Korotkov <akorotkov@postgresql.org>
date : Wed, 5 Feb 2025 00:15:17 +0200
committer: Alexander Korotkov <akorotkov@postgresql.org>
date : Wed, 5 Feb 2025 00:15:17 +0200
Protect against malformed timestamps. Also protect against negative WalSegSz
as it triggers division by zero:
((0x100000000UL) / (WalSegSz)) can turn into zero in
XLogFileName(xlogfilename, ControlFile->checkPointCopy.ThisTimeLineID,
segno, WalSegSz);
because if WalSegSz is -1 then by arithmetic rules in C we get
0x100000000UL / 0xFFFFFFFFFFFFFFFFUL == 0.
Author: Ilyasov Ian <ianilyasov@outlook.com>
Author: Anton Voloshin <a.voloshin@postgrespro.ru>
Backpatch-through: 13
M src/bin/pg_controldata/pg_controldata.c
vacuumdb: Add missing PQfinish() calls to vacuum_one_database().
commit : 8c2dd212d1bcefa8d2cbcbe53398de367c2d5f63
author : Nathan Bossart <nathan@postgresql.org>
date : Tue, 4 Feb 2025 13:26:57 -0600
committer: Nathan Bossart <nathan@postgresql.org>
date : Tue, 4 Feb 2025 13:26:57 -0600
A few of the version checks in vacuum_one_database() do not call
PQfinish() before exiting. This precedent was unintentionally
established in commit 00d1e88d36, and while it's probably not too
problematic, it seems better to properly close the connection.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/Z6JAwqN1I8ljTuXp%40nathan
Backpatch-through: 13
M src/bin/scripts/vacuumdb.c
Mention jsonlog in description of logging_collector in GUC table
commit : 061994fb31480e65c30f8747e8dcc55935174f4a
author : Michael Paquier <michael@paquier.xyz>
date : Sun, 2 Feb 2025 11:31:31 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Sun, 2 Feb 2025 11:31:31 +0900
logging_collector was only mentioning stderr and csvlog, and forgot
about jsonlog. Oversight in dc686681e079, that has added support for
jsonlog in log_destination.
While on it, the description in the GUC table is tweaked to be more
consistent with the documentation and postgresql.conf.sample.
Author: Umar Hayat
Reviewed-by: Ashutosh Bapat, Tom Lane
Discussion: https://postgr.es/m/CAD68Dp1K_vBYqBEukHw=1jF7e76t8aszGZTFL2ugi=H7r=a7MA@mail.gmail.com
Backpatch-through: 13
M src/backend/utils/misc/guc.c
Fix comment of StrategySyncStart()
commit : 3f14a6d84db2f21b5d61855c1df1943f7e95222b
author : Michael Paquier <michael@paquier.xyz>
date : Fri, 31 Jan 2025 11:06:10 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Fri, 31 Jan 2025 11:06:10 +0900
The top comment of StrategySyncStart() mentions BufferSync(), but this
function calls BgBufferSync(), not BufferSync().
Oversight in 9cd00c457e6a.
Author: Ashutosh Bapat
Discussion: https://postgr.es/m/CAExHW5tgkjag8i-s=RFrCn5KAWDrC4zEPPkfUKczfccPOxBRQQ@mail.gmail.com
Backpatch-through: 13
M src/backend/storage/buffer/freelist.c
Avoid integer overflow while testing wal_skip_threshold condition.
commit : b296e55b4272494c54a7a65bec99d60200480051
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Thu, 30 Jan 2025 15:36:07 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Thu, 30 Jan 2025 15:36:07 -0500
smgrDoPendingSyncs had two distinct risks of integer overflow while
deciding which way to ensure durability of a newly-created relation.
First, it accumulated the total size of all forks in a variable of
type BlockNumber (uint32). While we restrict an individual fork's
size to fit in that, I don't believe there's such a restriction on
all of them added together. Second, it proceeded to multiply the
sum by BLCKSZ, which most certainly could overflow a uint32.
(The exact expression is total_blocks * BLCKSZ / 1024. The
compiler might choose to optimize that to total_blocks * 8,
which is not at quite as much risk of overflow as a literal
reading would be, but it's still wrong.)
If an overflow did occur it could lead to a poor choice to
shove a very large relation into WAL instead of fsync'ing it.
This wouldn't be fatal, but it could be inefficient.
Change total_blocks to uint64 which should be plenty, and
rearrange the comparison calculation to be overflow-safe.
I noticed this while looking for ramifications of the proposed
change in MAX_KILOBYTES. It's not entirely clear to me why
wal_skip_threshold is limited to MAX_KILOBYTES in the
first place, but in any case this code is unsafe regardless
of the range of wal_skip_threshold.
Oversight in c6b92041d which introduced wal_skip_threshold,
so back-patch to v13.
Discussion: https://postgr.es/m/1a01f0-66ec2d80-3b-68487680@27595217
Backpatch-through: 13
M src/backend/catalog/storage.c
Handle default NULL insertion a little better.
commit : bb85d09355832e8249b1af40fd181b6e791d87b5
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Wed, 29 Jan 2025 15:31:55 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Wed, 29 Jan 2025 15:31:55 -0500
If a column is omitted in an INSERT, and there's no column default,
the code in preptlist.c generates a NULL Const to be inserted.
Furthermore, if the column is of a domain type, we wrap the Const
in CoerceToDomain, so as to throw a run-time error if the domain
has a NOT NULL constraint. That's fine as far as it goes, but
there are two problems:
1. We're being sloppy about the type/typmod that the Const is
labeled with. It really should have the domain's base type/typmod,
since it's the input to CoerceToDomain not the output. This can
result in coerce_to_domain inserting a useless length-coercion
function (useless because it's being applied to a null). The
coercion would typically get const-folded away later, but it'd
be better not to create it in the first place.
2. We're not applying expression preprocessing (specifically,
eval_const_expressions) to the resulting expression tree.
The planner's primary expression-preprocessing pass already happened,
so that means the length coercion step and CoerceToDomain node miss
preprocessing altogether.
This is at the least inefficient, since it means the length coercion
and CoerceToDomain will actually be executed for each inserted row,
though they could be const-folded away in most cases. Worse, it
seems possible that missing preprocessing for the length coercion
could result in an invalid plan (for example, due to failing to
perform default-function-argument insertion). I'm not aware of
any live bug of that sort with core datatypes, and it might be
unreachable for extension types as well because of restrictions of
CREATE CAST, but I'm not entirely convinced that it's unreachable.
Hence, it seems worth back-patching the fix (although I only went
back to v14, as the patch doesn't apply cleanly at all in v13).
There are several places in the rewriter that are building null
domain constants the same way as preptlist.c. While those are
before the planner and hence don't have any reachable bug, they're
still applying a length coercion that will be const-folded away
later, uselessly wasting cycles. Hence, make a utility routine
that all of these places can call to do it right.
Making this code more careful about the typmod assigned to the
generated NULL constant has visible but cosmetic effects on some
of the plans shown in contrib/postgres_fdw's regression tests.
Discussion: https://postgr.es/m/1865579.1738113656@sss.pgh.pa.us
Backpatch-through: 14
M contrib/postgres_fdw/expected/postgres_fdw.out
M src/backend/optimizer/prep/preptlist.c
M src/backend/parser/parse_coerce.c
M src/backend/rewrite/rewriteHandler.c
M src/backend/rewrite/rewriteManip.c
M src/include/parser/parse_coerce.h
Avoid breaking SJIS encoding while de-backslashing Windows paths.
commit : b17e3970c1ac207566a1a25063dc456d042423b5
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Wed, 29 Jan 2025 14:24:36 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Wed, 29 Jan 2025 14:24:36 -0500
When running on Windows, canonicalize_path() converts '\' to '/'
to prevent confusing the Windows command processor. It was
doing that in a non-encoding-aware fashion; but in SJIS there
are valid two-byte characters whose second byte matches '\'.
So encoding corruption ensues if such a character is used in
the path.
We can fairly easily fix this if we know which encoding is
in use, but a lot of our utilities don't have much of a clue
about that. After some discussion we decided we'd settle for
fixing this only in psql, and assuming that its value of
client_encoding matches what the user is typing.
It seems hopeless to get the server to deal with the problematic
characters in database path names, so we'll just declare that
case to be unsupported. That means nothing need be done in
the server, nor in utility programs whose only contact with
file path names is for database paths. But psql frequently
deals with client-side file paths, so it'd be good if it
didn't mess those up.
Bug: #18735
Reported-by: Koichi Suzuki <koichi.suzuki@enterprisedb.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Koichi Suzuki <koichi.suzuki@enterprisedb.com>
Discussion: https://postgr.es/m/18735-4acdb3998bb9f2b1@postgresql.org
Backpatch-through: 13
M src/bin/psql/command.c
M src/bin/psql/copy.c
M src/include/port.h
M src/port/path.c
At update of non-LP_NORMAL TID, fail instead of corrupting page header.
commit : dc02b98bd1c8d653370fd8828063b1aee0b5b9eb
author : Noah Misch <noah@leadboat.com>
date : Sat, 25 Jan 2025 11:28:14 -0800
committer: Noah Misch <noah@leadboat.com>
date : Sat, 25 Jan 2025 11:28:14 -0800
The right mix of DDL and VACUUM could corrupt a catalog page header such
that PageIsVerified() durably fails, requiring a restore from backup.
This affects only catalogs that both have a syscache and have DDL code
that uses syscache tuples to construct updates. One of the test
permutations shows a variant not yet fixed.
This makes !TransactionIdIsValid(TM_FailureData.xmax) possible with
TM_Deleted. I think core and PGXN are indifferent to that.
Per bug #17821 from Alexander Lakhin. Back-patch to v13 (all supported
versions). The test case is v17+, since it uses INJECTION_POINT.
Discussion: https://postgr.es/m/17821-dd8c334263399284@postgresql.org
M src/backend/access/heap/heapam.c
M src/include/access/tableam.h
Test ECPG decadd(), decdiv(), decmul(), and decsub() for risnull() input.
commit : 216294ba592fdfaa38bdf9767527f8424bd29b14
author : Noah Misch <noah@leadboat.com>
date : Sat, 25 Jan 2025 11:28:14 -0800
committer: Noah Misch <noah@leadboat.com>
date : Sat, 25 Jan 2025 11:28:14 -0800
Since commit 757fb0e5a9a61ac8d3a67e334faeea6dc0084b3f, these
Informix-compat functions return 0 without changing the output
parameter. Initialize the output parameter before the test call, making
that obvious. Before this, the expected test output has been depending
on freed stack memory. "gcc -ftrivial-auto-var-init=pattern" revealed
that. Back-patch to v13 (all supported versions).
Discussion: https://postgr.es/m/20250106192748.cf.nmisch@google.com
M src/interfaces/ecpg/test/compat_informix/dec_test.pgc
M src/interfaces/ecpg/test/expected/compat_informix-dec_test.c
M src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout
Doc: recommend "psql -X" for restoring pg_dump scripts.
commit : c768f54a25a431793ab17b7e64536c9360b96d8e
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 25 Jan 2025 12:42:05 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 25 Jan 2025 12:42:05 -0500
This practice avoids possible problems caused by non-default psql
options, such as disabling AUTOCOMMIT.
Author: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
Reviewed-by: Robert Treat <rob@xzilla.net>
Discussion: https://postgr.es/m/96ff23a5d858ff72ca8e823a014d16fe@oss.nttdata.com
Backpatch-through: 13
M doc/src/sgml/backup.sgml
M doc/src/sgml/ref/pg_dump.sgml
M doc/src/sgml/ref/pg_dumpall.sgml
Use the correct sizeof() in BufFileLoadBuffer
commit : 8cedf6a1b9fcbd33f97413b6a37b38d936522624
author : Tomas Vondra <tomas.vondra@postgresql.org>
date : Sat, 25 Jan 2025 00:36:48 +0100
committer: Tomas Vondra <tomas.vondra@postgresql.org>
date : Sat, 25 Jan 2025 00:36:48 +0100
The sizeof() call should reference buffer.data, because that's the
buffer we're reading data into, not the whole PGAlignedBuffer union.
This was introduced by 44cac93464, which replaced the simple buffer
with a PGAlignedBuffer field.
It's benign, because the buffer is the largest field of the union, so
the sizes are the same. But it's easy to trip over this in a patch, so
fix and backpatch. Commit 44cac93464 went into 12, but that's EOL.
Backpatch-through: 13
Discussion: https://postgr.es/m/928bdab1-6567-449f-98c4-339cd2203b87@vondra.me
M src/backend/storage/file/buffile.c
Don't ask for bug reports about pthread_is_threaded_np() != 0.
commit : 4d6e169c43418d3efed47356d7d004cf12e91c82
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Thu, 23 Jan 2025 14:23:04 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Thu, 23 Jan 2025 14:23:04 -0500
We thought that this condition was unreachable in ExitPostmaster,
but actually it's possible if you have both a misconfigured locale
setting and some other mistake that causes PostmasterMain to bail
out before reaching its own check of pthread_is_threaded_np().
Given the lack of other reports, let's not ask for bug reports if
this occurs; instead just give the same hint as in PostmasterMain.
Bug: #18783
Reported-by: anani191181515@gmail.com
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/18783-d1873b95a59b9103@postgresql.org
Discussion: https://postgr.es/m/206317.1737656533@sss.pgh.pa.us
Backpatch-through: 13
M src/backend/postmaster/postmaster.c
Repair incorrect handling of AfterTriggerSharedData.ats_modifiedcols.
commit : cdeed4de7729f455908e571a69dfd60bd6b52a82
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Wed, 22 Jan 2025 11:58:20 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Wed, 22 Jan 2025 11:58:20 -0500
This patch fixes two distinct errors that both ultimately trace
to commit 71d60e2aa, which added the ats_modifiedcols field.
The more severe error is that ats_modifiedcols wasn't accounted for
in afterTriggerAddEvent's scanning loop that looks for a pre-existing
duplicate AfterTriggerSharedData. Thus, a new event could be
incorrectly matched to an AfterTriggerSharedData that has a different
value of ats_modifiedcols, resulting in the wrong tg_updatedcols
bitmap getting passed to the trigger whenever it finally gets fired.
We'd not noticed because (a) few triggers consult tg_updatedcols,
and (b) we had no tests exercising a case where such a trigger was
called as an AFTER trigger. In the test case added by this commit,
contrib/lo's trigger fails to remove a large object when expected
because (without this fix) it thinks the LO OID column hasn't changed.
The other problem was introduced by commit ce5aaea8c, which copied the
modified-columns bitmap into trigger-related storage. It made a copy
for every trigger event, whereas what we really want is to make a new
copy only when we make a new AfterTriggerSharedData entry. (We could
imagine adding extra logic to reduce the number of bitmap copies still
more, but it doesn't look worthwhile at the moment.) In a simple test
of an UPDATE of 10000000 rows with a single AFTER trigger, this thinko
roughly tripled the amount of memory consumed by the pending-triggers
data structures, from 160446744 to 480443440 bytes.
Fixing the first problem requires introducing a bms_equal() call into
afterTriggerAddEvent's scanning loop, which is slightly annoying from
a speed perspective. However, getting rid of the excessive bms_copy()
calls from the second problem balances that out; overall speed of
trigger operations is the same or slightly better, in my tests.
Discussion: https://postgr.es/m/3496294.1737501591@sss.pgh.pa.us
Backpatch-through: 13
M contrib/lo/expected/lo.out
M contrib/lo/sql/lo.sql
M src/backend/commands/trigger.c
Fix detach of a partition that has a toplevel FK to a partitioned table
commit : 1bc092519b77a6386bb84918c7652c70b5d1fd48
author : Álvaro Herrera <alvherre@alvh.no-ip.org>
date : Tue, 21 Jan 2025 14:53:46 +0100
committer: Álvaro Herrera <alvherre@alvh.no-ip.org>
date : Tue, 21 Jan 2025 14:53:46 +0100
In common cases, foreign keys are defined on the toplevel partitioned
table; but if instead one is defined on a partition and references a
partitioned table, and the referencing partition is detached, we would
examine the pg_constraint row on the partition being detached, and fail
to realize that the sub-constraints must be left alone. This causes the
ALTER TABLE DETACH process to fail with
ERROR: could not find ON INSERT check triggers of foreign key constraint NNN
This is similar but not quite the same as what was fixed by
53af9491a043. This bug doesn't affect branches earlier than 15, because
the detach procedure was different there, so we only backpatch down to
15.
Fix by skipping such modifying constraints that are children of other
constraints being detached.
Author: Amul Sul <sulamul@gmail.com>
Diagnosys-by: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/CAAJ_b97GuPh6wQPbxQS-Zpy16Oh+0aMv-w64QcGrLhCOZZ6p+g@mail.gmail.com
M src/backend/commands/tablecmds.c
M src/test/regress/expected/foreign_key.out
M src/test/regress/sql/foreign_key.sql
Update time zone data files to tzdata release 2025a.
commit : 48bc95d0d0ff8685f8645a4cea09ad22a38d46c2
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 20 Jan 2025 16:49:15 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 20 Jan 2025 16:49:15 -0500
DST law changes in Paraguay.
Historical corrections for the Philippines.
Backpatch-through: 13
M src/timezone/data/tzdata.zi
Avoid using timezone Asia/Manila in regression tests.
commit : 8856267284a316f453655a025393fefecc338b0e
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 20 Jan 2025 15:47:53 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 20 Jan 2025 15:47:53 -0500
The freshly-released 2025a version of tzdata has a refined estimate
for the longitude of Manila, changing their value for LMT in
pre-standardized-timezone days. This changes the output of one of
our test cases. Since we need to be able to run with system tzdata
files that may or may not contain this update, we'd better stop
making that specific test.
I switched it to use Asia/Singapore, which has a roughly similar UTC
offset. That LMT value hasn't changed in tzdb since 2003, so we can
hope that it's well established.
I also noticed that this set of make_timestamptz tests only exercises
zones east of Greenwich, which seems rather sad, and was not the
original intent AFAICS. (We've already changed these tests once
to stabilize their results across tzdata updates, cf 66b737cd9;
it looks like I failed to consider the UTC-offset-sign aspect then.)
To improve that, add a test with Pacific/Honolulu. That LMT offset
is also quite old in tzdb, so we'll cross our fingers that it doesn't
get improved.
Reported-by: Christoph Berg <cb@df7cb.de>
Discussion: https://postgr.es/m/Z46inkznCxesvDEb@msg.df7cb.de
Backpatch-through: 13
M src/include/datatype/timestamp.h
M src/test/regress/expected/timestamptz.out
M src/test/regress/sql/timestamptz.sql
Fix header check for continuation records where standbys could be stuck
commit : 26554faccc975fbd23b7cc04c0b10079e5c839c9
author : Michael Paquier <michael@paquier.xyz>
date : Mon, 20 Jan 2025 09:30:37 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Mon, 20 Jan 2025 09:30:37 +0900
XLogPageRead() checks immediately for an invalid WAL record header on a
standby, to be able to handle the case of continuation records that need
to be read across two different sources. As written, the check was too
generic, applying to any target LSN. Based on an analysis by Kyotaro
Horiguchi, what really matters is to make sure that the page header is
checked when attempting to read a LSN at the boundary of a segment, to
handle the case of a continuation record that spawns across multiple
pages when dealing with multiple segments, as WAL receivers are spawned
they request WAL from the beginning of a segment. This fix has been
proposed by Kyotaro Horiguchi.
This could cause standbys to loop infinitely when dealing with a
continuation record during a timeline jump, in the case where the
contents of the record in the follow-up page are invalid.
Some regression tests are added to check such scenarios, able to
reproduce the original problem. In the test, the contents of a
continuation record are overwritten with junk zeros on its follow-up
page, and replayed on standbys. This is inspired by 039_end_of_wal.pl,
and is enough to show how standbys should react on promotion by not
being stuck. Without the fix, the test would fail with a timeout. The
test to reproduce the problem has been written by Alexander Kukushkin.
The original check has been introduced in 066871980183, for a similar
problem.
Author: Kyotaro Horiguchi, Alexander Kukushkin
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAFh8B=mozC+e1wGJq0H=0O65goZju+6ab5AU7DEWCSUA2OtwDg@mail.gmail.com
Backpatch-through: 13
M src/backend/access/transam/xlogrecovery.c
A src/test/recovery/t/043_no_contrecord_switch.pl
Fix readlink() for non-PostgreSQL junction points on Windows.
commit : 9b136b0f2e531e41d1949fba3f618405adfcef02
author : Thomas Munro <tmunro@postgresql.org>
date : Tue, 25 Oct 2022 15:21:42 +1300
committer: Andrew Dunstan <andrew@dunslane.net>
date : Tue, 25 Oct 2022 15:21:42 +1300
Since commit c5cb8f3b taught stat() to follow symlinks, and since initdb
uses pg_mkdir_p(), and that examines parent directories, our humble
readlink() implementation can now be exposed to junction points not of
PostgreSQL origin. Those might be corrupted by our naive path mangling,
which doesn't really understand NT paths in general.
Simply decline to transform paths that don't look like a drive absolute
path. That means that readlink() returns the NT path directly when
checking a parent directory of PGDATA that happen to point to a drive
using "rooted" format. That works for the purposes of our stat()
emulation.
Reported-by: Roman Zharkov <r.zharkov@postgrespro.ru>
Reviewed-by: Roman Zharkov <r.zharkov@postgrespro.ru>
Discussion: https://postgr.es/m/4590c37927d7b8ee84f9855d83229018%40postgrespro.ru
Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com
Backpatched commit f71007fb as above by Thomas Munro into releases 13 thru 15
Discussion: https://postgr.es/m/CA+hUKGLbnv+pe3q1fYOVkLD3pMra7GuihfMxUN-1831YH9RYQg@mail.gmail.com
M src/port/dirmod.c
Fix stat() for recursive junction points on Windows.
commit : e708f31881fd767af6d665968f6124ab82a08962
author : Thomas Munro <tmunro@postgresql.org>
date : Tue, 25 Oct 2022 15:24:41 +1300
committer: Andrew Dunstan <andrew@dunslane.net>
date : Tue, 25 Oct 2022 15:24:41 +1300
Commit c5cb8f3b supposed that we'd only ever have to follow one junction
point in stat(), because we don't construct longer chains of them ourselves.
When examining a parent directory supplied by the user, we should really be
able to cope with longer chains, just in case someone has their system
set up that way. Choose an arbitrary cap of 8, to match the minimum
acceptable value of SYMLOOP_MAX in POSIX.
Previously I'd avoided reporting ELOOP thinking Windows didn't have it,
but it turns out that it does, so we can use the proper error number.
Reviewed-by: Roman Zharkov <r.zharkov@postgrespro.ru>
Discussion: https://postgr.es/m/CA%2BhUKGJ7JDGWYFt9%3D-TyJiRRy5q9TtPfqeKkneWDr1XPU1%2Biqw%40mail.gmail.com
Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com
Backpatched commit 4517358e as above by Thomas Munro into releases 13 thru 15
Discussion: https://postgr.es/m/CA+hUKGLbnv+pe3q1fYOVkLD3pMra7GuihfMxUN-1831YH9RYQg@mail.gmail.com
M src/port/win32stat.c
Revert recent changes related to handling of 2PC files at recovery
commit : 42c900d31e57419359a2838861865622cf470489
author : Michael Paquier <michael@paquier.xyz>
date : Fri, 17 Jan 2025 13:27:45 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Fri, 17 Jan 2025 13:27:45 +0900
This commit reverts 8f67f994e8ea (down to v13) and c3de0f9eed38 (down to
v17), as these are proving to not be completely correct regarding two
aspects:
- In v17 and newer branches, c3de0f9eed38's check for epoch handling is
incorrect, and does not correctly handle frozen epochs. A logic closer
to widen_snapshot_xid() should be used. The 2PC code should try to
integrate deeper with FullTransactionIds, 5a1dfde8334b being not enough.
- In v13 and newer branches, 8f67f994e8ea is a workaround for the real
issue, which is that we should not attempt CLOG lookups without reaching
consistency. This exists since 728bd991c3c4, and this is reachable with
ProcessTwoPhaseBuffer() called by restoreTwoPhaseData() at the beginning
of recovery.
Per discussion with Noah Misch.
Discussion: https://postgr.es/m/20250116010051.f3.nmisch@google.com
Backpatch-through: 13
M src/backend/access/transam/twophase.c
M src/test/recovery/t/009_twophase.pl
Fix setrefs.c's failure to do expression processing on prune steps.
commit : 724ebebb110493f2000114404188cc3e935153f6
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Thu, 16 Jan 2025 20:40:07 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Thu, 16 Jan 2025 20:40:07 -0500
We should run the expression subtrees of PartitionedRelPruneInfo
structs through fix_scan_expr. Failure to do so means that
AlternativeSubPlans within those expressions won't be cleaned up
properly, resulting in "unrecognized node type" errors since v14.
It seems fairly likely that at least some of the other steps done
by fix_scan_expr are important here as well, resulting in as-yet-
undetected bugs. Therefore, I've chosen to back-patch this to
all supported branches including v13, even though the known
symptom doesn't manifest in v13.
Per bug #18778 from Alexander Lakhin.
Discussion: https://postgr.es/m/18778-24cd399df6c806af@postgresql.org
M src/backend/optimizer/plan/setrefs.c
M src/test/regress/expected/partition_prune.out
M src/test/regress/sql/partition_prune.sql
Move routines to manipulate WAL into PostgreSQL::Test::Cluster
commit : e5d113057d5fb8e6ecd39521784d48d4966f6a53
author : Michael Paquier <michael@paquier.xyz>
date : Thu, 16 Jan 2025 09:26:28 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Thu, 16 Jan 2025 09:26:28 +0900
These facilities were originally in the recovery TAP test
039_end_of_wal.pl. A follow-up bug fix with a TAP test doing similar
WAL manipulations requires them, and all these had better not be
duplicated due to their complexity. The routine names are tweaked to
use "wal" more consistently, similarly to the existing "advance_wal".
In v14 and v13, the new routines are moved to PostgresNode.pm.
039_end_of_wal.pl is updated to use the refactored routines, without
changing its coverage.
Reviewed-by: Alexander Kukushkin
Discussion: https://postgr.es/m/CAFh8B=mozC+e1wGJq0H=0O65goZju+6ab5AU7DEWCSUA2OtwDg@mail.gmail.com
Backpatch-through: 13
M src/test/perl/PostgreSQL/Test/Cluster.pm
M src/test/recovery/t/039_end_of_wal.pl
Avoid symbol collisions between pqsignal.c and legacy-pqsignal.c.
commit : a3b709cf73e0ae2be1259afec9994ff30416f695
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Tue, 14 Jan 2025 18:50:24 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Tue, 14 Jan 2025 18:50:24 -0500
In the name of ABI stability (that is, to avoid a library major
version bump for libpq), libpq still exports a version of pqsignal()
that we no longer want to use ourselves. However, since that has
the same link name as the function exported by src/port/pqsignal.c,
there is a link ordering dependency determining which version will
actually get used by code that uses libpq as well as libpgport.a.
It now emerges that the wrong version has been used by pgbench and
psql since commit 06843df4a rearranged their link commands. This
can result in odd failures in pgbench with the -T switch, since its
SIGALRM handler will now not be marked SA_RESTART. psql may have
some edge-case problems in \watch, too.
Since we don't want to depend on link ordering effects anymore,
let's fix this in the same spirit as b6c7cfac8: use macros to change
the actual link names of the competing functions. We cannot change
legacy-pqsignal.c's exported name of course, so the victim has to be
src/port/pqsignal.c.
In master, rename its exported name to be pqsignal_fe in frontend or
pqsignal_be in backend. (We could perhaps have gotten away with using
the same symbol in both cases, but since the FE and BE versions now
work a little differently, it seems advisable to use different names.)
In back branches, rename to pqsignal_fe in frontend but keep it as
pqsignal in backend. The frontend change could affect third-party
code that is calling pqsignal from libpgport.a or libpgport_shlib.a,
but only if the code is compiled against port.h from a different minor
release than libpgport. Since we don't support using libpgport as a
shared library, it seems unlikely that there will be such a problem.
I left the backend symbol unchanged to avoid an ABI break for
extensions. This means that the link ordering hazard still exists
for any extension that links against libpq. However, none of our own
extensions use both pqsignal() and libpq, and we're not making things
any worse for third-party extensions that do.
Report from Andy Fan, diagnosis by Fujii Masao, patch by me.
Back-patch to all supported branches, as 06843df4a was.
Discussion: https://postgr.es/m/87msfz5qv2.fsf@163.com
M src/include/port.h
M src/interfaces/libpq/legacy-pqsignal.c
M src/port/pqsignal.c
ecpg: Restore detection of unsupported COPY FROM STDIN.
commit : 71ef47cf0f20e680319e866220e05c208b3e2bd8
author : Fujii Masao <fujii@postgresql.org>
date : Wed, 15 Jan 2025 01:24:24 +0900
committer: Fujii Masao <fujii@postgresql.org>
date : Wed, 15 Jan 2025 01:24:24 +0900
The ecpg command includes code to warn about unsupported COPY FROM STDIN
statements in input files. However, since commit 3d009e45bd,
this functionality has been broken due to a bug introduced in that commit,
causing ecpg to fail to detect the statement.
This commit resolves the issue, restoring ecpg's ability to detect
COPY FROM STDIN and issue a warning as intended.
Back-patch to all supported versions.
Author: Ryo Kanbayashi
Reviewed-by: Hayato Kuroda, Tom Lane
Discussion: https://postgr.es/m/CANOn0Ez_t5uDCUEV8c1YORMisJiU5wu681eEVZzgKwOeiKhkqQ@mail.gmail.com
M src/interfaces/ecpg/preproc/ecpg.addons
Fix catcache invalidation of a list entry that's being built
commit : ce7c406f0f8d562e856d43a7e1feb82043f772bc
author : Heikki Linnakangas <heikki.linnakangas@iki.fi>
date : Tue, 14 Jan 2025 14:28:49 +0200
committer: Heikki Linnakangas <heikki.linnakangas@iki.fi>
date : Tue, 14 Jan 2025 14:28:49 +0200
If a new catalog tuple is inserted that belongs to a catcache list
entry, and cache invalidation happens while the list entry is being
built, the list entry might miss the newly inserted tuple.
To fix, change the way we detect concurrent invalidations while a
catcache entry is being built. Keep a stack of entries that are being
built, and apply cache invalidation to those entries in addition to
the real catcache entries. This is similar to the in-progress list in
relcache.c.
Back-patch to all supported versions.
Reviewed-by: Noah Misch
Discussion: https://www.postgresql.org/message-id/2234dc98-06fe-42ed-b5db-ac17384dc880@iki.fi
M src/backend/utils/cache/catcache.c
M src/backend/utils/cache/inval.c
M src/include/utils/catcache.h
M src/tools/pgindent/typedefs.list
Fix potential integer overflow in bringetbitmap()
commit : 9e9f30139eb87e30be64dc34fad6a01e029e8a56
author : Michael Paquier <michael@paquier.xyz>
date : Tue, 14 Jan 2025 15:13:17 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Tue, 14 Jan 2025 15:13:17 +0900
This function expects an "int64" as result and stores the number of
pages to add to the index scan bitmap as an "int", multiplying its final
result by 10. For a relation large enough, this can theoretically
overflow if counting more than (INT32_MAX / 10) pages, knowing that the
number of pages is upper-bounded by MaxBlockNumber.
To avoid the overflow, this commit redefines "totalpages", used to
calculate the result, to be an "int64" rather than an "int".
Reported-by: Evgeniy Gorbanyov
Author: James Hunter
Discussion: https://www.postgresql.org/message-id/07704817-6fa0-460c-b1cf-cd18f7647041@basealt.ru
Backpatch-through: 13
M src/backend/access/brin/brin.c
Fix HBA option count
commit : 9ad7a32b2a151a7052016e55acdeb20247acefdd
author : Daniel Gustafsson <dgustafsson@postgresql.org>
date : Sun, 12 Jan 2025 23:44:39 +0100
committer: Daniel Gustafsson <dgustafsson@postgresql.org>
date : Sun, 12 Jan 2025 23:44:39 +0100
Commit 27a1f8d108 missed updating the max HBA option count to
account for the new option added. Fix by bumping the counter
and adjust the relevant comment to match. Backpatch down to
all supported branches like the erroneous commit.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/286764.1736697356@sss.pgh.pa.us
Backpatch-through: v13
M src/backend/utils/adt/hbafuncs.c
Fix XMLTABLE() deparsing to quote namespace names if necessary.
commit : 7c0379516f367cf6a3709c46b2fed12777468130
author : Dean Rasheed <dean.a.rasheed@gmail.com>
date : Sun, 12 Jan 2025 12:59:40 +0000
committer: Dean Rasheed <dean.a.rasheed@gmail.com>
date : Sun, 12 Jan 2025 12:59:40 +0000
When deparsing an XMLTABLE() expression, XML namespace names were not
quoted. However, since they are parsed as ColLabel tokens, some names
require double quotes to ensure that they are properly interpreted.
Fix by using quote_identifier() in the deparsing code.
Back-patch to all supported versions.
Dean Rasheed, reviewed by Tom Lane.
Discussion: https://postgr.es/m/CAEZATCXTpAS%3DncfLNTZ7YS6O5puHeLg_SUYAit%2Bcs7wsrd9Msg%40mail.gmail.com
M src/backend/utils/adt/ruleutils.c
M src/test/regress/expected/xml.out
M src/test/regress/expected/xml_1.out
M src/test/regress/expected/xml_2.out
M src/test/regress/sql/xml.sql
Repair memory leaks in plpython.
commit : 71bb9c4b2a1425877e0ef2a983bb855c6ce7bffc
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 11 Jan 2025 11:45:56 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 11 Jan 2025 11:45:56 -0500
PLy_spi_execute_plan (PLyPlan.execute) and PLy_cursor_plan
(plpy.cursor) use PLy_output_convert to convert Python values
into Datums that can be passed to the query-to-execute. But they
failed to pay much attention to its warning that it can leave "cruft
generated along the way" behind. Repeated use of these methods can
result in a substantial memory leak for the duration of the calling
plpython function.
To fix, make a temporary memory context to invoke PLy_output_convert
in. This also lets us get rid of the rather fragile code that was
here for retail pfree's of the converted Datums. Indeed, we don't
need the PLyPlanObject.values field anymore at all, though I left it
in place in the back branches in the name of ABI stability.
Mat Arye and Tom Lane, per report from Mat Arye. Back-patch to all
supported branches.
Discussion: https://postgr.es/m/CADsUR0DvVgnZYWwnmKRK65MZg7YLUSTDLV61qdnrwtrAJgU6xw@mail.gmail.com
M src/pl/plpython/plpy_cursorobject.c
M src/pl/plpython/plpy_spi.c
Fix missing ldapscheme option in pg_hba_file_rules()
commit : 830215a4c8879dbed6aeec2ae67be050ec9b7d60
author : Daniel Gustafsson <dgustafsson@postgresql.org>
date : Fri, 10 Jan 2025 22:02:58 +0100
committer: Daniel Gustafsson <dgustafsson@postgresql.org>
date : Fri, 10 Jan 2025 22:02:58 +0100
The ldapscheme option was missed when inspecing the HbaLine for
assembling rows for the pg_hba_file_rules function. Backpatch
to all supported versions.
Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reported-by: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Bug: 18769
Discussion: https://postgr.es/m/18769-dd8610cbc0405172@postgresql.org
Backpatch-through: v13
M src/backend/utils/adt/hbafuncs.c
Fix off_t overflow in pg_basebackup on Windows.
commit : 6b6901a26f5683d30e6a3192d5b3a7da1dba69c3
author : Thomas Munro <tmunro@postgresql.org>
date : Thu, 9 Jan 2025 13:17:36 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Thu, 9 Jan 2025 13:17:36 +1300
walmethods.c used off_t to navigate around a pg_wal.tar file that could
exceed 2GB, which doesn't work on Windows and would fail with misleading
errors. Use pgoff_t instead.
Back-patch to all supported branches.
Author: Davinder Singh <davinder.singh@enterprisedb.com>
Reported-by: Jakub Wartak <jakub.wartak@enterprisedb.com>
Discussion: https://postgr.es/m/CAKZiRmyM4YnokK6Oenw5JKwAQ3rhP0YTz2T-tiw5dAQjGRXE3Q%40mail.gmail.com
M src/bin/pg_basebackup/receivelog.c
M src/bin/pg_basebackup/walmethods.c
M src/bin/pg_basebackup/walmethods.h
Provide 64-bit ftruncate() and lseek() on Windows.
commit : 70a7a37610f70f546ea651021c970f7d0c4b81b3
author : Thomas Munro <tmunro@postgresql.org>
date : Thu, 9 Jan 2025 12:10:26 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Thu, 9 Jan 2025 12:10:26 +1300
Change our ftruncate() macro to use the 64-bit variant of chsize(), and
add a new macro to redirect lseek() to _lseeki64().
Back-patch to all supported releases, in preparation for a bug fix.
Tested-by: Davinder Singh <davinder.singh@enterprisedb.com>
Discussion: https://postgr.es/m/CAKZiRmyM4YnokK6Oenw5JKwAQ3rhP0YTz2T-tiw5dAQjGRXE3Q%40mail.gmail.com
M src/include/port.h
M src/include/port/win32_port.h
M src/pl/plperl/plperl_system.h
Fix C error reported by Oracle compiler.
commit : 190054e61f5d2079aa5bc7a50206fe96bf4438f5
author : Thomas Munro <tmunro@postgresql.org>
date : Wed, 8 Jan 2025 16:54:45 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Wed, 8 Jan 2025 16:54:45 +1300
Commit 66aaabe7 (branches 13 - 17 only) was not acceptable to the Oracle
Developer Studio compiler on build farm animal wrasse. It accidentally
used a C++ style return statement to wrap a void function. None of the
usual compilers complained, but it is right, that is not allowed in C.
Fix.
Reported-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/Z33vgfVgvOnbFLN9%40paquier.xyz
M src/backend/storage/smgr/smgr.c
Restore smgrtruncate() prototype in back-branches.
commit : 3181befdca7142d3563b4399e51e5c60da54def0
author : Thomas Munro <tmunro@postgresql.org>
date : Wed, 8 Jan 2025 07:50:30 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Wed, 8 Jan 2025 07:50:30 +1300
It's possible that external code is calling smgrtruncate(). Any
external callers might like to consider the recent changes to
RelationTruncate(), but commit 38c579b0 should not have changed the
function prototype in the back-branches, per ABI stability policy.
Restore smgrtruncate()'s traditional argument list in the back-branches,
but make it a wrapper for a new function smgrtruncate2(). The three
callers in core can use smgrtruncate2() directly. In master (18-to-be),
smgrtruncate2() is effectively renamed to smgrtruncate(), so this wart
is cleaned up.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKG%2BThae6x6%2BjmQiuALQBT2Ae1ChjMh1%3DkMvJ8y_SBJZrvA%40mail.gmail.com
M contrib/pg_visibility/pg_visibility.c
M src/backend/catalog/storage.c
M src/backend/storage/smgr/smgr.c
M src/include/storage/smgr.h
Document strange jsonb sort order for empty top level arrays
commit : bd606ed8ec65f7ad388c3dead9ba200bf0b4fa6a
author : Andrew Dunstan <andrew@dunslane.net>
date : Fri, 3 Jan 2025 09:23:46 -0500
committer: Andrew Dunstan <andrew@dunslane.net>
date : Fri, 3 Jan 2025 09:23:46 -0500
Slightly faulty logic in the original jsonb code (commit d9134d0a355)
results in an empty top level array sorting less than a json null. We
can't change the sort order now since it would affect btree indexes over
jsonb, so document the anomaly.
Backpatch to all live branches (13 .. 17)
In master, also add a code comment noting the anomaly.
Reported-by: Yan Chengpen
Reviewed-by: Jian He
Discussion: https://postgr.es/m/OSBPR01MB45199DD8DA2D1CECD50518188E272@OSBPR01MB4519.jpnprd01.prod.outlook.com
M doc/src/sgml/json.sgml
Update copyright for 2025
commit : 24a1948ad2b3a9a6730bb1f084891d8d2be85fea
author : Bruce Momjian <bruce@momjian.us>
date : Wed, 1 Jan 2025 11:21:54 -0500
committer: Bruce Momjian <bruce@momjian.us>
date : Wed, 1 Jan 2025 11:21:54 -0500
Backpatch-through: 13
M COPYRIGHT
M doc/src/sgml/legal.sgml
Fix memory leak in pgoutput with relation attribute map
commit : da8bd5d424e00a3dd38bfc4f440c0ac346c5b3c1
author : Michael Paquier <michael@paquier.xyz>
date : Mon, 30 Dec 2024 13:34:01 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Mon, 30 Dec 2024 13:34:01 +0900
pgoutput caches the attribute map of a relation, that is free()'d only
when validating a RelationSyncEntry. However, this code path is not
taken when calling any of the SQL functions able to do some logical
decoding, like pg_logical_slot_{get,peek}_changes(), leaking some memory
into CacheMemoryContext on repeated calls.
To address this, a relation's attribute map is allocated in
PGOutputData's cachectx, free()'d at the end of the execution of these
SQL functions when logical decoding ends. This is available down to 15.
v13 and v14 have a similar leak, which will be dealt with later.
Reported-by: Masahiko Sawada
Author: Vignesh C
Reviewed-by: Hou Zhijie
Discussion: https://postgr.es/m/CAD21AoDkAhQVSukOfH3_reuF-j4EU0-HxMqU3dU+bSTxsqT14Q@mail.gmail.com
Discussion: https://postgr.es/m/CALDaNm1hewNAsZ_e6FF52a=9drmkRJxtEPrzCB6-9mkJyeBBqA@mail.gmail.com
Backpatch-through: 15
M src/backend/replication/pgoutput/pgoutput.c
Fix handling of orphaned 2PC files in the future at recovery
commit : d9ade6f2d4cdf95097bbb927f575611b525a2646
author : Michael Paquier <michael@paquier.xyz>
date : Mon, 30 Dec 2024 08:06:43 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Mon, 30 Dec 2024 08:06:43 +0900
Before 728bd991c3c4, that has improved the support for 2PC files during
recovery, the initial logic scanning files in pg_twophase was done so as
files in the future of the transaction ID horizon were checked first,
followed by a check if a transaction ID is aborted or committed which
could involve a pg_xact lookup. After this commit, these checks have
been done in reverse order.
Files detected as in the future do not have a state that can be checked
in pg_xact, hence this caused recovery to fail abruptly should an
orphaned 2PC file in the future of the transaction ID horizon exist in
pg_twophase at the beginning of recovery.
A test is added to check for this scenario, using an empty 2PC with a
transaction ID large enough to be in the future when running the test.
This test is added in 16 and older versions for now. 17 and newer
versions are impacted by a second bug caused by the addition of the
epoch in the 2PC file names. An equivalent test will be added in these
branches in a follow-up commit, once the second set of issues reported
are fixed.
Author: Vitaly Davydov, Michael Paquier
Discussion: https://postgr.es/m/11e597-676ab680-8d-374f23c0@145466129
Backpatch-through: 13
M src/backend/access/transam/twophase.c
M src/test/recovery/t/009_twophase.pl
Exclude parallel workers from connection privilege/limit checks.
commit : 3d1ecc92a0d15eb299ea37c9842b3367eddc0081
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 28 Dec 2024 16:08:50 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 28 Dec 2024 16:08:50 -0500
Cause parallel workers to not check datallowconn, rolcanlogin, and
ACL_CONNECT privileges. The leader already checked these things
(except for rolcanlogin which might have been checked for a different
role). Re-checking can accomplish little except to induce unexpected
failures in applications that might not even be aware that their query
has been parallelized. We already had the principle that parallel
workers rely on their leader to pass a valid set of authorization
information, so this change just extends that a bit further.
Also, modify the ReservedConnections, datconnlimit and rolconnlimit
logic so that these limits are only enforced against regular backends,
and only regular backends are counted while checking if the limits
were already reached. Previously, background processes that had an
assigned database or role were subject to these limits (with rather
random exclusions for autovac workers and walsenders), and the set of
existing processes that counted against each limit was quite haphazard
as well. The point of these limits, AFAICS, is to ensure the
availability of PGPROC slots for regular backends. Since all other
types of processes have their own separate pools of PGPROC slots, it
makes no sense either to enforce these limits against them or to count
them while enforcing the limit.
While edge-case failures of these sorts have been possible for a
long time, the problem got a good deal worse with commit 5a2fed911
(CVE-2024-10978), which caused parallel workers to make some of these
checks using the leader's current role where before we had used its
AuthenticatedUserId, thus allowing parallel queries to fail after
SET ROLE. The previous behavior was fairly accidental and I have
no desire to return to it.
This patch includes reverting 73c9f91a1, which was an emergency hack
to suppress these same checks in some cases. It wasn't complete,
as shown by a recent bug report from Laurenz Albe. We can also revert
fd4d93d26 and 492217301, which hacked around the same problems in one
regression test.
In passing, remove the special case for autovac workers in
CheckMyDatabase; it seems cleaner to have AutoVacWorkerMain pass
the INIT_PG_OVERRIDE_ALLOW_CONNS flag, now that that does what's
needed.
Like 5a2fed911, back-patch to supported branches (which sadly no
longer includes v12).
Discussion: https://postgr.es/m/1808397.1735156190@sss.pgh.pa.us
M src/backend/access/transam/parallel.c
M src/backend/access/transam/twophase.c
M src/backend/postmaster/autovacuum.c
M src/backend/storage/ipc/procarray.c
M src/backend/storage/lmgr/proc.c
M src/backend/utils/init/miscinit.c
M src/backend/utils/init/postinit.c
M src/include/miscadmin.h
M src/include/storage/proc.h
In REASSIGN OWNED of a database, lock the tuple as mandated.
commit : 83bb52375630ce93660bde938ffba93c6dc7fc63
author : Noah Misch <noah@leadboat.com>
date : Sat, 28 Dec 2024 07:16:22 -0800
committer: Noah Misch <noah@leadboat.com>
date : Sat, 28 Dec 2024 07:16:22 -0800
Commit aac2c9b4fde889d13f859c233c2523345e72d32b mandated such locking
and attempted to fulfill that mandate, but it missed REASSIGN OWNED.
Hence, it remained possible to lose VACUUM's inplace update of
datfrozenxid if a REASSIGN OWNED processed that database at the same
time. This didn't affect the other inplace-updated catalog, pg_class.
For pg_class, REASSIGN OWNED calls ATExecChangeOwner() instead of the
generic AlterObjectOwner_internal(), and ATExecChangeOwner() fulfills
the locking mandate.
Like in GRANT, implement this by following the locking protocol for any
catalog subject to the generic AlterObjectOwner_internal(). It would
suffice to do this for IsInplaceUpdateOid() catalogs only. Back-patch
to v13 (all supported versions).
Kirill Reshke. Reported by Alexander Kukushkin.
Discussion: https://postgr.es/m/CAFh8B=mpKjAy4Cuun-HP-f_vRzh2HSvYFG3rhVfYbfEBUhBAGg@mail.gmail.com
M src/backend/catalog/objectaddress.c
M src/backend/commands/alter.c
M src/include/catalog/objectaddress.h
M src/test/regress/expected/database.out
M src/test/regress/sql/database.sql
Fix memory leak in pgoutput with publication list cache
commit : 6c9b3975407dcfaabd195c2e239cdd0451f7fbd1
author : Michael Paquier <michael@paquier.xyz>
date : Mon, 23 Dec 2024 12:48:09 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Mon, 23 Dec 2024 12:48:09 +0900
The pgoutput module caches publication names in a list and frees it upon
invalidation. However, the code forgot to free the actual publication
names within the list elements, as publication names are pstrdup()'d in
GetPublication(). This would cause memory to leak in
CacheMemoryContext, bloating it over time as this context is not
cleaned.
This is a problem for WAL senders running for a long time, as an
accumulation of invalidation requests would bloat its cache memory
usage. A second case, where this leak is easier to see, involves a
backend calling SQL functions like pg_logical_slot_{get,peek}_changes()
which create a new decoding context with each execution. More
publications create more bloat.
To address this, this commit adds a new memory context within the
logical decoding context and resets it each time the publication names
cache is invalidated, based on a suggestion from Amit Kapila. This
ensures that the lifespan of the publication names aligns with that of
the logical decoding context.
Contrary to the HEAD-only commit f0c569d71515 that has changed
PGOutputData to track this new child memory context, the context is
tracked with a static variable whose state is reset with a MemoryContext
reset callback attached to PGOutputData->context, so as ABI
compatibility is preserved in stable branches. This approach is based
on an suggestion from Amit Kapila.
Analyzed-by: Michael Paquier, Jeff Davis
Author: Masahiko Sawada
Reviewed-by: Amit Kapila, Michael Paquier, Euler Taveira, Hou Zhijie
Discussion: https://postgr.es/m/Z0khf9EVMVLOc_YY@paquier.xyz
Backpatch-through: 13
M src/backend/replication/pgoutput/pgoutput.c
Update TransactionXmin when MyProc->xmin is updated
commit : acd5c28db5d0cdb9446c5cb09996629f78287b82
author : Heikki Linnakangas <heikki.linnakangas@iki.fi>
date : Sat, 21 Dec 2024 23:42:39 +0200
committer: Heikki Linnakangas <heikki.linnakangas@iki.fi>
date : Sat, 21 Dec 2024 23:42:39 +0200
GetSnapshotData() set TransactionXmin = MyProc->xmin, but when
SnapshotResetXmin() advanced MyProc->xmin, it did not advance
TransactionXmin correspondingly. That meant that TransactionXmin could
be older than MyProc->xmin, and XIDs between than TransactionXmin and
the real MyProc->xmin could be vacuumed away. One known consequence is
in pg_subtrans lookups: we might try to look up the status of an XID
that was already truncated away.
Back-patch to all supported versions.
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/d27a046d-a1e4-47d1-a95c-fbabe41debb4@iki.fi
M src/backend/utils/time/snapmgr.c
Fix corruption when relation truncation fails.
commit : fb540b6aa5abe68d7ba1d84a90f5afd9c78564e3
author : Thomas Munro <tmunro@postgresql.org>
date : Fri, 20 Dec 2024 21:53:25 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Fri, 20 Dec 2024 21:53:25 +1300
RelationTruncate() does three things, while holding an
AccessExclusiveLock and preventing checkpoints:
1. Logs the truncation.
2. Drops buffers, even if they're dirty.
3. Truncates some number of files.
Step 2 could previously be canceled if it had to wait for I/O, and step
3 could and still can fail in file APIs. All orderings of these
operations have data corruption hazards if interrupted, so we can't give
up until the whole operation is done. When dirty pages were discarded
but the corresponding blocks were left on disk due to ERROR, old page
versions could come back from disk, reviving deleted data (see
pgsql-bugs #18146 and several like it). When primary and standby were
allowed to disagree on relation size, standbys could panic (see
pgsql-bugs #18426) or revive data unknown to visibility management on
the primary (theorized).
Changes:
* WAL is now unconditionally flushed first
* smgrtruncate() is now called in a critical section, preventing
interrupts and causing PANIC on file API failure
* smgrtruncate() has a new parameter for existing fork sizes,
because it can't call smgrnblocks() itself inside a critical section
The changes apply to RelationTruncate(), smgr_redo() and
pg_truncate_visibility_map(). That last is also brought up to date with
other evolutions of the truncation protocol.
The VACUUM FileTruncate() failure mode had been discussed in older
reports than the ones referenced below, with independent analysis from
many people, but earlier theories on how to fix it were too complicated
to back-patch. The more recently invented cancellation bug was
diagnosed by Alexander Lakhin. Other corruption scenarios were spotted
by me while iterating on this patch and earlier commit 75818b3a.
Back-patch to all supported releases.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reported-by: rootcause000@gmail.com
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/18146-04e908c662113ad5%40postgresql.org
Discussion: https://postgr.es/m/18426-2d18da6586f152d6%40postgresql.org
M contrib/pg_visibility/pg_visibility.c
M src/backend/catalog/storage.c
M src/backend/storage/smgr/md.c
M src/backend/storage/smgr/smgr.c
M src/include/storage/md.h
M src/include/storage/smgr.h
Replace durable_rename_excl() by durable_rename(), take two
commit : c1c9df3159cfa91416bebe56ae50bc32d8a4e10b
author : Michael Paquier <michael@paquier.xyz>
date : Tue, 5 Jul 2022 10:16:12 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Tue, 5 Jul 2022 10:16:12 +0900
durable_rename_excl() attempts to avoid overwriting any existing files
by using link() and unlink(), and it falls back to rename() on some
platforms (aka WIN32), which offers no such overwrite protection. Most
callers use durable_rename_excl() just in case there is an existing
file, but in practice there shouldn't be one (see below for more
details).
Furthermore, failures during durable_rename_excl() can result in
multiple hard links to the same file. As per Nathan's tests, it is
possible to end up with two links to the same file in pg_wal after a
crash just before unlink() during WAL recycling. Specifically, the test
produced links to the same file for the current WAL file and the next
one because the half-recycled WAL file was re-recycled upon restarting,
leading to WAL corruption.
This change replaces all the calls of durable_rename_excl() to
durable_rename(). This removes the protection against accidentally
overwriting an existing file, but some platforms are already living
without it and ordinarily there shouldn't be one. The function itself
is left around in case any extensions are using it. It will be removed
on HEAD via a follow-up commit.
Here is a summary of the existing callers of durable_rename_excl() (see
second discussion link at the bottom), replaced by this commit. First,
basic_archive used it to avoid overwriting an archive concurrently
created by another server, but as mentioned above, it will still
overwrite files on some platforms. Second, xlog.c uses it to recycle
past WAL segments, where an overwrite should not happen (origin of the
change at f0e37a8) because there are protections about the WAL segment
to select when recycling an entry. The third and last area is related
to the write of timeline history files. writeTimeLineHistory() will
write a new timeline history file at the end of recovery on promotion,
so there should be no such files for the same timeline.
What remains is writeTimeLineHistoryFile(), that can be used in parallel
by a WAL receiver and the startup process, and some digging of the
buildfarm shows that EEXIST from a WAL receiver can happen with an error
of "could not link file \"pg_wal/xlogtemp.NN\" to \"pg_wal/MM.history\",
which would cause an automatic restart of the WAL receiver as it is
promoted to FATAL, hence this should improve the stability of the WAL
receiver as rename() would overwrite an existing TLI history file
already fetched by the startup process at recovery.
This is the second time this change is attempted, ccfbd92 being the
first one, but this time no assertions are added for the case of a TLI
history file written concurrently by the WAL receiver or the startup
process because we can expect one to exist (some of the TAP tests are
able to trigger with a proper timing).
This commit has been originally applied on v16~ as of dac1ff30906b, and
we have received more reports of this issue, where clusters can become
corrupted at replay in older stable branches with multiple links
pointing to the same physical WAL segment file. This backpatch
addresses the problem for the v13~v15 range.
Author: Nathan Bossart
Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/20220407182954.GA1231544@nathanxps13
Discussion: https://postgr.es/m/Ym6GZbqQdlalSKSG@paquier.xyz
Discussion: https://postgr.es/m/CAJhEC04tBkYPF4q2uS_rCytauvNEVqdBAzasBEokfceFhF=KDQ@mail.gmail.com
M contrib/basic_archive/basic_archive.c
M src/backend/access/transam/timeline.c
M src/backend/access/transam/xlog.c
Fix Assert failure in WITH RECURSIVE UNION queries
commit : ef178d38bb493baa49dc0824656e60717e8a8b1d
author : David Rowley <drowley@postgresql.org>
date : Thu, 19 Dec 2024 13:13:01 +1300
committer: David Rowley <drowley@postgresql.org>
date : Thu, 19 Dec 2024 13:13:01 +1300
If the non-recursive part of a recursive CTE ended up using
TTSOpsBufferHeapTuple as the table slot type, then a duplicate value
could cause an Assert failure in CheckOpSlotCompatibility() when
checking the hash table for the duplicate value. The expected slot type
for the deform step was TTSOpsMinimalTuple so the Assert failed when the
TTSOpsBufferHeapTuple slot was used.
This is a long-standing bug which we likely didn't notice because it
seems much more likely that the non-recursive term would have required
projection and used a TTSOpsVirtual slot, which CheckOpSlotCompatibility
is ok with.
There doesn't seem to be any harm done here other than the Assert
failure. Both TTSOpsMinimalTuple and TTSOpsBufferHeapTuple slot types
require tuple deformation, so the EEOP_*_FETCHSOME ExprState step would
have properly existed in the ExprState.
The solution is to pass NULL for the ExecBuildGroupingEqual's 'lops'
parameter. This means the ExprState's EEOP_*_FETCHSOME step won't
expect a fixed slot type. This makes CheckOpSlotCompatibility() happy as
no checking is performed when the ExprEvalStep is not expecting a fixed
slot type.
Reported-by: Richard Guo
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAMbWs4-8U9q2LAtf8+ghV11zeUReA3AmrYkxzBEv0vKnDxwkKA@mail.gmail.com
Backpatch-through: 13, all supported versions
M src/backend/executor/execGrouping.c
M src/test/regress/expected/with.out
M src/test/regress/sql/with.sql
Accommodate very large dshash tables.
commit : 9f7b7d5168acd9f0147655716b1d00ec54aeaa22
author : Nathan Bossart <nathan@postgresql.org>
date : Tue, 17 Dec 2024 15:24:45 -0600
committer: Nathan Bossart <nathan@postgresql.org>
date : Tue, 17 Dec 2024 15:24:45 -0600
If a dshash table grows very large (e.g., the dshash table for
cumulative statistics when there are millions of tables), resizing
it may fail with an error like:
ERROR: invalid DSA memory alloc request size 1073741824
To fix, permit dshash resizing to allocate more than 1 GB by
providing the DSA_ALLOC_HUGE flag.
Reported-by: Andreas Scherbaum
Author: Matthias van de Meent
Reviewed-by: Cédric Villemain, Michael Paquier, Andres Freund
Discussion: https://postgr.es/m/80a12d59-0d5e-4c54-866c-e69cd6536471%40pgug.de
Backpatch-through: 13
M src/backend/lib/dshash.c
Make 009_twophase.pl test pass with recovery_min_apply_delay set
commit : 2af2457fa6d849868a88128bfef9fab5c18c0b50
author : Heikki Linnakangas <heikki.linnakangas@iki.fi>
date : Mon, 16 Dec 2024 15:56:38 +0200
committer: Heikki Linnakangas <heikki.linnakangas@iki.fi>
date : Mon, 16 Dec 2024 15:56:38 +0200
The test failed if you ran the regression tests with TEMP_CONFIG with
recovery_min_apply_delay = '500ms'. Fix the race condition by waiting
for transaction to be applied in the replica, like in a few other
tests.
The failing test was introduced in commit cbfbda7841. Backpatch to all
supported versions like that commit (except v12, which is no longer
supported).
Reported-by: Alexander Lakhin
Discussion: https://www.postgresql.org/message-id/09e2a70a-a6c2-4b5c-aeae-040a7449c9f2@gmail.com
M src/test/recovery/t/009_twophase.pl
pgbench: fix misprocessing of some nested \if constructs.
commit : ff88db91036fed10dedb9e38786506d0460bd7dd
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Sun, 15 Dec 2024 14:14:15 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Sun, 15 Dec 2024 14:14:15 -0500
An \if command appearing within a false (not-to-be-executed) \if
branch was incorrectly treated the same as \elif. This could allow
statements within the inner \if to be executed when they should
not be. Also the missing inner \if stack entry would result in an
assertion failure (in assert-enabled builds) when the final \endif
is reached.
Report and patch by Michail Nikolaev. Back-patch to all
supported branches.
Discussion: https://postgr.es/m/CANtu0oiA1ke=SP6tauhNqkUdv5QFsJtS1p=aOOf_iU+EhyKkjQ@mail.gmail.com
M src/bin/pgbench/pgbench.c
M src/bin/pgbench/t/001_pgbench_with_server.pl
Fix possible crash in pg_dump with identity sequences.
commit : 6978129b4e95aa9d75171ede5ff22f6cf8612055
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Fri, 13 Dec 2024 14:21:36 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Fri, 13 Dec 2024 14:21:36 -0500
If an owned sequence is considered interesting, force its owning
table to be marked interesting too. This ensures, in particular,
that we'll fetch the owning table's column names so we have the
data needed for ALTER TABLE ... ADD GENERATED. Previously there were
edge cases where pg_dump could get SIGSEGV due to not having filled in
the column names. (The known case is where the owning table has been
made part of an extension while its identity sequence is not a member;
but there may be others.)
Also, if it's an identity sequence, force its dumped-components mask
to exactly match the owning table: dump definition only if we're
dumping the table's definition, dump data only if we're dumping the
table's data, etc. This generalizes the code introduced in commit
b965f2617 that set the sequence's dump mask to NONE if the owning
table's mask is NONE. That's insufficient to prevent failures,
because for example the table's mask might only request dumping ACLs,
which would lead us to still emit ALTER TABLE ADD GENERATED even
though we didn't create the table. It seems better to treat an
identity sequence as though it were an inseparable aspect of the
table, matching the treatment used in the backend's dependency logic.
Perhaps this policy needs additional refinement, but let's wait to
see some field use-cases before changing it further.
While here, add a comment in pg_dump.h warning against writing tests
like "if (dobj->dump == DUMP_COMPONENT_NONE)", which was a bug in this
case. There is one other example in getPublicationNamespaces, which
if it's not a bug is at least remarkably unclear and under-documented.
Changing that requires a separate discussion, however.
Per report from Artur Zakirov. Back-patch to all supported branches.
Discussion: https://postgr.es/m/CAKNkYnwXFBf136=u9UqUxFUVagevLQJ=zGd5BsLhCsatDvQsKQ@mail.gmail.com
M src/bin/pg_dump/pg_dump.c
M src/bin/pg_dump/pg_dump.h
Improve reporting of pg_upgrade log files on test failure
commit : 8a8f26dda2255c64b991ce23ae0b2f612fc2b9d4
author : Michael Paquier <michael@paquier.xyz>
date : Wed, 11 Dec 2024 08:48:55 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Wed, 11 Dec 2024 08:48:55 +0900
On failure, the pg_upgrade log files are automatically appended to the
test log file, but the information reported was inconsistent.
A header, with the log file name, was reported with note(), while the
log contents and a footer used print(), making it harder to diagnose
failures when these are split into console output and test log file
because the pg_upgrade log file path in the header may not be included
in the test log file.
The output is now consolidated so as the header uses print() rather than
note(). An extra note() is added to inform that the contents of a
pg_upgrade log file are appended to the test log file.
The diffs from the regression test suite and dump files all use print()
to show their contents on failure.
Author: Joel Jacobson
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/49f7e64a-b9be-4a90-a9fe-210a7740405e@app.fastmail.com
Backpatch-through: 15
M src/bin/pg_upgrade/t/002_pg_upgrade.pl
Fix elog(FATAL) before PostmasterMain() or just after fork().
commit : 839da50bd431587c1182b7d96e82853895961cd1
author : Noah Misch <noah@leadboat.com>
date : Tue, 10 Dec 2024 13:51:59 -0800
committer: Noah Misch <noah@leadboat.com>
date : Tue, 10 Dec 2024 13:51:59 -0800
Since commit 97550c0711972a9856b5db751539bbaf2f88884c, these failed with
"PANIC: proc_exit() called in child process" due to uninitialized or
stale MyProcPid. That was reachable if close() failed in
ClosePostmasterPorts() or setlocale(category, "C") failed, both
unlikely. Back-patch to v13 (all supported versions).
Discussion: https://postgr.es/m/20241208034614.45.nmisch@google.com
M src/backend/main/main.c
M src/backend/postmaster/fork_process.c
M src/backend/postmaster/postmaster.c
Doc: fix incorrect EXPLAIN ANALYZE output for bloom indexes
commit : 5e9e2a4ac7bbdc5a8c8ec06f1b6ac78cfb5fca67
author : David Rowley <drowley@postgresql.org>
date : Tue, 10 Dec 2024 09:26:13 +1300
committer: David Rowley <drowley@postgresql.org>
date : Tue, 10 Dec 2024 09:26:13 +1300
It looks like the example case was once modified to increase the number
of rows but the EXPLAIN ANALYZE output wasn't updated to reflect that.
Also adjust the text which discusses the index sizes. With the example
table size, the bloom index isn't quite 8 times more space efficient
than the btree indexes.
Discussion: https://postgr.es/m/CAApHDvovx8kQ0=HTt85gFDAwmTJHpCgiSvRmQZ_6u_g-vQYM_w@mail.gmail.com
Backpatch-through: 13, all supported versions
M doc/src/sgml/bloom.sgml
Simplify executor's determination of whether to use parallelism.
commit : 4089b9bd6f19362ccb093d7e1d55282a6577acb4
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 9 Dec 2024 14:38:19 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 9 Dec 2024 14:38:19 -0500
Our parallel-mode code only works when we are executing a query
in full, so ExecutePlan must disable parallel mode when it is
asked to do partial execution. The previous logic for this
involved passing down a flag (variously named execute_once or
run_once) from callers of ExecutorRun or PortalRun. This is
overcomplicated, and unsurprisingly some of the callers didn't
get it right, since it requires keeping state that not all of
them have handy; not to mention that the requirements for it were
undocumented. That led to assertion failures in some corner
cases. The only state we really need for this is the existing
QueryDesc.already_executed flag, so let's just put all the
responsibility in ExecutePlan. (It could have been done in
ExecutorRun too, leading to a slightly shorter patch -- but if
there's ever more than one caller of ExecutePlan, it seems better
to have this logic in the subroutine than the callers.)
This makes those ExecutorRun/PortalRun parameters unnecessary.
In master it seems okay to just remove them, returning the
API for those functions to what it was before parallelism.
Such an API break is clearly not okay in stable branches,
but for them we can just leave the parameters in place after
documenting that they do nothing.
Per report from Yugo Nagata, who also reviewed and tested
this patch. Back-patch to all supported branches.
Discussion: https://postgr.es/m/20241206062549.710dc01cf91224809dd6c0e1@sraoss.co.jp
M src/backend/executor/execMain.c
M src/backend/tcop/postgres.c
M src/backend/tcop/pquery.c
M src/include/executor/execdesc.h
M src/include/utils/portal.h
Improve comment about dropped entries in pgstat.c
commit : a0ab20f16541cab2a2094433b3b812b1db2392f1
author : Michael Paquier <michael@paquier.xyz>
date : Mon, 9 Dec 2024 14:35:48 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Mon, 9 Dec 2024 14:35:48 +0900
pgstat_write_statsfile() discards any entries marked as dropped from
being written to the stats file at shutdown, and also included an
assertion based on the same condition.
The intention of the assertion is to track that no pgstats entries
should be left around as terminating backends should drop any entries
they still hold references on before the stats file is written by the
checkpointer, and it not worth taking down the server in this case if
there is a bug making that possible.
Let's improve the comment of this area to document clearly what's
intended.
Based on a discussion with Bertrand Drouvot and Anton A. Melnikov.
Author: Bertrand Drouvot
Discussion: https://postgr.es/m/a13e8cdf-b97a-4ecb-8f42-aaa367974e29@postgrespro.ru
Backpatch-through: 15
M src/backend/utils/activity/pgstat.c
Fix invalidation of local pgstats references for entry reinitialization
commit : 1df1e1e787621cefeb9741b6e9fe74cd38143eee
author : Michael Paquier <michael@paquier.xyz>
date : Mon, 9 Dec 2024 10:46:10 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Mon, 9 Dec 2024 10:46:10 +0900
818119afccd3 has introduced the "generation" concept in pgstats entries,
incremented a counter when a pgstats entry is reinitialized, but it did
not count on the fact that backends still holding local references to
such entries need to be refreshed if the cache age is outdated. The
previous logic only updated local references when an entry was dropped,
but it needs also to consider entries that are reinitialized.
This matters for replication slot stats (as well as custom pgstats kinds
in 18~), where concurrent drops and creates of a slot could cause
incorrect stats to be locally referenced. This would lead to an
assertion failure at shutdown when writing out the stats file, as the
backend holding an outdated local reference would not be able to drop
during its shutdown sequence the stats entry that should be dropped, as
the last process holding a reference to the stats entry. The
checkpointer was then complaining about such an entry late in the
shutdown sequence, after the shutdown checkpoint is finished with the
control file updated, causing the stats file to not be generated. In
non-assert builds, the entry would just be skipped with the stats file
written.
Note that only logical replication slots use statistics.
A test case based on TAP is added to test_decoding, where a persistent
connection peeking at a slot's data is kept with concurrent drops and
creates of the same slot. This is based on the isolation test case that
Anton has sent. As it requires a node shutdown with a check to make
sure that the stats file is written with this specific sequence of
events, TAP is used instead.
Reported-by: Anton A. Melnikov
Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/56bf8ff9-dd8c-47b2-872a-748ede82af99@postgrespro.ru
Backpatch-through: 15
M contrib/test_decoding/t/001_repl_stats.pl
M src/backend/utils/activity/pgstat_shmem.c
Fix possible crash during WindowAgg evaluation
commit : d54378e984d72914abc1dd84898f494ba2168c27
author : David Rowley <drowley@postgresql.org>
date : Mon, 9 Dec 2024 14:24:54 +1300
committer: David Rowley <drowley@postgresql.org>
date : Mon, 9 Dec 2024 14:24:54 +1300
When short-circuiting WindowAgg node evaluation on the top-level
WindowAgg node using quals on monotonic window functions, because the
WindowAgg run condition can mean there's no need to evaluate subsequent
window function results in the same partition once the run condition
becomes false, it was possible that the executor would use stale results
from the previous invocation of the window function in some cases.
A fix for this was partially done by a5832722, but that commit only
fixed the issue for non-top-level WindowAgg nodes. I mistakenly thought
that the top-level WindowAgg didn't have this issue, but Jayesh's example
case clearly shows that's incorrect. At the time, I also thought that
this only affected 32-bit systems as all window functions which then
supported run conditions returned BIGINT, however, that's wrong as
ExecProject is still called and that could cause evaluation of any other
window function belonging to the same WindowAgg node, one of which may
return a byref type.
The only queries affected by this are WindowAggs with a "Run Condition"
which contains at least one window function with a byref result type,
such as lead() or lag() on a byref column. The window clause must also
contain a PARTITION BY clause (without a PARTITION BY, execution of the
WindowAgg stops immediately when the run condition becomes false and
there's no risk of using the stale results).
Reported-by: Jayesh Dehankar
Discussion: https://postgr.es/m/193261e2c4d.3dd3cd7c1842.871636075166132237@zohocorp.com
Backpatch-through: 15, where WindowAgg run conditions were added
M src/backend/executor/nodeWindowAgg.c
M src/test/regress/expected/window.out
M src/test/regress/sql/window.sql
Ensure that pg_amop/amproc entries depend on their lefttype/righttype.
commit : 0e4fa06ba55e99203c4ca96faed4d2e701e87686
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 7 Dec 2024 15:56:28 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 7 Dec 2024 15:56:28 -0500
Usually an entry in pg_amop or pg_amproc does not need a dependency on
its amoplefttype/amoprighttype/amproclefttype/amprocrighttype types,
because there is an indirect dependency via the argument types of its
referenced operator or procedure, or via the opclass it belongs to.
However, for some support procedures in some index AMs, the argument
types of the support procedure might not mention the column data type
at all. Also, the amop/amproc entry might be treated as "loose" in
the opfamily, in which case it lacks a dependency on any particular
opclass; or it might be a cross-type entry having a reference to a
datatype that is not its opclass' opcintype.
The upshot of all this is that there are cases where a datatype can
be dropped while leaving behind amop/amproc entries that mention it,
because there is no path in pg_depend showing that those entries
depend on that type. Such entries are harmless in normal activity,
because they won't get used, but they cause problems for maintenance
actions such as dropping the operator family. They also cause pg_dump
to produce bogus output. The previous commit put a band-aid on the
DROP OPERATOR FAMILY failure, but a real fix is needed.
To fix, add pg_depend entries showing that a pg_amop/pg_amproc entry
depends on its lefttype/righttype. To avoid bloating pg_depend too
much, skip this if the referenced operator or function has that type
as an input type. (I did not bother with considering the possible
indirect dependency via the opclass' opcintype; at least in the
reported case, that wouldn't help anyway.)
Probably, the reason this has escaped notice for so long is that
add-on datatypes and relevant opclasses/opfamilies are usually
packaged as extensions nowadays, so that there's no way to drop
a type without dropping the referencing opclasses/opfamilies too.
Still, in the absence of pg_depend entries there's nothing that
constrains DROP EXTENSION to drop the opfamily entries before the
datatype, so it seems possible for a DROP failure to occur anyway.
The specific case that was reported doesn't fail in v13, because
v13 prefers to attach the support procedure to the opclass not the
opfamily. But it's surely possible to construct other edge cases
that do fail in v13, so patch that too.
Per report from Yoran Heling. Back-patch to all supported branches.
Discussion: https://postgr.es/m/Z1MVCOh1hprjK5Sf@gmai021
M src/backend/commands/opclasscmds.c
M src/include/access/amapi.h
Make getObjectDescription robust against dangling amproc type links.
commit : d401914679dea7e14536a9444bee0742c7de4f2f
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 7 Dec 2024 14:28:16 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 7 Dec 2024 14:28:16 -0500
Yoran Heling reported a case where a data type could be dropped
while references to its OID remain behind in pg_amproc. This
causes getObjectDescription to fail, which blocks dropping the
operator family (since our DROP code likes to construct descriptions
of everything it's dropping). The proper fix for this requires
adding more pg_depend entries. But to allow DROP to go through with
already-corrupt catalogs, tweak getObjectDescription to print "???"
for the type instead of failing when it processes such an entry.
I changed the logic for pg_amop similarly, for consistency,
although it is not known that the problem can manifest in pg_amop.
Per report from Yoran Heling. Back-patch to all supported
branches (although the problem may be unreachable in v13).
Discussion: https://postgr.es/m/Z1MVCOh1hprjK5Sf@gmai021
M src/backend/catalog/objectaddress.c
Fix is_digit labeling of to_timestamp's FFn format codes.
commit : d2f59497a3781a4146b06415177cb7545930beab
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 7 Dec 2024 13:12:32 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Sat, 7 Dec 2024 13:12:32 -0500
These format codes produce or consume strings of digits, so they
should be labeled with is_digit = true, but they were not.
This has effect in only one place, where is_next_separator()
is checked to see if the preceding format code should slurp up
all the available digits. Thus, with a format such as '...SSFF3'
with remaining input '12345', the 'SS' code would consume all
five digits (and then complain about seconds being out of range)
when it should eat only two digits.
Per report from Nick Davies. This bug goes back to d589f9446
where the FFn codes were introduced, so back-patch to v13.
Discussion: https://postgr.es/m/AM8PR08MB6356AC979252CFEA78B56678B6312@AM8PR08MB6356.eurprd08.prod.outlook.com
M src/backend/utils/adt/formatting.c
M src/test/regress/expected/horology.out
M src/test/regress/sql/horology.sql
Avoid low-probability crash on out-of-memory.
commit : 4398507dfafcbb92efd4fab816f3f8a1c0f34761
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Thu, 5 Dec 2024 12:54:41 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Thu, 5 Dec 2024 12:54:41 -0500
check_restrict_nonsystem_relation_kind() correctly uses guc_malloc()
in v16 and later. But in older branches it must use malloc()
directly, and it forgot to check for failure return.
Faulty backpatching of 66e94448a.
Karina Litskevich
Discussion: https://postgr.es/m/CACiT8iZ=atkguKVbpN4HmJFMb4+T9yEowF5JuPZG8W+kkZ9L6w@mail.gmail.com
M src/backend/tcop/postgres.c
RelationTruncate() must set DELAY_CHKPT_START.
commit : a501fe5a971ed06c50f173406c4184b3d32cdf93
author : Thomas Munro <tmunro@postgresql.org>
date : Tue, 3 Dec 2024 09:27:05 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Tue, 3 Dec 2024 09:27:05 +1300
Previously, it set only DELAY_CHKPT_COMPLETE. That was important,
because it meant that if the XLOG_SMGR_TRUNCATE record preceded a
XLOG_CHECKPOINT_ONLINE record in the WAL, then the truncation would also
happen on disk before the XLOG_CHECKPOINT_ONLINE record was
written.
However, it didn't guarantee that the sync request for the truncation
was processed before the XLOG_CHECKPOINT_ONLINE record was written. By
setting DELAY_CHKPT_START, we guarantee that if an XLOG_SMGR_TRUNCATE
record is written to WAL before the redo pointer of a concurrent
checkpoint, the sync request queued by that operation must be processed
by that checkpoint, rather than being left for the following one.
This is a refinement of commit 412ad7a5563. Back-patch to all supported
releases, like that commit.
Author: Robert Haas <robertmhaas@gmail.com>
Reported-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2B-2rjGZC2kwqr2NMLBcEBp4uf59QT1advbWYF_uc%2B0Aw%40mail.gmail.com
M src/backend/catalog/storage.c
Fix broken list-munging in ecpg's remove_variables().
commit : 60b47525c14b1f98927ae61acc033bb6ac80d84b
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Sun, 1 Dec 2024 14:15:37 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Sun, 1 Dec 2024 14:15:37 -0500
The loops over cursor argument variables neglected to ever advance
"prevvar". The code would accidentally do the right thing anyway
when removing the first or second list entry, but if it had to
remove the third or later entry then it would also remove all
entries between there and the first entry. AFAICS this would
only matter for cursors that reference out-of-scope variables,
which is a weird Informix compatibility hack; between that and
the lack of impact for short lists, it's not so surprising that
nobody has complained. Nonetheless it's a pretty obvious bug.
It would have been more obvious if these loops used a more standard
coding style for chasing the linked lists --- this business with the
"prev" pointer sometimes pointing at the current list entry is
confusing and overcomplicated. So rather than just add a minimal
band-aid, I chose to rewrite the loops in the same style we use
elsewhere, where the "prev" pointer is NULL until we are dealing with
a non-first entry and we save the "next" pointer at the top of the
loop. (Two of the four loops touched here are not actually buggy,
but it seems better to make them all look alike.)
Coverity discovered this problem, but not until 2b41de4a5 added code
to free no-longer-needed arguments structs. With that, the incorrect
link updates are possibly touching freed memory, and it complained
about that. Nonetheless the list corruption hazard is ancient, so
back-patch to all supported branches.
M src/interfaces/ecpg/preproc/variable.c
Fix MinGW %d vs %lu warnings in back branches.
commit : 29cd8a41ccb500c475dc4c5bd9b88e1a8156ae7c
author : Thomas Munro <tmunro@postgresql.org>
date : Fri, 29 Nov 2024 10:58:01 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Fri, 29 Nov 2024 10:58:01 +1300
Commit 352f6f2d used %d instead of %lu to format DWORD (unsigned long)
with psprintf(). The _WIN32_WINNT value recently changed for MinGW in
REL_15_STABLE (commit d700e8d7), so the code was suddenly being
compiled, with warnings from gcc.
The warnings were already fixed in 16+ by commits 495ed0ef and a9bc04b2
after the _WIN32_WINNT value was increase there. 14 and 13 didn't warn
because they still use a lower value for MinGW, and supported versions
of Visual Studio should compile the code in all live branches but don't
check our format string.
The change doesn't affect the result: sizeof(int) == sizeof(long) on
this platform, and the values are computed with expressions that cannot
exceed INT_MAX so were never interpreted as negative.
Back-patch the formatting change from those commits into 13-15. This
should turn CI's 15 branch green again and stop fairywren from warning
about that on 15.
Reported-by: Andres Freund <andres@anarazel.de>
Reported-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/t2vjrcb3bloxf5qqvxjst6r7lvrefqyecxgt2koy5ho5b5glr2%40yuupmm6whgob
M src/backend/utils/adt/pg_locale.c
Skip SectionMemoryManager.h in cpluspluscheck.
commit : b792aa989bb80ae924867e311d456c9875411828
author : Thomas Munro <tmunro@postgresql.org>
date : Thu, 28 Nov 2024 15:32:57 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Thu, 28 Nov 2024 15:32:57 +1300
Commit 9044fc1d45a0 skipped SectionMemoryManager.h in headerscheck, and
by extension also cpluspluscheck, because it's C++ and would fail both
tests. That worked in master and REL_17_STABLE due to 7b8e2ae2fd3b, but
older branches have a separate cpluspluscheck script. We need to copy
the filtering rule into there too.
This problem was being reported by CI's CompilerWarnings task in the 15
and 16 branches, but was a victim of alert fatigue syndrome (unrelated
problems in the back-branches).
Fix 16, and back-patch to 13, as those are the live branches that have a
separate cpluspluscheck script.
M src/tools/pginclude/cpluspluscheck
Revert "Handle better implicit transaction state of pipeline mode"
commit : 6d1183515e4fca753eea69b0349766e81248bdb4
author : Michael Paquier <michael@paquier.xyz>
date : Thu, 28 Nov 2024 09:43:24 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Thu, 28 Nov 2024 09:43:24 +0900
This reverts commit d77f91214fb7 on all stable branches, due to concerns
regarding the compatility side effects this could create in a minor
release. The change still exists on HEAD.
Discussion: https://postgr.es/m/CA+TgmoZqRgeFTg4+Yf_CMRRXiHuNz1u6ZC4FvVk+rxw0RmOPnw@mail.gmail.com
Backpatch-through: 13
M doc/src/sgml/protocol.sgml
M src/backend/access/transam/xact.c
M src/backend/tcop/postgres.c
M src/bin/pgbench/t/001_pgbench_with_server.pl
ci: Fix cached MacPorts installation management
commit : 3bbfa2ef785a36c6aa48d9a7780274dcbc6d63fd
author : Andres Freund <andres@anarazel.de>
date : Wed, 27 Nov 2024 11:30:19 -0500
committer: Andres Freund <andres@anarazel.de>
date : Wed, 27 Nov 2024 11:30:19 -0500
1. The error reporting of "port setrequested list-of-packages..."
changed, hiding errors we were relying on to know if all packages in our
list were already installed. Use a loop instead.
2. The cached MacPorts installation was shared between PostgreSQL
major branches, though each branch wanted different packages. Add the
list of packages to cache key, so that different branches, when tested
in one github account/repo such as postgres/postgres, stop fighting with
each other, adding and removing packages.
Back-patch to 15 where CI began.
Author: Thomas Munro <thomas.munro@gmail.com>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/au2uqfuy2nf43nwy2txmc5t2emhwij7kzupygto3d2ffgtrdgr%40ckvrlwyflnh2
M .cirrus.tasks.yml
M src/tools/ci/ci_macports_packages.sh
pgbench: Ensure previous progress message is fully cleared when updating.
commit : 1e46f7351abcc8f17d37b0d7498fb672a105eb26
author : Fujii Masao <fujii@postgresql.org>
date : Wed, 27 Nov 2024 23:03:44 +0900
committer: Fujii Masao <fujii@postgresql.org>
date : Wed, 27 Nov 2024 23:03:44 +0900
During pgbench's table initialization, progress updates could display
leftover characters from the previous message if the new message
was shorter. This commit resolves the issue by appending spaces to
the current message to fully overwrite any remaining characters from
the previous line.
Back-patch to all the supported versions.
Author: Yushi Ogiwara, Tatsuo Ishii, Fujii Masao
Reviewed-by: Tatsuo Ishii, Fujii Masao
Discussion: https://postgr.es/m/9a9b8b95b6a709877ae48ad5b0c59bb9@oss.nttdata.com
M src/bin/pgbench/pgbench.c
Fix typo
commit : f2353d03cd0d81ae1829362dcbde85467ea828fd
author : Peter Eisentraut <peter@eisentraut.org>
date : Wed, 27 Nov 2024 11:12:09 +0100
committer: Peter Eisentraut <peter@eisentraut.org>
date : Wed, 27 Nov 2024 11:12:09 +0100
from commit 9044fc1d45a
M src/tools/pgindent/exclude_file_patterns
Exclude LLVM files from whitespace checks
commit : 26b74b3cad7b585626569dd536f2c1fafac20a3e
author : Peter Eisentraut <peter@eisentraut.org>
date : Wed, 27 Nov 2024 11:08:12 +0100
committer: Peter Eisentraut <peter@eisentraut.org>
date : Wed, 27 Nov 2024 11:08:12 +0100
Commit 9044fc1d45a added some files from upstream LLVM. These files
have different whitespace rules, which make the git whitespace checks
powered by gitattributes fail. To fix, add those files to the exclude
list.
M .gitattributes
If a C23 compiler is detected, try asking for C17.
commit : f00c401c65afa2cf18f0d67d13eeb382df84c37b
author : Thomas Munro <tmunro@postgresql.org>
date : Wed, 27 Nov 2024 15:43:18 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Wed, 27 Nov 2024 15:43:18 +1300
Branches before 16 can't be compiled with a C23 compiler (see
deprecation warnings silenced by commit f9a56e72, and non-back-patchable
changes made in 16 by commit 1c27d16e). Test __STDC_VERSION__, and if
it's above C17 then try appending -std=gnu17. The test is done with the
user's CFLAGS, so an acceptable language version can also be configured
manually that way.
This is done in branches 15 and older, back to 9.2, per policy of
keeping them buildable with modern tools.
Discussion: https://postgr.es/m/87o72eo9iu.fsf%40gentoo.org
M configure
M configure.ac
Handle better implicit transaction state of pipeline mode
commit : 88a16b3dbb16dc127894ad56c57c53d309ae816a
author : Michael Paquier <michael@paquier.xyz>
date : Wed, 27 Nov 2024 09:31:40 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Wed, 27 Nov 2024 09:31:40 +0900
When using a pipeline, a transaction starts from the first command and
is committed with a Sync message or when the pipeline ends.
Functions like IsInTransactionBlock() or PreventInTransactionBlock()
were already able to understand a pipeline as being in a transaction
block, but it was not the case of CheckTransactionBlock(). This
function is called for example to generate a WARNING for SET LOCAL,
complaining that it is used outside of a transaction block.
The current state of the code caused multiple problems, like:
- SET LOCAL executed at any stage of a pipeline issued a WARNING, even
if the command was at least second in line where the pipeline is in a
transaction state.
- LOCK TABLE failed when invoked at any step of a pipeline, even if it
should be able to work within a transaction block.
The pipeline protocol assumes that the first command of a pipeline is
not part of a transaction block, and that any follow-up commands is
considered as within a transaction block.
This commit changes the backend so as an implicit transaction block is
started each time the first Execute message of a pipeline has finished
processing, with this implicit transaction block ended once a sync is
processed. The checks based on XACT_FLAGS_PIPELINING in the routines
checking if we are in a transaction block are not necessary: it is
enough to rely on the existing ones.
Some tests are added to pgbench, that can be backpatched down to v17
when \syncpipeline is involved and down to v14 where \startpipeline and
\endpipeline are available. This is unfortunately limited regarding the
error patterns that can be checked, but it provides coverage for various
pipeline combinations to check if these succeed or fail. These tests
are able to capture the case of SET LOCAL's WARNING. The author has
proposed a different feature to improve the coverage by adding similar
meta-commands to psql where error messages could be checked, something
more useful for the cases where commands cannot be used in transaction
blocks, like REINDEX CONCURRENTLY or VACUUM. This is considered as
future work for v18~.
Author: Anthonin Bonnefoy
Reviewed-by: Jelte Fennema-Nio, Michael Paquier
Discussion: https://postgr.es/m/CAO6_XqrWO8uNBQrSu5r6jh+vTGi5Oiyk4y8yXDORdE2jbzw8xw@mail.gmail.com
Backpatch-through: 13
M doc/src/sgml/protocol.sgml
M src/backend/access/transam/xact.c
M src/backend/tcop/postgres.c
M src/bin/pgbench/t/001_pgbench_with_server.pl
Fix NULLIF()'s handling of read-write expanded objects.
commit : 80cd33bad172c6d625f04d3b4f74f8f00c2a58de
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 25 Nov 2024 18:08:58 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 25 Nov 2024 18:08:58 -0500
If passed a read-write expanded object pointer, the EEOP_NULLIF
code would hand that same pointer to the equality function
and then (unless equality was reported) also return the same
pointer as its value. This is no good, because a function that
receives a read-write expanded object pointer is fully entitled
to scribble on or even delete the object, thus corrupting the
NULLIF output. (This problem is likely unobservable with the
equality functions provided in core Postgres, but it's easy to
demonstrate with one coded in plpgsql.)
To fix, make sure the pointer passed to the equality function
is read-only. We can still return the original read-write
pointer as the NULLIF result, allowing optimization of later
operations.
Per bug #18722 from Alexander Lakhin. This has been wrong
since we invented expanded objects, so back-patch to all
supported branches.
Discussion: https://postgr.es/m/18722-fd9e645448cc78b4@postgresql.org
M src/backend/executor/execExpr.c
M src/backend/executor/execExprInterp.c
M src/backend/jit/llvm/llvmjit_expr.c
M src/include/executor/execExpr.h
M src/test/regress/expected/case.out
M src/test/regress/sql/case.sql
Avoid "you don't own a lock of type ExclusiveLock" in GRANT TABLESPACE.
commit : 941e0c0df2bcdeb63bd822a1c46818fa9d7d8599
author : Noah Misch <noah@leadboat.com>
date : Mon, 25 Nov 2024 14:42:35 -0800
committer: Noah Misch <noah@leadboat.com>
date : Mon, 25 Nov 2024 14:42:35 -0800
This WARNING appeared because SearchSysCacheLocked1() read
cc_relisshared before catcache initialization, when the field is false
unconditionally. On the basis of reading false there, it constructed a
locktag as though pg_tablespace weren't relisshared. Only shared
catalogs could be affected, and only GRANT TABLESPACE was affected in
practice. SearchSysCacheLocked1() callers use one other shared-relation
syscache, DATABASEOID. DATABASEOID is initialized by the end of
CheckMyDatabase(), making the problem unreachable for pg_database.
Back-patch to v13 (all supported versions). This has no known impact
before v16, where ExecGrant_common() first appeared. Earlier branches
avoid trouble by having a separate ExecGrant_Tablespace() that doesn't
use LOCKTAG_TUPLE. However, leaving this unfixed in v15 could ensnare a
future back-patch of a SearchSysCacheLocked1() call.
Reported by Aya Iwata.
Discussion: https://postgr.es/m/OS7PR01MB11964507B5548245A7EE54E70EA212@OS7PR01MB11964.jpnprd01.prod.outlook.com
M src/backend/utils/cache/syscache.c
M src/test/regress/expected/tablespace.out
M src/test/regress/sql/tablespace.sql
Update configure probes for CFLAGS needed for ARM CRC instructions.
commit : 851c6ff18fd359bebd97b7f2b73bb46981a1a8d2
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 25 Nov 2024 12:50:17 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Mon, 25 Nov 2024 12:50:17 -0500
On ARM platforms where the baseline CPU target lacks CRC instructions,
we need to supply a -march flag to persuade the compiler to compile
such instructions. It turns out that our existing choice of
"-march=armv8-a+crc" has not worked for some time, because recent gcc
will interpret that as selecting software floating point, and then
will spit up if the platform requires hard-float ABI, as most do
nowadays. The end result was to silently fall back to software CRC,
which isn't very desirable since in practice almost all currently
produced ARM chips do have hardware CRC.
We can fix this by using "-march=armv8-a+crc+simd" to enable the
correct ABI choice. (This has no impact on the code actually
generated, since neither of the files we compile with this flag
does any floating-point stuff, let alone SIMD.) Keep the test for
"-march=armv8-a+crc" since that's required for soft-float ABI,
but try that second since most platforms we're likely to build on
use hard-float.
Since this isn't working as-intended on the last several years'
worth of gcc releases, back-patch to all supported branches.
Discussion: https://postgr.es/m/4496616.iHFcN1HehY@portable-bastien
M configure
M configure.ac
Add support for Tcl 9
commit : a5f9cbde99c50aec7aa9fbafdb5b2b492ffdab0d
author : Peter Eisentraut <peter@eisentraut.org>
date : Mon, 25 Nov 2024 08:03:16 +0100
committer: Peter Eisentraut <peter@eisentraut.org>
date : Mon, 25 Nov 2024 08:03:16 +0100
Tcl 9 changed several API functions to take Tcl_Size, which is
ptrdiff_t, instead of int, for 64-bit enablement. We have to change a
few local variables to be compatible with that. We also provide a
fallback typedef of Tcl_Size for older Tcl versions.
The affected variables are used for quantities that will not approach
values beyond the range of int, so this doesn't change any
functionality.
Reviewed-by: Tristan Partin <tristan@partin.io>
Discussion: https://www.postgresql.org/message-id/flat/bce0fe54-75b4-438e-b42b-8e84bc7c0e9c%40eisentraut.org
M src/pl/tcl/pltcl.c
Assume that <stdbool.h> conforms to the C standard.
commit : 94817d9d94d06e5edae408a919de907d2df4cee8
author : Thomas Munro <tmunro@postgresql.org>
date : Mon, 25 Nov 2024 13:11:28 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Mon, 25 Nov 2024 13:11:28 +1300
Previously we checked "for <stdbool.h> that conforms to C99" using
autoconf's AC_HEADER_STDBOOL macro. We've required C99 since PostgreSQL
12, so the test was redundant, and under C23 it was broken: autoconf
2.69's implementation doesn't understand C23's new empty header (the
macros it's looking for went away, replaced by language keywords).
Later autoconf versions fixed that, but let's just remove the
anachronistic test.
HAVE_STDBOOL_H and HAVE__BOOL will no longer be defined, but they
weren't directly tested in core or likely extensions (except in 11, see
below). PG_USE_STDBOOL (or USE_STDBOOL in 11 and 12) is still defined
when sizeof(bool) is 1, which should be true on all modern systems.
Otherwise we define our own bool type and values of size 1, which would
fail to compile under C23 as revealed by the broken test. (We'll
probably clean that dead code up in master, but here we want a minimal
back-patchable change.)
This came to our attention when GCC 15 recently started using using C23
by default and failed to compile the replacement code, as reported by
Sam James and build farm animal alligator.
Back-patch to all supported releases, and then two older versions that
also know about <stdbool.h>, per the recently-out-of-support policy[1].
12 requires C99 so it's much like the supported releases, but 11 only
assumes C89 so it now uses AC_CHECK_HEADERS instead of the overly picky
AC_HEADER_STDBOOL. (I could find no discussion of which historical
systems had <stdbool.h> but failed the conformance test; if they ever
existed, they surely aren't relevant to that policy's goals.)
[1] https://wiki.postgresql.org/wiki/Committing_checklist#Policies
Reported-by: Sam James <sam@gentoo.org>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org> (master version)
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (approach)
Discussion: https://www.postgresql.org/message-id/flat/87o72eo9iu.fsf%40gentoo.org
M configure
M configure.ac
M src/include/c.h
M src/include/pg_config.h.in
M src/tools/msvc/Solution.pm
doc: Fix example with __next__() in PL/Python function
commit : ef2f4727b3fabf1b4463def531a7d99264bc321b
author : Michael Paquier <michael@paquier.xyz>
date : Mon, 25 Nov 2024 09:15:45 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Mon, 25 Nov 2024 09:15:45 +0900
Per PEP 3114, iterator.next() has been renamed to iterator.__next__(),
and one example in the documentation still used next(). This caused the
example provided to fail the function creation since Python 2 is not
supported anymore since 19252e8ec93.
Author: Erik Wienhold
Discussion: https://postgr.es/m/173209043143.2092749.13692266486972491694@wrigleys.postgresql.org
Backpatch-through: 15
M doc/src/sgml/plpython.sgml
jit: Use -mno-outline-atomics for bitcode on ARM.
commit : 15ab513fed049984fc5f1400b3ceb3be4ae7c987
author : Thomas Munro <tmunro@postgresql.org>
date : Fri, 22 Nov 2024 14:53:21 +1300
committer: Thomas Munro <tmunro@postgresql.org>
date : Fri, 22 Nov 2024 14:53:21 +1300
If the executable's .o files were produced by a compiler (probably gcc)
not using -moutline-atomics, and the corresponding .bc files were
produced by clang using -moutline-atomics (probably by default), then
the generated bitcode functions would have the target attribute
"+outline-atomics", and could fail at runtime when inlined. If the
target ISA at bitcode generation time was armv8-a (the most conservative
aarch64 target, no LSE), then LLVM IR atomic instructions would generate
calls to functions in libgcc.a or libclang_rt.*.a that switch between
LL/SC and faster LSE instructions depending on a runtime AT_HWCAP check.
Since the corresponding .o files didn't need those functions, they
wouldn't have been included in the executable, and resolution would
fail.
At least Debian and Ubuntu are known to ship gcc and clang compilers
that target armv8-a but differ on the use of outline atomics by default.
Fix, by suppressing the outline atomics attribute in bitcode explicitly.
Inline LL/SC instructions will be generated for atomic operations in
bitcode built for armv8-a. Only configure scripts are adjusted for now,
because the meson build system doesn't generate bitcode yet.
This doesn't seem to be a new phenomenon, so real cases of functions
using atomics that are inlined by JIT must be rare in the wild given how
long it took for a bug report to arrive. The reported case could be
reduced to:
postgres=# set jit_inline_above_cost = 0;
SET
postgres=# set jit_above_cost = 0;
SET
postgres=# select pg_last_wal_receive_lsn();
WARNING: failed to resolve name __aarch64_swp4_acq_rel
FATAL: fatal llvm error: Program used external function
'__aarch64_swp4_acq_rel' which could not be resolved!
The change doesn't affect non-ARM systems or later target ISAs.
Back-patch to all supported releases.
Reported-by: Alexander Kozhemyakin <a.kozhemyakin@postgrespro.ru>
Discussion: https://postgr.es/m/18610-37bf303f904fede3%40postgresql.org
M configure
M configure.ac
Fix newly introduced 010_keep_recycled_wals.pl
commit : 93d8d4fe1255fa129d3447054a6b3e492de6788a
author : Álvaro Herrera <alvherre@alvh.no-ip.org>
date : Thu, 21 Nov 2024 17:04:26 +0100
committer: Álvaro Herrera <alvherre@alvh.no-ip.org>
date : Thu, 21 Nov 2024 17:04:26 +0100
It failed to set the archive_command as it desired because of a syntax
problem. Oversight in commit 90bcc7c2db1d.
This bug doesn't cause the test to fail, because the test only checks
pg_rewind's output messages, not the actual outcome (and the outcome in
both cases is that the file is kept, not deleted). But in either case
the message about the file being kept is there, so it's hard to get
excited about doing much more.
Reported-by: Antonin Houska <ah@cybertec.at>
Author: Alexander Kukushkin <cyberdemn@gmail.com>
Discussion: https://postgr.es/m/7822.1732167825@antos
M src/bin/pg_rewind/t/010_keep_recycled_wals.pl
Fix outdated bit in README.tuplock
commit : c0ff8261e2b9ba2f78c2b7ed4128f18a260751e0
author : Álvaro Herrera <alvherre@alvh.no-ip.org>
date : Thu, 21 Nov 2024 16:54:36 +0100
committer: Álvaro Herrera <alvherre@alvh.no-ip.org>
date : Thu, 21 Nov 2024 16:54:36 +0100
Apparently this information has been outdated since first committed,
because we adopted a different implementation during development per
reviews and this detail was not updated in the README.
This has been wrong since commit 0ac5ad5134f2 introduced the file in
2013. Backpatch to all live branches.
Reported-by: Will Mortensen <will@extrahop.com>
Discussion: https://postgr.es/m/CAMpnoC6yEQ=c0Rdq-J7uRedrP7Zo9UMp6VZyP23QMT68n06cvA@mail.gmail.com
M src/backend/access/heap/README.tuplock
Fix memory leak in pgoutput for the WAL sender
commit : 6fc30c24cb7f648a4997c7508333bbe7f5a7de9c
author : Michael Paquier <michael@paquier.xyz>
date : Thu, 21 Nov 2024 15:14:15 +0900
committer: Michael Paquier <michael@paquier.xyz>
date : Thu, 21 Nov 2024 15:14:15 +0900
RelationSyncCache, the hash table in charge of tracking the relation
schemas sent through pgoutput, was forgetting to free the TupleDesc
associated to the two slots used to store the new and old tuples,
causing some memory to be leaked each time a relation is invalidated
when the slots of an existing relation entry are cleaned up.
This is rather hard to notice as the bloat is pretty minimal, but a
long-running WAL sender would be in trouble over time depending on the
workload. sysbench has proved to be pretty good at showing the problem,
coupled with some memory monitoring of the WAL sender.
Issue introduced in 52e4f0cd472d, that has added row filters for tables
logically replicated.
Author: Boyu Yang
Reviewed-by: Michael Paquier, Hou Zhijie
Discussion: https://postgr.es/m/DM3PR84MB3442E14B340E553313B5C816E3252@DM3PR84MB3442.NAMPRD84.PROD.OUTLOOK.COM
Backpatch-through: 15
M src/backend/replication/pgoutput/pgoutput.c
Avoid assertion failure if a setop leaf query contains setops.
commit : 9b9689e26cb66ded459e9453a74d6de057eead4c
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Wed, 20 Nov 2024 12:03:47 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Wed, 20 Nov 2024 12:03:47 -0500
Ordinarily transformSetOperationTree will collect all UNION/
INTERSECT/EXCEPT steps into the setOperations tree of the topmost
Query, so that leaf queries do not contain any setOperations.
However, it cannot thus flatten a subquery that also contains
WITH, ORDER BY, FOR UPDATE, or LIMIT. I (tgl) forgot that in
commit 07b4c48b6 and wrote an assertion in rule deparsing that
a leaf's setOperations would always be empty.
If it were nonempty then we would want to parenthesize the subquery
to ensure that the output represents the setop nesting correctly
(e.g. UNION below INTERSECT had better get parenthesized). So
rather than just removing the faulty Assert, let's change it into
an additional case to check to decide whether to add parens. We
don't expect that the additional case will ever fire, but it's
cheap insurance.
Man Zeng and Tom Lane
Discussion: https://postgr.es/m/tencent_7ABF9B1F23B0C77606FC5FE3@qq.com
M src/backend/utils/adt/ruleutils.c
Compare collations before merging UNION operations.
commit : fd3383ff1107dd2d530178dd980423d96b3ade27
author : Tom Lane <tgl@sss.pgh.pa.us>
date : Tue, 19 Nov 2024 18:26:19 -0500
committer: Tom Lane <tgl@sss.pgh.pa.us>
date : Tue, 19 Nov 2024 18:26:19 -0500
In the dim past we figured it was okay to ignore collations
when combining UNION set-operation nodes into a single N-way
UNION operation. I believe that was fine at the time, but
it stopped being fine when we added nondeterministic collations:
the semantics of distinct-ness are affected by those. v17 made
it even less fine by allowing per-child sorting operations to
be merged via MergeAppend, although I think we accidentally
avoided any live bug from that.
Add a check that collations match before deciding that two
UNION nodes are equivalent. I also failed to resist the
temptation to comment plan_union_children() a little better.
Back-patch to all supported branches (v13 now), since they
all have nondeterministic collations.
Discussion: https://postgr.es/m/3605568.1731970579@sss.pgh.pa.us
M src/backend/optimizer/prep/prepunion.c