During promotion, there is a window where RecoveryInProgress() returns
true but the WAL segments of the old timeline have already been removed.
A logical decoding could pick up the old timeline in this window when
reading a page, failing with the following error:
ERROR: requested WAL segment ... has already been removed
This issue does not lead to any data correctness issue, as retrying to
decode the data works in follow-up decoding attempts. It impacts
availability, though. Other WAL page read callbacks have a similar
issue, this commit takes care of what should be the noisiest code path:
logical decoding with START_REPLICATION in a WAL sender.
A TAP test, based on an injection point waiting in the startup process
after the segments have been removed/recycled, is added. This part is
backpatched down to v17.
This issue has been causing sporadic failures in the buildfarm, and
was reproducible manually. This issue happens since logical decoding on
standbys exists, down to v16.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/7daef094-abf3-4672-bc23-3df4763b16a3@gmail.com
Backpatch-through: 16
Commit 85c17f6 mistakenly declared a variable storing catalog_xmin as
XLogRecPtr, even though catalog_xmin is a TransactionId.
This caused no functional issue, but the type was clearly incorrect.
Therefore, this commit fixes it to use the correct type TransactionId
instead, and backpatch to v17 where the issue was introduced.
Author: Imran Zaheer <imran.zhir@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CA+UBfa=mNeLt-4BFjEP4tqdDsnq+oMqqPr7fd9Wji2_9YXmQdA@mail.gmail.com
Strings built by this function are not supposed to be subject to
NLS translation, but commit 6566133c5 missed that memo, so that
object identities like "membership of role %s in role %s" were
translated.
Previously, outlen was miscalculated if case_sensitive was false and
str_tolower() changed the byte length of the string. If outlen was too
large, pnstrdup() would stop at the NUL terminator, preventing
overrun. But if outlen was too small, it would cause truncation.
Fix by just removing outlen. It was only used in a single site, which
could just as well use pstrdup().
Discussion: https://postgre.es/m/1101e1a3afbbabb503317069c40374b82e6f4cac.camel@j-davis.com
Reviewed-by: Tristan Partin <tristan@partin.io>
Backpatch-through: 14
The operators for array_eq, record_eq, range_eq, and multirange_eq
are all marked oprcanhash, but there's a pitfall: their hash functions
can fail at runtime if the contained type(s) are not hashable.
Therefore, the planner has to check hashability of the contained types
before deciding it can use hashing in these cases. Not every place
had gotten this memo, and noplace at all had considered the issue
for ranges or multiranges. In particular we could attempt to use
hashing for a ScalarArrayOpExpr on a container type when it won't
actually work, leading to "could not identify a hash function ..."
runtime failures.
For the most part we should fix this in the lookup functions provided
by lsyscache.c, to wit get_op_hash_functions and op_hashjoinable.
But there's a problem: get_op_hash_functions is not passed the input
data type it would need to check. We mustn't change the API of that
exported function in a back-patched fix, and even if we wanted to,
its call sites in the executor mostly don't have easy access to the
required data type OID. Fortunately, the executor call sites don't
actually need fixing, because it's expected that the planner verified
hashability before building a plan that requires it. Therefore,
leave get_op_hash_functions as-is and invent a wrapper function
get_op_hash_functions_ext that does the additional checking needed
in the planner's uses.
We also need to fix hash_ok_operator (extending the fix in 647889667).
While at it, neaten up a couple of places in lookup_type_cache where
relevant code for multirange cases was written differently from the
code for other container types.
Note: while this touches pg_operator.dat, it's only to add oid_symbol
macros. So there's no on-disk data change and no need for a
catversion bump.
Reported-by: Andrei Lepikhov <lepihov@gmail.com>
Author: Andrei Lepikhov <lepihov@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/ed221f95-f09b-4a9c-b05b-e1fed621ec87@gmail.com
Backpatch-through: 14
Previously, ecpg accepted multiple descriptor header items in GET DESCRIPTOR
and SET DESCRIPTOR, but generated broken C code when they were used.
Although the grammar allowed this syntax, the implementation did not actually
support it.
This commit tightens the ecpg grammar so the header form of GET/SET DESCRIPTOR
accepts only a single header item, matching the implementation and preventing
generation of broken C code.
Also update the documentation synopsis accordingly.
Backpatch to all supported versions.
Author: Masashi Kamura <kamura.masashi@fujitsu.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Lakshmi G <lakshmigcdac@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/OS9PR01MB13174AD7D1829D0644B6BB90E9447A@OS9PR01MB13174.jpnprd01.prod.outlook.com
Backpatch-through: 14
When called from a parallel worker, this function calls initStringInfo()
and pq_beginmessage(), causing a StringInfo allocation to happen twice.
pq_endmessage() frees only the second allocation, with each call leaking
~1 kB into the per-worker memory context. This could cause a few
hundred megabytes worth of memory to pile up until the worker exits (the
message allocations happen in the parallel worker context), with the
situation being worse the longer a parallel worker runs.
Oversight in f1889729dd.
Author: Baji Shaik <baji.pgdev@gmail.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Tristan Partin <tristan@partin.io>
Discussion: https://postgr.es/m/CA+fm-RMopta1Dmq8udiU5sp+zwTvhUf4+xfbr3rZDfczH+p-xw@mail.gmail.com
Backpatch-through: 17
When a table's columns are narrower than the record header line, the
expanded aligned format produced misaligned output because the data
column width was not adjusted to match the record header width, leading
to output like:
+-[ RECORD 1 ]-+
| a | 10 |
| b | 20 |
+---+----+
This commit adjusts the output so as the column width match with the
header line, giving:
+-[ RECORD 1 ]-+
| a | 10 |
| b | 20 |
+---+----------+
Author: Pavel Stehule <pavel.stehule@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CAFj8pRCzGpsr9zTHbtTd4mGh2YPJqOEgLgt8JLiopuYA9_1xGw@mail.gmail.com
Backpatch-through: 14
Before v17, WAL senders can shut down after the checkpointer. If a WAL
sender still has pending statistics when the checkpointer has already
exited, its shutdown callback may attempt to report those statistics and
trigger assertions in pgstats. In that case, the pending statistics are
lost.
This commit adjusts the assertion handling so that attempts to report
pending WAL sender statistics after the checkpointer has completed its
final stats flush are skipped.
Preserving the existing assertion would require backpatching an
equivalent of 87a6690cc6, ensuring that the checkpointer is always the
last process to exit. Such a change would be considerably more invasive
and risky for stable branches because it alters the shutdown sequence,
and the consequence is only some loss of stats data for the WAL sender.
This assertion failure was periodically detected in the buildfarm,
leading to spurious failures.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/18158-88f667028dbc7e7b@postgresql.org
Backpatch-through: 15-17
Presently, refint stores plans in a per-backend cache to avoid
re-preparing in each call. This has a few problems. For one,
check_foreign_key() embeds the new key values in its cascade-UPDATE
queries, so a cached plan reuses the values from preparation.
Also, the cache is never invalidated, so it can return stale
entries that cause other problems. There may very well be more
bugs lurking.
We could spend a lot of time trying to address all these problems,
but this module is primarily intended as sample code, and by all
indications, it sees minimal use. Furthermore, there is a growing
consensus for removing refint in v20. However, since we'll need to
support it on the back-branches for a while longer, it probably
still makes sense to fix some of the more egregious bugs.
Therefore, let's just remove refint's plan cache entirely. That
means we'll re-prepare on every call, but that seems quite unlikely
to bother anyone. On v17 and older versions, the regression test
for triggers fails after this change, so I've borrowed pieces of
commit 8cfbdf8f4d to fix it.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAJTYsWXU%2BfhuzrEd_bnrxyGH3%2Bny8QRQC2QHf3ws6s9iki3c2Q%40mail.gmail.com
Backpatch-through: 14
The NFC recomposition incorrectly included TBASE as a valid T syllable,
which is incorrect based on the Unicode specification (TBASE is one
below the start of the range, range beginning at U+11A8).
This would cause the TBASE to be silently swallowed in the
normalization, leading to an incorrect result.
A couple of regression tests are added to check more patterns with
Hangul recomposition and decomposition, on top of a test to check the
problem with TBASE. Diego has submitted the code fix, and I have
written the tests.
Author: Diego Frias <mail@dzfrias.dev>
Co-authored-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/B92ED640-7D4A-4505-B09F-3548F58CBB16@dzfrias.dev
Backpatch-through: 14
This commit addresses two related issues:
tsvector_filter() assumed it could print an incorrect weight value
with %c. This could result in an invalidly-encoded error message
if the database encoding is multibyte and the char value has its
high bit set. Weight values that are ASCII control characters
could render illegibly too. Fix by printing such values in octal
(\ooo), similarly to how charout() would render them.
tsvector_setweight() and tsvector_setweight_by_filter() reported
the same unrecognized-weight error condition with elog(), as though
it were an internal error. That'd not translate, would produce an
unwanted XX000 SQLSTATE code, and also reported the bad value as a
decimal integer which seems unhelpful. Fix by refactoring so that
all three functions share one copy of the code that interprets a
weight argument.
The invalid-encoding aspect seems to me (tgl) to justify
back-patching.
Author: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAON2xHNaeLAUzRCXL5AmXLcXaSE_gWAVjWQRmLzc_oZ=1_Vf4Q@mail.gmail.com
Backpatch-through: 14
Like 8f1791c61, this fixes a case of implicitly casting away
const by not treating the result of strrchr() on a const pointer
as const. This was missed at the time because the machines
reporting those warnings weren't building with --with-llvm.
While here, clean up another infelicity: in the probably-
impossible case that the input string contains only one dot,
this function would call pnstrdup() with a length of -1
and thereby emit a module name equal to the function name.
It seems to me we should emit modname = NULL instead.
Also remove a useless Assert and two redundant assignments.
Back-patch, as 8f1791c61 was, so that users of back branches
don't see this warning when building with late-model gcc.
Reported-by: hubert depesz lubaczewski <depesz@depesz.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/aiGNJ89PBqvq2Yyz@depesz.com
Backpatch-through: 14
When releasing an ephemeral replication slot, ReplicationSlotRelease()
drops the slot via ReplicationSlotDropAcquired().
However, after dropping the slot, ReplicationSlotRelease() continued
to use its local "slot" pointer, which still referenced the dropped
slot's former shared-memory entry. It could then update fields such as
effective_xmin in that entry.
Once an ephemeral slot has been dropped (via ReplicationSlotDropAcquired()),
its slot array entry can be reused immediately by another backend
creating a new slot. As a result, those updates could corrupt
the state of an unrelated replication slot.
Fix by skipping those shared-memory updates for phemeral slots and
performing them only for non-ephemeral slots, whose shared-memory
entries remain valid after release.
Backpatch to all supported versions.
Author: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Masao Fujii <masao.fujii@gmail.com>
Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/TY4PR01MB177184FF9EE916F577E1F554194082@TY4PR01MB17718.jpnprd01.prod.outlook.com
Backpatch-through: 14
The code failed to initialize the second isnull argument passed to
FunctionCallInvoke(). This is harmless for existing in-core extended
hash support functions, since FunctionCallInvoke() does not use the
value (note that all the in-core extended hash functions are strict),
examining only the argument values. However, extension-provided
extended hash functions could be affected if they inspect
PG_ARGISNULL(1).
Oversight in 01e658fa74.
Author: Man Zeng <zengman@halodbtech.com>
Discussion: https://postgr.es/m/tencent_7818173C01E01836109848C3@qq.com
Backpatch-through: 14
The check for window functions (point 4) guarded on the wrong bit: it
tested UNSAFE_NOTIN_DISTINCTON_CLAUSE while setting
UNSAFE_NOTIN_PARTITIONBY_CLAUSE. Each check in this loop guards on
the same bit it is about to set, as an idempotency optimization, since
unsafeFlags[] is accumulated across the arms of a set operation and
there is no point recomputing a column's status once its bit is
present.
This is not a live bug. When UNSAFE_NOTIN_PARTITIONBY_CLAUSE is
already set but UNSAFE_NOTIN_DISTINCTON_CLAUSE is not, the guard fails
to skip targetIsInAllPartitionLists() and recomputes it, but setting
the same bit again changes nothing. When
UNSAFE_NOTIN_DISTINCTON_CLAUSE is already set, point 4 is skipped and
UNSAFE_NOTIN_PARTITIONBY_CLAUSE is left unset; but such a column is
already unsafe for pushdown via UNSAFE_NOTIN_DISTINCTON_CLAUSE, so the
outcome is unchanged.
To fix, test UNSAFE_NOTIN_PARTITIONBY_CLAUSE, matching the bit being
set and the pattern of the surrounding checks.
Back-patch to v15, where the buggy check was introduced.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49Q_xnF_P2QSUyDzJ34MnrO7dh-cUAaK2HJPgSgh88NcA@mail.gmail.com
Backpatch-through: 15
With address sanitizer's stack-use-after-return check, stack variables are
moved to heap allocations, to allow to detect references to the memory at a
later time. That broke our stack-depth check, which is why we had to disable
detect_stack_use_after_return in CI. Luckily __builtin_frame_address() works
correctly, even under asan, so use that.
We started using __builtin_frame_address() with de447bb8e6, however as of
that commit we just used it for the stack base address, not for the value to
compare to the base address. Now we use it for both.
When building without __builtin_frame_address() support, we continue to use
stack variables for the stack depth determination.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2kk4z4odvuyrg7qlwjd7ft4eron4cle4btb33v4qatgsdkayir@gj6e62rgsel4
Backpatch-through: 14
Previously, ProcSignalInit() read the global barrier generation before
publishing its PID into pss_pid. This created a race condition: a
process could initialize its local generation with an older global
value, while a concurrent EmitProcSignalBarrier() might skip that
process because its pss_pid was still zero. This resulted in
WaitForProcSignalBarrier() hanging indefinitely.
Fix this by publishing pss_pid before reading psh_barrierGeneration
with a memory barrier so that the store to pss_pid is ordered before
the load. A concurrent EmitProcSignalBarrier() then either observes
the published PID and signals this slot, or completes its generation
increment before we load it.
While this race has become more visible due to recent features using
signal barriers in more places (such as online wal_level changes), the
issue is theoretically present since signal barriers were introduced
to release smgr caches (e.g., in DROP DATABASE). v14 has the
procsiangl barrier infrastricutre but no in-tree caller that actually
emits a barrier, so the case is unreachable there.
This issue was also reported by buildfarm member flaviventris.
Reported-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/CAEze2WgAJmWReDN7Chtba8Er2YBvKCoa0KVN25-1evnTrHsLyA@mail.gmail.com
Backpatch-through: 15
Concurrent DDL can leave behind objects referencing other objects that
no longer exist. This can happen if an object is dropped, while a new
object that depends on it is created concurrently. For example:
session 1: BEGIN; CREATE FUNCTION myschema.myfunc() ...;
session 2: DROP SCHEMA myschema;
session 1: COMMIT;
DROP SCHEMA does check that there are no objects dependending on the
schema being dropped, but it does not see objects being concurrently
created by other sessions. Even if it did, this scenario would still
fail:
session 1: BEGIN: DROP SCHEMA myschema;
session 2: CREATE FUNCTION myschema.myfunc() ...;
session 1: COMMIT;
When the DROP SCHEMA runs, the schema was empty, but the new function
is created in it before the dropping transaction completes. The CREATE
FUNCTION does not see that the schema is concurrently being dropped.
In both of these scenarios, the function is left behind in the schema
that no longer exists.
To fix, acquire AccessShareLock on all referenced objects when
recording dependencies. This conflicts with the AccessExclusiveLock
taken by DROP, preventing the race. After acquiring the lock, verify
that the object still exists, and if it was dropped concurrently,
report an error. We already had such a mechanism for shared
dependencies, but for some reason we didn't do it for in-database
dependendies.
Ideally the locks would be acquired much earlier when creating a new
object, but that will require modifying a lot of callers. This check
while recording the dependency is a nice wholesale protection, and
even if we change all the CREATE commands to acquire locks earlier,
it's still good to have this as a backstop to catch any cases where we
forgot to do so.
The patch adds a few tests for some cases that left behind orphaned
objects before this. It also adds a test for roles, which already had
such protection, although that test is partially disabled because the
error message includes an OID which is not predictable.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Discussion: https://postgr.es/m/ZiYjn0eVc7pxVY45@ip-10-97-1-34.eu-west-3.compute.internal
Backpatch-through: 14
When creating a relation with a dropped column, we called
recordDependencyOn() also on the datatype of the dropped column, which
is always InvalidOid. In versions 15 and above, that was harmless
because recordDependencyOn() considers InvalidOid as a pinned object,
and skips over it. On version 14, isPinnedObject() does not consider
InvalidOid as pinned, so we created a bogus pg_depend entry with
refobjectid == 0.
As far as I can tell, the only case when AddNewAttributeTuples() is
called with dropped columns is when performing a table-rewriting ALTER
TABLE command. That temporarily creates a new relation with the same
columns, including dropped ones, then swaps the relations, and drops
the newly created table again. So even on version 14, the bogus
pg_depend entry was only on the transient relation that was dropped at
the end of the ALTER TABLE command, which was harmless.
Even though this is harmless, let's be tidy, similar to commit
713bce9484. The reason I noticed this now and why I backported this,
is because the next commit will add code to acquire locks on the
referenced objects, and we don't want to acquire a lock on InvalidOid.
Discussion: https://postgr.es/m/ZiYjn0eVc7pxVY45@ip-10-97-1-34.eu-west-3.compute.internal
Backpatch-through: 14
DisownLatch() was executed after the PGPROC entry of the process
terminated is pushed back into a freelist. A newly-forked backend that
recycles the slot could call OwnLatch() and PANIC with a "latch already
owned by PID", taking down the server.
There were two scenarios related to lock groups where this issue could
be reached:
* A follower pushes the leader's PGPROC back to the freelist while the
leader has not yet called DisownLatch() in its own ProcKill().
* A leader outliving all its followers pushes its own PGPROC onto the
freelist before reaching DisownLatch(), which would be the most common
scenario.
This issue is fixed by calling SwitchBackToLocalLatch() and
DisownLatch() at an earlier phase of ProcKill(), before any freelist
manipulation happens, so that the slot of the backend terminated is
never exposed as owning a latch.
Note that pgstat_reset_wait_event_storage() is kept at a later stage.
An upcoming commit will take advantage of that by introducing a test
able to check the original PANIC scenario.
Author: Vlad Lesin <vladlesin@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/d2983796-2603-41b7-a66e-fc8489ddb954@gmail.com
Backpatch-through: 14
This commit fixes two bugs in ProcKill()'s lock-group teardown freelist
publication:
* a double push of the leader's PGPROC that corrupts the freelist.
* a leak of the last follower's PGPROC slot.
ProcKill()'s lock-group teardown had two PGPROC freelist updates
scattered through the function, done under two separate freeProcsLock
acquisitions:
* A follower's push of the leader's PGPROC, done when a follower is the
last group member exiting.
* Every backend's self-push at the bottom of the function.
The two freelist updates were coordinated only by inspecting
proc->lockGroupLeader, which a follower could clear as a side effect of
pushing the leader. This coordination was broken. For example, with
two concurrent backends:
* The follower clears leader->lockGroupLeader and pushes the leader's
PGPROC under leader_lwlock.
* The follower does not clear its own proc->lockGroupLeader, being
skipped.
* When the leader reaches the bottom of ProcKill(), it sees a NULL
proc->lockGroupLeader (the follower cleared it) and pushes itself,
causing a second dlist_push_tail() of the same node onto the same
freelist.
* The follower at the bottom sees its own proc->lockGroupLeader being
not NULL (never cleared) and skips its own push, causing its own slot
to leak.
This commit refactors the freelist manipulation to be done in two
distinct phases, each step using its own lock acquisition to ensure that
each freelist operation happens in an isolated manner for each backend
(follower or leader):
- First, under a single leader_lwlock acquisition, check the state of
the lock-group. Depending on if we are dealing with a follower and/or a
leader, and if the leader has exited before a follower, then set some
state booleans that define which actions should be taken with the
freelist.
- Second, under a single freeProcsLock acquisition, perform the cleanup
actions, self-push of a backend and/or push of the leader back to the
freelist.
This is an old issue, dating back to 9.6 where parallel workers and lock
grouping has been added.
Author: Vlad Lesin <vladlesin@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/d2983796-2603-41b7-a66e-fc8489ddb954@gmail.com
Backpatch-through: 14
When pg_createsubscriber fails after creating logical replication
objects, it should remove the publication and replication slot that
it created on the publisher.
Previously, if dropping subscriber-side objects failed,
pg_createsubscriber reset its internal cleanup state too early. As a
result, the exit-time cleanup could skip removing the publication or
replication slot on the publisher.
This could leave pg_createsubscriber-created objects behind on
the publisher after a failed run. That can make a retry harder,
because the leftover publication or replication slot may need to be
removed manually before running pg_createsubscriber again.
In the case of a replication slot, leaving it behind can also retain
WAL files longer than expected.
The cause of this issue was that the flags made_publication and
made_replslot tracking whether pg_createsubscriber created
a publication or replication slot on the primary were incorrectly
reset to false when failures occurred while dropping objects
on the subscriber.
This commit fixes the issue by preventing those cleanup flags from
being reset even when failures occurred while dropping objects
on the subscriber, ensuring proper cleanup of primary objects
before exit on failure.
Backpatch to v17, where pg_createsubscriber was added.
Author: Nisha Moond <nisha.moond412@gmail.com>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/CABdArM5V9QKK1PkLY9dpgAcZa3kUp84-wPqPovxvdLOri4=69w@mail.gmail.com
Backpatch-through: 17
EventTriggerOnLogin() tries to clear pg_database.dathasloginevt when
the database no longer has any login event triggers but the flag is
still set. To make that safe against concurrent flag setters, it
takes a conditional AccessExclusiveLock on the database object.
On a hot standby, that lock acquisition fails outright with
FATAL: cannot acquire lock mode AccessExclusiveLock on database
objects while recovery is in progress
because LockAcquireExtended() refuses locks stronger than
RowExclusiveLock on database objects during recovery. The standby
already replays the flag's value from the primary, so the dangling
flag is the result of replaying a state in which the primary had
already dropped its login event triggers but not yet run a login
event trigger pass to clear the flag. Any session connecting to the
standby in that window therefore fails to connect.
Skip the cleanup on a standby. The flag will be cleared via WAL
replay once the primary clears it on its side.
Add a recovery TAP test that reproduces the original report: create
and drop a login event trigger on the primary in one session, wait
for the standby to replay, then verify that a fresh connection to
the standby succeeds.
Backpatch to v17, where the login event triggers were introduced.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reported-by: Egor Chindyaskin <kyzevan23@mail.ru>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/19488-d7ccfca2bf6b74b0%40postgresql.org
Backpatch-through: 17
The check for the minimum expected bytea size of a MVDependencies object
was using SizeOfItem() for its calculation. This macro uses the number
of attributes in a single dependency.
This minimum size calculation should be based on MinSizeOfItems(), that
computes the minimum expected size as the header plus the
minimally-sized number of dependency items.
Oversight in d08c44f7a4.
Author: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Discussion: https://postgr.es/m/4b8d299d-2505-4c30-bf80-0f697410db35@tantorlabs.com
Backpatch-through: 14
When reusing an existing WAL receiver after it has reached
WALRCV_WAITING for new instructions, RequestXLogStreaming() copied
PrimaryConnInfo into WalRcv->conninfo before switching the state to
WALRCV_RESTARTING. At that point ready_to_display could still be true,
so pg_stat_wal_receiver could expose the raw connection string,
including sensitive fields, but it should only show the user-displayable
version of the connection string.
WALRCV_RESTARTING does not establish a new connection. The waiting WAL
receiver reuses its existing connection and only needs a new startpoint
and timeline, so there is no need to copy the raw connection string into
shared memory again. Let's only copy conninfo when launching a new WAL
receiver after WALRCV_STOPPED, not while waiting for instructions.
This commit adds coverage for the case fixed by this commit to the
timeline-switch test by verifying that the WAL receiver conninfo remains
consistent across the jump.
Backpatch all the way down, as this issue is possible since
pg_stat_wal_receiver has been introduced.
Author: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/EF91FF76-1E2B-4F3B-9162-290B4DC517FF@gmail.com
Backpatch-through: 14
Commit c37b3d08ca attempted to preserve group permissions on pg_recvlogical
output files when group access was enabled on the source cluster. However,
the output files were still created with a fixed S_IRUSR | S_IWUSR mode,
preventing group-read permissions from being applied.
This commit fixes the issue by creating output files with pg_file_create_mode
instead of a hard-coded mode. This allows pg_recvlogical to correctly preserve
group permissions from the source cluster.
Backpatch to all supported branches.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwHhpizYzMo3nFP4GkNMueSNMY3QfC-gBN1VTXtuiANDvw@mail.gmail.com
Backpatch-through: 14
This commit moves the definitions of InjectionPointConditionType and
InjectionPointCondition into a new header local to the test module
injection_points.h, so as these can be shared across more files in the
module. A patch for a bug fix is under discussion, whose proposed test
will benefit from this refactoring.
Backpatch down to where the module exists, as this should be useful for
future bug fixes, even cases unrelated to the thread where this change
has been discussed.
Author: Andrey Borodin <x4mmm@yandex-team.ru>
Author: Vlad Lesin <vladlesin@gmail.com>
Discussion: https://postgr.es/m/d2983796-2603-41b7-a66e-fc8489ddb954@gmail.com
Backpatch-through: 17
Three locations use Assert() to guard against a mismatch between the
number of columns advertised in the RELATION message and the number
actually received in the subsequent INSERT/UPDATE tuple message. Since
these values originate from the publisher, the check must survive into
production builds.
A malicious or buggy publisher can send a RELATION claiming N columns
and an INSERT claiming M < N columns. The subscriber's apply worker
indexes into colvalues[]/colstatus[] using column indices from the
RELATION message's attribute map, causing a heap out-of-bounds read when
the tuple's column array is smaller than expected. We've looked, without
success, for a scenario in which the publisher holds sufficient control
over these out-of-bounds bytes to exploit this or even to reach a
SIGSEGV. Despite not finding one, the code has been fragile. Back-patch
to v14 (all supported versions).
Reported-by: Varik Matevosyan <varikmatevosyan@gmail.com>
Author: Varik Matevosyan <varikmatevosyan@gmail.com>
Discussion: https://postgr.es/m/CA+bBoog3cCogktzfLb9bppUByu-10B3CFp8u=iKXG_OvtAguCw@mail.gmail.com
Backpatch-through: 14
Commit b7b0f3f272 ("Use streaming I/O in sequential scans") routed
sequential scans through read_stream_next_buffer(), bypassing the
RELATION_IS_OTHER_TEMP() check in ReadBufferExtended(). As a result,
a superuser can attempt to read or modify temp tables of other
sessions through the read-stream path. When the query plan uses no index,
SELECT/UPDATE/DELETE/MERGE silently see no rows / report zero affected rows,
and COPY produces an empty output -- because the buffer manager has no
visibility into the owning session's local buffers and silently returns
nothing. Any query plan that uses, for instance, a btree index
still errors out via the existing check in ReadBufferExtended(), which
is reached from hio.c and nbtree respectively, but this is incidental.
Fix by enforcing RELATION_IS_OTHER_TEMP() at the three additional
buffer-manager entry points:
- read_stream_begin_impl() rejects the read at stream setup time,
covering sequential and bitmap scans that go through the
read-stream path.
- ReadBuffer_common() becomes the canonical place for the check,
consolidating the existing one previously kept in
ReadBufferExtended(). All ReadBufferExtended() callers go through
ReadBuffer_common(), so the consolidation is behavior-preserving.
- StartReadBuffersImpl() catches direct callers of StartReadBuffers()
that bypass both of the above. This is currently defense-in-depth,
but documents the contract for future code.
The companion test in src/test/modules/test_misc was added in the
preceding commit; this commit updates the assertions for SELECT,
UPDATE, DELETE, MERGE, and COPY (which previously documented the
bug as silent success) to expect the new error.
Author: Jim Jones <jim.jones@uni-muenster.de>
Author: Daniil Davydov <3danissimo@gmail.com>
Co-authored-by: Alexander Korotkov <aekorotkov@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Soumya S Murali <soumyamurali.work@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com
Backpatch-through: 17
Add a TAP test in src/test/modules/test_misc that documents what
happens when one session attempts to read or modify another session's
temporary table. This commit only adds tests; it does not change
backend behavior, so the assertions reflect current behavior:
- SELECT, UPDATE, DELETE, MERGE, COPY on a table without an index
silently succeed with no error and zero rows / zero affected rows.
These commands run through the read-stream path, which currently
bypasses the RELATION_IS_OTHER_TEMP() check. This is the
underlying bug to be fixed in a follow-up.
- INSERT errors with "cannot access temporary tables of other
sessions" because hio.c calls ReadBufferExtended() to find a page
with free space and is caught by the existing check there.
- Index scan errors via the same existing check, reached through
nbtree -> ReadBuffer -> ReadBufferExtended.
- TRUNCATE / ALTER TABLE / ALTER INDEX / CLUSTER fail with their
command-specific error messages.
- VACUUM is silently skipped to avoid noise during database-wide
VACUUM (vacuum_rel() returns without warning).
- DROP TABLE is intentionally allowed: DROP does not touch the
table's contents, and autovacuum relies on this to clean up
temp relations orphaned by a crashed backend.
- ALTER FUNCTION / DROP FUNCTION on an owner-created function over
its own temp row type work as catalog operations -- they don't
read the underlying data.
- CREATE FUNCTION from a separate session, using another session's
temp row type as an argument, is allowed but emits a NOTICE: the
function is moved into the creator's pg_temp namespace with an
auto-dependency on the borrowed type, so it disappears together
with the session that created it.
- A bare DROP TABLE on a temp table that has a cross-session
dependent function fails with a catalog-level dependency error.
- LOCK TABLE in ACCESS SHARE mode on another session's temp table
succeeds and properly blocks the owner's session-exit cleanup
(which acquires AccessExclusiveLock via findDependentObjects).
This exercises the same LockRelationOid path used by autovacuum
when cleaning up orphaned temp relations.
- When the owner session ends, the normal session-exit cleanup
cascades through DEPENDENCY_NORMAL and removes both the temp
objects and any cross-session functions that depended on them.
Also, document the contract for RELATION_IS_OTHER_TEMP() so that
future buffer-access entry points enforce the same rule.
Backpatch this through PostgreSQL 17, where b7b0f3f272 introduces a code
path bypassing this check.
Author: Jim Jones <jim.jones@uni-muenster.de>
Author: Daniil Davydov <3danissimo@gmail.com>
Co-authored-by: Alexander Korotkov <aekorotkov@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Soumya S Murali <soumyamurali.work@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com
Backpatch-through: 17
When pgbench runs with multiple threads and verbose error reporting is
enabled (--verbose-errors), multiple clients can build verbose error
messages concurrently. Previously, a function-local static
PQExpBuffer was used for these messages, causing the buffer to be
shared across threads. This was not thread-safe and could result in
corrupted or incorrect log output.
Fix this by using a local PQExpBufferData instead of a static buffer.
This keeps verbose error messages correct during concurrent execution.
Backpatch to v15, where this issue was introduced.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Alex Guo <guo.alex.hengchen@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwER1AjGXpkKB9t9820NBhMQ_Ghv7=HsKeodUr3=SZsF4g@mail.gmail.com
Backpatch-through: 15
Two cases fixed by 2b5ba2a0a1 were not covered, to emulate the
handling of corrupted data, for:
- set control bit with a valid 2-byte match tag where offset is 0.
- set control bit with a valid 2-byte match tag where offset exceeds
output written.
Oversight in 67d318e704.
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/agF4xkIdRcrCIprs@paquier.xyz
Backpatch-through: 14
Previously, pg_stat_progress_copy in the subscriber could continue to show
the initial COPY operation for logical replication table synchronization as
active even after the data copy had finished. The stale progress entry
remained visible until synchronization caught up with the publisher.
This happened because the table synchronization code called BeginCopyFrom()
and CopyFrom(), but failed to call EndCopyFrom() afterward.
This commit fixes the issue by adding the missing EndCopyFrom() call so that
the COPY progress state in the subscriber is cleared as soon as the initial
data copy completes.
Backpatch to all supported branches.
Author: Shinya Kato <shinya11.kato@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: ChangAo Chen <cca5507@qq.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CAOzEurQKuy3RiPkd=25PEwEzaqHuGvEOf=X7vaVzhgNjaukYzA@mail.gmail.com
Backpatch-through: 14
The postmaster test 004_negotiate.pl could fail due to IO::Socket::INET
gone missing, in environments that cannot use Unix sockets.
Oversight in the backport done in 6dffaeb8e5, so like the other commit
this is applied across the v14~17 range. Per buildfarm member drongo.
Security: CVE-2026-6479
Backpatch-through: 14
When result_is_int is set to 0, PQfn() cannot validate that the
result fits in result_buf, so it will write data beyond the end of
the buffer when the server returns more data than requested. Since
this function is insecurable and obsolete, add a warning to the top
of the pertinent documentation advising against its use.
The only in-tree caller of PQfn() is the frontend large object
interface. To fix that, add a buf_size parameter to
pqFunctionCall3() that is used to protect against overruns, and use
it in a private version of PQfn() that also accepts a buf_size
parameter.
Reported-by: Yu Kunpeng <yu443940816@live.com>
Reported-by: Martin Heistermann <martin.heistermann@unibe.ch>
Author: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Security: CVE-2026-6477
Backpatch-through: 14
If you accumulate many arrays full of NULLs, you could overflow
'nitems', before reaching the MaxAllocSize limit on the allocations.
Add an explicit check that the number of items doesn't grow too large.
With more than MaxArraySize items, getting the final result with
makeArrayResultArr() would fail anyway, so better to error out early.
Reported-by: Xint Code
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Backpatch-through: 14
Security: CVE-2026-6473
pg_locale_icu.c was full of places where a very long input string
could cause integer overflow while calculating a buffer size,
leading to buffer overruns.
It also was cavalier about using char-type local arrays as buffers
holding arrays of UChar. The alignment of a char[] variable isn't
guaranteed, so that this risked failure on alignment-picky platforms.
The lack of complaints suggests that such platforms are very rare
nowadays; but it's likely that we are paying a performance price on
rather more platforms. Declare those arrays as UChar[] instead,
keeping their physical size the same.
pg_locale_libc.c's strncoll_libc_win32_utf8() also had the
disease of assuming it could double or quadruple the input
string length without concern for overflow.
Reported-by: Xint Code
Reported-by: Pavel Kohout <pavel.kohout@aisle.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Backpatch-through: 14
Security: CVE-2026-6473
pg_rewind and pg_basebackup could be fed paths from rogue endpoints that
could overwrite the contents of the client when received, achieving path
traversal.
There were two areas in the tree that were sensitive to this problem:
- pg_basebackup, through the astreamer code, where no validation was
performed before building an output path when streaming tar data. This
is an issue in v15 and newer versions.
- pg_rewind file operations for paths received through libpq, for all
the stable branches supported.
In order to address this problem, this commit adds a helper function in
path.c, that reuses path_is_relative_and_below_cwd() after applying
canonicalize_path(). This can be used to validate the paths received
from a connection point. A path is considered invalid if any of the two
following conditions is satisfied:
- The path is absolute.
- The path includes a direct parent-directory reference.
Reported-by: XlabAI Team of Tencent Xuanwu Lab
Reported-by: Valery Gubanov <valerygubanov95@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Backpatch-through: 14
Security: CVE-2026-6475
A few functions in this file were incautious about multiplying a
possibly large integer by a factor more than 1 and then using it as
an allocation size. This is harmless on 64-bit systems where we'd
compute a size exceeding MaxAllocSize and then fail, but on 32-bit
systems we could overflow size_t, leading to an undersized
allocation and buffer overrun. To fix, use palloc_array() or
mul_size() instead of handwritten multiplication.
Reported-by: Sven Klemm <sven@tigerdata.com>
Reported-by: Xint Code
Author: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tatsuo Ishii <ishii@postgresql.org>
Security: CVE-2026-6473
Backpatch-through: 14
This omission allowed roles to create multirange types in any
schema, potentially leading to privilege escalations. Note that
when a multirange type name is not specified in CREATE TYPE, it is
automatically placed in the range type's schema, which is checked
at the beginning of DefineRange().
Reported-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Security: CVE-2026-6472
Backpatch-through: 14
drop_existing_subscription() neglected to escape the subscription
name when generating its query string. To fix, use
PQescapeIdentifier() to construct a properly escaped name, and use
it in the ALTER SUBSCRIPTION and DROP SUBSCRIPTION commands.
Reported-by: Yu Kunpeng <yu443940816@live.com>
Author: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Security: CVE-2026-6476
Backpatch-through: 17
Although pg_strftime() has defined error conditions, no callers bother
to check for errors. This is problematic because the output string is
very likely not null-terminated if an error occurs, so that blindly
using it is unsafe. Rather than trusting that we can find and fix all
the callers, let's alter the function's API spec slightly: make it
guarantee a null-terminated result so long as maxsize > 0.
Furthermore, if we do get an error, let's make that null-terminated
result be an empty string. We could instead truncate at the buffer
length, but that risks producing mis-encoded output if the tz_name
string contains multibyte characters. It doesn't seem reasonable for
src/timezone/ to make use of our encoding-aware truncation logic.
Also, the only really likely source of a failure is a user-supplied
timezone name that is intentionally trying to overrun our buffers.
I don't feel a need to be particularly friendly about that case.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Backpatch-through: 14
Security: CVE-2026-6474
timeofday() assumed that the output of pg_strftime() could not contain
% signs, other than the one it explicitly asks for with %%. However,
we don't have that guarantee with respect to the time zone name (%Z).
A crafted time zone setting could abuse the subsequent snprintf()
call, resulting in crashes or disclosure of server memory.
To fix, split the pg_strftime() call into two and then treat the
outputs as literal strings, not a snprintf format string. The
extra pg_strftime() call doesn't really cost anything, since the
bulk of the conversion work was done by pg_localtime().
Also, adjust buffer widths so that we're not risking string truncation
during the snprintf() step, as that would create a hazard of producing
mis-encoded output.
This also fixes a latent portability issue: the format string expects
an int, but tp.tv_usec is long int on many platforms.
Reported-by: Xint Code
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Backpatch-through: 14
Security: CVE-2026-6474
ALTER SUBSCRIPTION ... REFRESH PUBLICATION interpolates schema and
relation names into SQL without quoting them. A crafted subscriber
relation name can inject arbitrary SQL on the publisher. Test such a
name. Back-patch to v16, where commit
8756930190 first appeared.
Reported-by: Pavel Kohout <pavel.kohout@aisle.com>
Author: Pavel Kohout <pavel.kohout@aisle.com>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Backpatch-through: 16
Security: CVE-2026-6638
This commit applies timingsafe_bcmp() to authentication paths that
handle attributes or data previously compared with memcpy() or strcmp(),
which are sensitive to timing attacks.
The following data is concerned by this change, some being in the
backend and some in the frontend:
- For a SCRAM or MD5 password, the computed key or the MD5 hash compared
with a password during a plain authentication.
- For a SCRAM exchange, the stored key, the client's final nonce and the
server nonce.
- RADIUS (up to v18), the encrypted password.
- For MD5 authentication, the MD5(MD5()) hash.
Reported-by: Joe Conway <mail@joeconway.com>
Security: CVE-2026-6478
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Backpatch-through: 14
timingsafe_bcmp() should be used instead of memcmp() or a naive
for-loop, when comparing passwords or secret tokens, to avoid leaking
information about the secret token by timing. This commit just
introduces the function but does not change any existing code to use
it yet.
This has been initially applied as of 09be391126 in v18 and newer
versions, and will be used in all the stable branches for an upcoming
fix.
Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>
Discussion: https://www.postgresql.org/message-id/7b86da3b-9356-4e50-aa1b-56570825e234@iki.fi
Security: CVE-2026-6478
Backpatch-through: 14
Define MaxAllocSize in src/include/common/fe_memutils.h rather
than having several copies of it in different src/common/*.c files.
This also provides an opportunity to document it better.
Back-patch of commit 11b7de4a7, needed now because assorted security
fixes are adding additional references to MaxAllocSize in frontend
code.
Backpatch-through: 14-17
Security: CVE-2026-6473
