ReplicationSlotRelease() copied MyProc->statusFlags unconditionally
when resetting PROC_IN_LOGICAL_DECODING, using its pgxactoff to decide
which area to update.
Slot invalidations can be done by the checkpointer or the startup
process, and ReplicationSlotRelease() ignored the fact that pgxactoff is
only meaningful for processes in the proc array, that auxiliary
processes never join. pgxactoff would remain at 0 for these, meaning
that an incorrect set of status flags would be updated.
The consequences of this issue could be various, depending on the
statsFlags lost in the proc slot incorrectly overwritten. Losing
PROC_AFFECTS_ALL_HORIZONS has been mentioned as the worse case, causing
VACUUM to remove rows a standby may needs even if standby feedback is
enabled. Assertions failures are another possibility.
Let's skip the update of statusFlags unless PROC_IN_LOGICAL_DECODING is
set. Nothing but StartupDecodingContext() sets it, and no auxiliary
processes call it.
Author: Vlad Lesin <vladlesin@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/889a06dd-45fc-423b-9dd2-87d5b5dc60c6@gmail.com
Backpatch-through: 14
calcSumX2 is used to inform the aggregate transition function whether it
should include the square of the transition value in the aggregate
state. The deserialize functions for various numeric aggregates that do
use calcSumX2 weren't properly setting the property to true. This does
not matter in core as we never perform any aggregate transitions after a
deserialize operation.
This fix is only required for external code which plumbs aggregate paths
together in such a way that could result in aggregate transitions being
performed after states have been deserialized.
Author: Maxime Schoemans <maxime.schoemans@enterprisedb.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/FEBF5C3D-CC53-455F-AA9D-4D377A2E957C@enterprisedb.com
Backpatch-through: 14
When searching the local relation by index, the apply worker can use the
first match only if the index is the relation's replica identity or
primary key. Otherwise, every match must be compared with the search slot.
Only the index OID was saved in the relation map, so this was determined
again during the search. Concurrent DROP INDEX or REINDEX can change the
index status between these two checks, causing the tuple search to fail
and the change to be silently skipped as an update_missing conflict.
Fix this by recording the answer as idxisreplident in the relation map
entry and avoid getting it second time in FindReplTupleInLocalRel() and
RelationFindReplTupleByIndex().
Oversight in 89e46da5e5.
Author: Mikhail Nikalayeu <mihailnikalayeu@gmail.com>
Author: vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Discussion: https://postgr.es/m/CADzfLwUJovFcnknCC9wjZKECX9xecgnGzC2r2TMV8h4QDD_jwQ@mail.gmail.com
Backpatch-through: 16, where it was introduced
Previously, pg_locks.waitstart could report the start time of a previous
lock wait for a new wait. For a regular backend, this could happen briefly
before the new start time was stored. On a standby, the startup process could
show the old time throughout the next wait, making it appear to have started
earlier than it did.
This happened because PGPROC->waitStart could remain set after a previous
wait in two cases. First, RemoveFromWaitQueue() did not clear it when a wait
was canceled due to a lock timeout, cancellation, or deadlock. Second, even
after a successful lock grant, ProcWakeup() could clear it before the waiter
stored its new start time.
Fix this by clearing waitStart in RemoveFromWaitQueue() when a failed wait
is removed, at the end of ProcSleep() to handle writes made after
ProcWakeup() cleared it, and in LockErrorCleanup() when an error bypasses
the ProcSleep() reset. This prevents pg_locks.waitstart from showing the
previous wait's time for a new wait.
Backpatch to all supported versions.
Author: Shihao Zhong <zhong950419@gmail.com>
Reported-by: Alex Shapalov <shapalov@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Andrew Krylosov <krylosov.andrew@gmail.com>
Discussion: https://postgr.es/m/CAPrb+Q+XN=sNusXiUeWmMo2H7Qgq3Y4uPekSSLkHcnCyf7GhXg@mail.gmail.com
Discussion: https://postgr.es/m/CAGRkXqQLxZBr-ouVrtaX2utMggi4+TiVMbgH04b_0JLgKryxbA@mail.gmail.com
Backpatch-through: 14
_reconnectToDB() failed to consider the possibility that it could
be called without any restrict_key set. This is reachable while
generating the "restore.sql" file in tar dump mode, which nobody
noticed because that file is mostly unused (and nothing really
bad happens anyway unless you specified --inserts mode). But
I wonder if there are other code paths that reach the issue.
Bug: #19718
Reported-by: Pavel Kopylov <pkopylov@cloudlinux.com>
Author: Pavel Kopylov <pkopylov@cloudlinux.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19718-7e945d0ff9b5a589@postgresql.org
Backpatch-through: 14
The header comment for this file says it assumes that "the CPU is not
picky about alignment", which we wouldn't really accept for Postgres.
The two functions that have alignment requirements have been hacked in
ways that are inconsistent and yet both ugly. des_setkey just casts
its "const char *" argument to "const uint32 *", relying on the caller
to ensure that that pointer is actually word-aligned, which the caller
does by declaring the buffer as uint32[] and then casting to "char *".
Meanwhile des_cipher carefully memcpy's to and from an internal
uint32[] buffer, which is pretty silly given that what it's passed
must be word-aligned for the benefit of des_setkey. And on top of
that we have a bunch of ugly casting and oddly-written pointer
arithmetic in the caller px_crypt_des. The odd pointer arithmetic
causes warnings about possible buffer overrun when using recent gcc
at -O3 or higher.
Let's clean this up by converting the buffer variable into a union
of a uint8 array and a uint32 array, so that we can remove all these
casts, and also replace the strange loop logic with a simple subscript
variable to satisfy gcc.
Reported-by: Andres Freund <andres@anarazel.de>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Co-authored-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/2005885.1790359097@sss.pgh.pa.us
Discussion: https://postgr.es/m/3nuudxv365kjnmwjhnygdakhxuktpdjvf26rt26eb44esgqdrj@y2x3vomkrfoo
Backpatch-through: 14
gcc 12.5.0 complains about this instance in LookupFuncWithArgs().
This is the same type of problem fixed in commit f04781df5, and
it's just as obscure why this compiler version picks on this case
when others don't. Still, silence it in the same way as f04781df5.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/1812498.1790279465@sss.pgh.pa.us
Backpatch-through: 14
SnapBuildInitialSnapshot() converts the snapshot builder's list of
committed transactions into a regular MVCC snapshot, which is then used
with HeapTupleSatisfiesMVCC(). However, a snapshot produced that way
and used as an MVCC snapshot can potentially cause data corruption, if
it captures a transaction after it writes its commit WAL record but
before it has updated its CLOG entry: it will be incorrectly used to set
hint bits as if that transaction had aborted.
Fix by having SnapBuildInitialSnapshot() wait until such transactions
have removed themselves from procarray, which guarantees correct
visibility.
Other uses of SnapBuildBuildSnapshot only use the snapshot as
historical, so they don't need the waits, but add commentary in that
function to alert possible future callers.
While at this, backpatch commit 504fe10d1d, "Fix misplaced comment in
snapbuild.c." The comment it moved was nearby the place we modify.
This bug was diagnosed by stress-testing of REPACK (CONCURRENTLY) in
branch 19, so add a test case that tickles it using that feature in that
branch and master. However, the bug exists in all branches, and in
branch 18 we can add a test with regular logical decoding using
injection points that tickles it. Unfortunately that test cannot be
backpatched to earlier branches for lack of facilities.
Reported-by: Mihail Nikalayeu <mihailnikalayeu@gmail.com>
Author: Antonin Houska <ah@cybertec.at>
Author: Rui Zhao <zhaorui126@gmail.com>
Backpatch-through: 14
Discussion: https://postgr.es/m/85833.1768840165@localhost
Discussion: https://postgr.es/m/CADzfLwU78as45To9a%3D-Qkr5jEg3tMxc5rUtdKy2MTv4r_SDGng%40mail.gmail.com
heapgettup_advance_block() (or heapgettup() and heapgettup_pagemode() in
versions before v16) would incorrectly decrement rs_numblocks when
advancing to the next block. Ordinarily this was fine, but if the TID
Range scan plan was used by a scrollable cursor and the cursor was fetched
backwards enough to result in another block being required, then
rs_numblocks may have reached 0 prematurely resulting in the scan ending
prematurely.
Reported-by: Jeff Davis <pgsql@j-davis.com>
Author: Ewan Young <kdbase.hack@gmail.com>
Author: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/6b50233fcc0f7d6f4845f083777fa9702d9612e4.camel@j-davis.com
Backpatch-through: 14
When executing a WHEN NOT MATCHED BY SOURCE action in MERGE, if the
tuple from the target table was concurrently deleted, the retry code
would incorrectly execute a WHEN NOT MATCHED BY TARGET action for the
row instead, giving incorrect results.
Fix in ExecMergeMatched() -- when handling a TM_Deleted failure
status, it should only set *matched to false (telling the caller to
execute any WHEN NOT MATCHED BY TARGET actions) if the failed action
was a WHEN MATCHED action. For a failed WHEN NOT MATCHED BY SOURCE
action, the source tuple did not exist, and the target tuple has just
been deleted, so it should do nothing.
Back-patch to v17, where support for WHEN NOT MATCHED BY SOURCE
actions was introduced.
Reported-by: Jeff Davis <pgsql@j-davis.com>
Author: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/ccdab5ba02c65af195b5a6d2d744a01d9de47cd3.camel@j-davis.com
Backpatch-through: 17
6dbb490261 combined pruning and freezing in a single WAL record but
unconditionally requested a cleanup lock during replay. Prior to version
17, freeze-only records were replayed under an ordinary exclusive lock.
1252a4ee28 subsequently folded visibility map updates into the same
records, extending the unnecessary cleanup-lock requirement to VM-only
updates starting in version 19.
Request a cleanup lock in redo only when the record actually prunes.
Freezing and visibility updates do not move or remove tuple storage, so
they need not wait for other buffer pins to be released. This avoids
unnecessary replay delays and cancellation of standby queries holding
buffer pins.
The fix is the same for all versions, though it addresses two oversights
in master/19 and one in 18/17.
Reported-by: Satyanarayana Narlapuram <satyanarlapuram@gmail.com>
Reported-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/85b6968b-49f2-4243-86a6-9b4116e6439b@iki.fi
Discussion: https://postgr.es/m/CAHg+QDf3NcB3vOAqQ5EFcV5DYYUZ9snCSJAU-x1JDbtcjK3eBQ@mail.gmail.com
Discussion: https://postgr.es/m/F9BF304E-1298-481C-829A-B4E10AED025A@yandex-team.ru
Backpatch-through: 17
Similar to commit "Remove unused global variables", silence
-Wunused-but-set-global warnings from clang 23 (included in -Wall).
These are variables that are used in assert-enabled builds only, so
annotate them with PG_USED_FOR_ASSERTS_ONLY.
When a join has an alias, the parser marks every Var referenced
through that alias with the join's RT index as its syntactic referent
(varnosyn), even though varno points at the underlying base relation.
Since commit 2ebf25e7d, join removal strips the removed relids from
the query tree using ChangeVarNodes() with INVALID_VAR, which asserts
that the relid doesn't appear in any field identifying a single
relation. A varnosyn that names the removed join violates that, so a
query like
SELECT j.x FROM (t1 s(x) LEFT JOIN t2 ON s.x = t2.a) j;
with t2.a unique fails an assertion. In non-assert builds, varnosyn
is silently set to INVALID_VAR.
Such syntactic references are still valid after the removal: the join
is only taken out of the jointree, and its RTE remains in the
rangetable through to the finished plan. So, when deleting a relid,
leave varnosyn unchanged, as join removal did before 2ebf25e7d.
Back-patch to v16, as with commit 2ebf25e7d.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAMbWs484SQSdzWTgZYKvwtuC6J4m7acw0LfBjp3aWPvav7gC_w@mail.gmail.com
Backpatch-through: 16
Commit e28cf96e9 preprocessed the copies of a query level's
PlaceHolderVars that were pushed down into its subqueries in a single
walk at the start of subquery_planner. That was wrong in two ways.
First, such copies can also be inserted later, when a join alias Var
of the outer level is expanded within a LATERAL subquery or a
SubLink's subselect and the alias expression contains a PHV. Those
copies were never preprocessed, so a SubLink within them survived into
the subquery's lateral references or into a SubPlan's argument list,
tripping an assertion in identify_current_nestloop_params or failing
in cost_qual_eval.
Second, when a pushed-down copy directly contains another PHV of the
same level, the walk preprocessed the inner one first and then the
outer one, so the outer one's preprocessing ran into the SubPlans just
created in the inner one and tripped an assertion in
flatten_join_alias_vars.
To fix, preprocess the copies right after join alias expansion
instead: within preprocess_expression, just before SubLinks are turned
into SubPlans, and for LATERAL subquery RTEs, right after their
aliases are flattened. Preprocessing a copy handles everything within
it, including any further copies nested inside its SubLinks, so the
walker no longer looks inside a copy.
Back-patch to v16, as with commit e28cf96e9.
Reported-by: Fujii Masao <masao.fujii@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwH-acRnAk1=Ki4EiAJEt67+Rz=_jyg7pbP_c6BK4oHZ4Q@mail.gmail.com
Backpatch-through: 16
The PASSING arguments of JSON_EXISTS(), JSON_QUERY(), JSON_VALUE() and
JSON_TABLE() are evaluated by plain expression steps and given to the
jsonpath executor as-is.
When dealing with a varlena, JsonItemFromDatum() was building a
jbvString directly the contents of a varlena. For a varatt_external
(varatt_external[oid/oid8] on HEAD), this ignored that the contents of
the varlena need to be detoasted, leading to incorrect contents
returned.
Let's detoast the value, like the jsonb and json cases. b0feda79fd
has fixed the same issue for subscript values in jsonb.
Note that the results of DatumGetTextPP() are allocated in the memory
context used for the evaluation of the expression, which is reset once
per row.
Oversight in 6185c9737c.
Author: Chaitanya Choudhary <chaitanyyachoudhary@gmail.com>
Author: Shihao Zhong <zhong950419@gmail.com>
Discussion: https://postgr.es/m/19693-2ecd2b52c838b3e5@postgresql.org
Backpatch-through: 17
pgoutput records the toplevel XID in the relation's streamed_txns
list, allocated in CacheMemoryContext, when it sends a relation's
schema during a streamed transaction. We clean up that list at stream
commit and stream abort, but missed stream prepare, so the XIDs stay
around until a relcache invalidation for the relation or until the
walsender exits.
Fix by cleaning up the cache after writing the stream prepare message,
marking the schema as sent as we do at stream commit. The subscriber
applies the spooled relation messages at PREPARE time and its relation
map isn't rolled back by ROLLBACK PREPARED, so we don't need to send
the schema again even if the prepared transaction eventually aborts.
Oversight in commit 63cf61cdeb.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAJTYsWWt5aaKHCnV=jNOwABzspd-WbnhWLndDqtESDQRQ-BkKQ@mail.gmail.com
Backpatch-through: 15
We unintentionally copied the original query's limitOption field while
building the modified subquery. If it was LIMIT_OPTION_WITH_TIES,
that behavior would propagate through to execution, potentially
causing a "more than one row returned by a subquery used as an
expression" error. Fix by resetting to the default LIMIT_OPTION_COUNT.
This bug dates back to the introduction of WITH TIES.
Bug: #19694
Reported-by: Chaitanya Choudhary <chaitanyyachoudhary@gmail.com>
Author: Chaitanya Choudhary <chaitanyyachoudhary@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19694-4e61bd0475398eef@postgresql.org
Backpatch-through: 14
Some code added in 489247b0e tried to prune the DEFAULT partition when
the next partition had a MINVALUE clause and likewise when the final
partition to scan had a MAXVALUE clause for the final partition key in
the pruning step. This code was incorrect as it could prune the default
partition incorrectly in cases such as:
p: PARTITION BY RANGE (a, b)
p1: FOR VALUES FROM (13, 0) TO (19, MAXVALUE)
pd: DEFAULT
SELECT * FROM t WHERE a = 32 AND b >= -7;
Here the pruning step for a = 32 and b >= -7 would see that only the
default partition needs to be scan, but it would then see that the
partition prior to the default had a MAXVALUE bound then prune away the
default thinking that it needn't be scanned. This could result in
incorrect results.
Fix this by moving the code that looks for the MAXVALUE bound into the
code handling BTEqualStrategyNumber so that when we're pruning with a
prefix of the partition keys, we check if the bound for the offset we've
calculated lands on a partition where the next partition key is bounded
with MAXVALUE. If so we don't include the default partition.
This leaves only the case of the first partition key. We handle that by
modifying the existing code that was checking the last key covered by
the given values.
Author: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAON2xHO=sqdqp=z8zWkybnWp0AuvefnAi2ez2vOYWrhXB6hHWQ@mail.gmail.com
Backpatch-through: 14
Building the partition pruning steps for excessively complex queries
can take some time. Add CHECK_FOR_INTERRUPTS() to
get_steps_using_prefix_recurse() to allow more timely processing of
interruptions.
Bug: #19692
Reported-by: Qifan Liu <imchifan@163.com>
Discussion: https://postgr.es/m/19692-dacfc57cda5035fc@postgresql.org
Backpatch-through: 14
pgstat_get_entry_ref_cached() inserts a local entry_ref with
shmem-related fields set to NULL, expecting pgstat_get_entry_ref() (its
sole caller) to fill them up before returning. If an ERROR happens
while pgstat_get_entry_ref() runs, it could be possible to finish with a
local pgstats entry partially filled.
This could lead to a crash of pgstat_gc_entry_refs(), which tolerates a
NULL shared_stats in an assertion but unconditionally dereferenced its
"dropped" and "generation" fields.
This extends 4069df21be, being a cheap insurance against NULL pointer
dereference, if some code paths of pgstat_get_entry_ref() are not able
to perform any cleanup actions (for example after a dsm_create()
throwing an ERROR).
Reviewed-by: Grigorev Jurij <ju.grigorev@ftdata.ru>
Discussion: https://postgr.es/m/aqtiKTvl519bu8-V@paquier.xyz
Backpatch-through: 15
Previously, aborting an internal subtransaction during COMMIT or
PREPARE TRANSACTION could cause the following assertion failure.
This could happen, for example, when a deferred constraint trigger fired
at COMMIT and its PL/pgSQL exception block caught an error raised
while executing the trigger function.
TRAP: failed Assert("s->blockState == TBLOCK_SUBINPROGRESS || s->blockState
== TBLOCK_INPROGRESS || s->blockState == TBLOCK_IMPLICIT_INPROGRESS ||
s->blockState == TBLOCK_PARALLEL_INPROGRESS || s->blockState ==
TBLOCK_STARTED"), File: "xact.c", Line: 4851, PID: 73455
An internal subtransaction should be able to be aborted while the parent
transaction is in the COMMIT or PREPARE TRANSACTION phase. However,
RollbackAndReleaseCurrentSubTransaction()'s assertion check previously
did not allow TBLOCK_END and TBLOCK_PREPARE as parent transaction
states, causing the assertion failure.
This commit fixes the assertion check by allowing those two parent
transaction states.
Backpatch to all supported versions.
Reported-by: Fabrízio de Royes Mello <fabrizio@planetscale.com>
Author: Patrick Reynolds <piki@planetscale.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Fabrízio de Royes Mello <fabrizio@planetscale.com>
Discussion: https://postgr.es/m/CABo-N97AeMbWuYTWg-3%3D2DkTR3EkvS%2BFt%3DyEaWB181STsR1mBg%40mail.gmail.com
Backpatch-through: 14
A semijoin whose RHS is unique-ified groups the RHS on the expressions in
SpecialJoinInfo.semi_rhs_exprs. Those were recorded with whatever collation
the RHS expression itself exposes, which need not be the collation the join
compares with. Neither SortGroupClause nor the pathkey machinery carries a
collation of its own, so both Unique-over-Sort and HashAggregate then grouped
by the wrong equality: values the join considers equal survived, and the
following inner join emitted the outer row once per survivor.
With a non-deterministic collation on one side, "SELECT count(*) FROM t WHERE
c IN (SELECT c0 FROM t2)" therefore counted more rows than the same predicate
reports for the rows of t.
Label each RHS expression with the operator's input collation, the same
treatment process_equivalence() gives to equivalence class members. Every
consumer of semi_rhs_exprs reads the collation off the expression, so this
fixes the sort-based and hash-based paths together; in the branches where
create_unique_path() also passes these expressions to
relation_has_unique_index_for(), it likewise stops a unique index built with a
different collation from being taken as proof that unique-ification can be
skipped. The same goes for a subquery's DISTINCT computed under a different
collation, since translate_sub_tlist() punts on the relabeled expressions.
Reported-by: Suyang Zhong <syzhong16@gmail.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/19633-647cd4c73a84b085%40postgresql.org
Backpatch-through: 14
INSERT ... ON CONFLICT decides what to do based on the conflicting row
found by the arbiter index probe, but SSI never saw that read: the
probe runs with a dirty snapshot, which predicate locking ignores, and
the later fetch of the row uses SnapshotAny. When the statement then
writes nothing, as with DO NOTHING, DO UPDATE with a WHERE clause
rejecting the row, or DO SELECT, nothing records the read at all. A
concurrent writer of that row went unnoticed and write skew could
commit at SERIALIZABLE, even though the same schedule with a plain
SELECT of the row fails with a serialization error.
To fix, read the conflicting tuple again with the query snapshot,
right where the probe finds it. The table AM takes the SIREAD lock
and checks for a concurrent writer of the tuple as part of that read,
both under the buffer lock, so a writer either sees the lock or is
seen. A predicate lock by itself acquired separately after the probe
could not offer that: a writer passing its conflict check in between
would be missed. Doing this in the probe covers every conflict
action, including rows that the WHERE clause of DO UPDATE or DO SELECT
then rejects.
The DO NOTHING and DO UPDATE cases have been broken since ON CONFLICT
was added in 9.5; DO SELECT is new in v19. Backpatch to all supported
branches.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Author: Andrey Borodin <x4mmm@yandex-team.ru>
Reported-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reported-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://postgr.es/m/787936C5-4155-4CF9-939D-39DC0EC1C892@yandex-team.ru
Discussion: https://postgr.es/m/CAN4CZFM1GkHJkpMeo4G5rxtacVsfeKCJYiik9E9AKX1E9VYQ1w@mail.gmail.com
Backpatch-through: 14
A JSON constructor with a RETURNING clause uses a CaseTestExpr as the
placeholder for its result in the coercion expression. When such a
constructor appears in a WHEN clause of a simple CASE whose test
expression is a constant, eval_const_expressions substituted that
constant for the placeholder, so the coercion produced the CASE's test
value instead of the constructor's result. For instance,
CASE 'x' WHEN JSON_OBJECT('a': 'b' RETURNING text) THEN 1 ELSE 0 END
evaluated to 1.
To fix, keep case_val out of scope while simplifying the coercion, as
is already done for the elemexpr of an ArrayCoerceExpr.
Back-patch to v16, where the SQL/JSON constructor functions were
introduced.
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAMbWs48A=VCFbteTkuCoknO1_0-Cu0aMBT0M07dm7vj1QyixDg@mail.gmail.com
Backpatch-through: 16
When a subquery references an output of another subquery that gets
pulled up, and that output must be wrapped in a PlaceHolderVar because
of an intermediate outer join, the PHV expression is pushed down into
the subquery. That copy is not preprocessed along with the outer
query's expressions, so the two copies can diverge. This used to be
harmless, but since commit 2ebf25e7d join removal edits the whole
query tree, walking into subqueries, and can trip an assert in
ChangeVarNodes if it removes a rel whose Var survives only in such a
copy.
To fix, preprocess these copies at their owning query level, early in
subquery_planner, before anything can consume them (in particular
before SubLinks are turned into SubPlans). This covers copies pushed
into both LATERAL subquery RTEs and SubLink subselects, and handles
nested copies innermost-first. extract_lateral_references no longer
preprocesses the copies it pulls out.
Correspondingly, the subquery's own processing must leave the contents
of an upper-level PHV alone, since the owning level has already
preprocessed them: eval_const_expressions returns such a PHV
unchanged, and flatten_join_alias_vars no longer recurses into it.
Back-patch to v16, as with commit 2ebf25e7d.
Reported-by: Tender Wang <tndrwang@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAHewXN=kWGAXV537mKtSyBYobGdHhYJVDJJMXXZEmmPWE_zaPw@mail.gmail.com
Backpatch-through: 16
The in-tree getopt_long() moves each non-option to the end of argv,
which might put it right where an option's argument lookup expects
to find the argument. For example, "vacuumdb postgres --jobs"
takes "postgres" as the number of jobs instead of complaining that
--jobs is missing its argument. To fix, stop the argument lookups
at the start of the moved non-options, which we already track to
know when to stop scanning.
Oversight in commit 411b720343.
Author: Sehrope Sarkuni <sehrope@jackdb.com>
Reviewed-by: solai v <solai.cdac@gmail.com>
Discussion: https://postgr.es/m/CAH7T-arxDuVCSkorO%3Dk7%2BM-_JV0JFzMpN_EtKMyD2K0RDqZ2OA%40mail.gmail.com
Backpatch-through: 17
Several global variables are only used in Windows build. This causes
-Wunused-but-set-global warnings that the new clang 23 enables via
-Wall.
This commit marks these with an unused attribute to silence the
warnings.
(In the master branch, this is addressed differently, but for
backpatching, this just silences the warnings without any behavior
change.)
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/eb013f9d-2247-444e-8815-9d17b4ce78e7%40eisentraut.org
The new clang 23 has a new warning about set-but-unused
static (internal-linkage) global variables: -Wunused-but-set-global,
which is activated in PostgreSQL builds via -Wall. This triggers a
few warnings in PostgreSQL code. This commit removes several such
variables that were either never used or whose last use was removed
some time ago.
For pq_init_crypto_lib, we apply the same #ifdef HAVE_CRYPTO_LOCK that
the other uses of the variable already have, so that the variable is
either fully used or fully nonexistent, depending on the
configuration, not half-way.
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/eb013f9d-2247-444e-8815-9d17b4ce78e7%40eisentraut.org
DROP TABLESPACE checked pg_shdepend for dependent objects without
first locking the tablespace. A concurrent command that recorded a
shared dependency on the tablespace right after that check could still
commit, leaving an object whose pg_shdepend entry (or, for a relation,
pg_class.reltablespace) pointed to a tablespace that no longer existed.
Close the race by having DropTableSpace() take an AccessExclusiveLock
on the tablespace before calling checkSharedDependencies(). That
conflicts with the AccessShareLock shdepLockAndCheckObject() takes when
recording a new dependency, so the loser of the race blocks and
rechecks once the winner commits.
That AccessExclusiveLock creates a new deadlock: ALTER TABLESPACE
RENAME/SET and the internal ACL/owner updates in DROP OWNED and
REASSIGN OWNED touched the catalog tuple without locking the
tablespace, risking a lock-order cycle with DROP. Fix by taking an
AccessShareLock first in all four paths, rechecking pg_shdepend after
any wait in the DROP OWNED and REASSIGN OWNED cases. GRANT, REVOKE,
and ALTER TABLESPACE ... OWNER TO already lock the object.
Add isolation tests covering both orderings of the original race
(dependency-first and drop-first) and all four previously-unlocked
update paths.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/CAJTYsWXjAQFnGKzXsht3XK8KHhyhontwFJw4JAHsUSfs_ptR4g@mail.gmail.com
Backpatch-through: 14
This reverts commit c06d1a4ba6.
The commit assumed that if the FSM code tolerates torn pages, everything else
does so. Readers that do not tolerate that include RelationCopyStorage(),
used by ALTER TABLE ... SET TABLESPACE for every fork, the read stream in
RelationCopyStorageUsingBuffer() used by CREATE DATABASE ... STRATEGY =
wal_log, and, most awkwardly, the checksum verification in base backups and
pg_checksums.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260901211837.f6.noahmisch%40microsoft.com
Backpatch-through: 14
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
The following CVEs were listed in the release notes of some stable
branches, but they should not:
- CVE-2026-16238, not on v17 and older
- CVE-2026-14676, not on v17 and older
- CVE-2026-14681, not on v16 and older
- CVE-2026-14672, not on v15 and older
Author: Yogesh Sharma <yogesh.sharma@catprosystems.com>
Discussion: https://postgr.es/m/340842e1-31ce-4551-9d79-71de490f67e9@CatProSystems.com
Backpatch-through: 14-17
On slow machines, this test can take long enough to generate WAL that a
time-based checkpoint occurs before the primary is stopped. If the
checkpoint record is written to the tail WAL segment that the test later
removes, a standby initialized from the resulting backup tries to read
the missing checkpoint record and fails with a PANIC during startup.
This caused the test to fail on buildfarm member skink.
Fix this by setting checkpoint_timeout high enough to prevent unrelated
checkpoints during the test. This follows the approach used by other
recovery tests, such as 043_no_contrecord_switch.pl, that depend on a
specific WAL layout.
Backpatch to all supported versions.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwFajfjKhvNSTkTcE-wFn30p_A0_i1cvk-Q=FJhDr_5cXA@mail.gmail.com
Discussion: https://postgr.es/m/9ffdb19a-7a89-424e-925a-dd981c37f0ba@gmail.com
Backpatch-through: 14
When creating a nestloop plan for a partitionwise child join, the
outer rel's relids are child relids, but PlaceHolderInfo.ph_eval_at is
always expressed in terms of the topmost parent rels. As a result,
replace_nestloop_params() and identify_current_nestloop_params()
failed to recognize that a PlaceHolderVar evaluated at the outer child
rel can be supplied as a nestloop param. Instead, the Vars within the
PHV's expression were replaced with params, but the outer child rel
emits only the PHV, not those bare Vars, leading to "variable not
found in subplan target list" errors from setrefs.c.
To fix, also include the outer rel's top parent relids in the relid
set used for these checks, so that ph_eval_at comparisons are done in
terms of parent rels while Var checks continue to work in terms of
child rels.
On v18 and later, the required-outer set passed to
identify_current_nestloop_params() has the same problem: it is in
terms of child rels once a parameterized child join path has been
reparameterized by an upper child join. With the above fix in place,
a PlaceHolderVar that depends on both the outer rel and the parameter
source becomes a single NestLoopParam, and that param was never
claimed by any nestloop node, leading to "failed to assign all
NestLoopParams to plan nodes" errors. To fix, also include the top
parents of any child rels in that set. Older branches lack this code
path, so they receive only the first change.
Back-patch to all supported branches.
Bug: #19653
Reported-by: Annie <10215501441@stu.ecnu.edu.cn>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19653-9352cc6ba17b662f@postgresql.org
Backpatch-through: 14
When neither BKPIMAGE_HAS_HOLE nor BKPIMAGE_COMPRESSED is set,
DecodeXLogRecord() checks that bimg_len needs to be equal to BLCKSZ.
However, the associated error message reported the data length
associated to a block (DecodedBkpBlock.data_len), and not the length of
the block (DecodedBkpBlock.bimg_len).
Oversight in 57aa5b2bb1, probably due to some copy-paste from the
surroundings.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAJTYsWVz9ymE7aMt2ZiQvs1bxNvhjfTf+XaJEsdFw7MT5dvtYQ@mail.gmail.com
Backpatch-through: 14
When adding support for OpenSSL 4, one occurrence of OpenSSL error
message matching was missed in the backpatches to PostgreSQL 17 and
16.
Reported-by: Christoph Berg <cb@df7cb.de>
Discussion: https://postgr.es/m/apky-gqiLVtFkR-w@msg.df7cb.de
Backpatch-through: 16,17
Previously, a session that had created temporary objects could hang during
exit if its default transaction mode was SERIALIZABLE READ ONLY DEFERRABLE
and a concurrent serializable transaction was prepared. During backend
exit, temporary-relation cleanup pushed a regular transaction snapshot,
which honored the session's default settings and could wait for a safe
serializable snapshot. So, if the conflicting transaction was prepared,
this wait could last indefinitely.
The wait occurred while the backend was already exiting and interrupts
were held off, so even pg_terminate_backend() could not cancel it. The
backend therefore remained until the prepared transaction was resolved.
Temporary-relation cleanup only needs an active MVCC snapshot for fetching
TOAST data from catalog tuples while dropping temporary objects. It does
not need a transaction snapshot affected by the user's default isolation
settings. So, use a catalog snapshot instead, which provides the needed
protection without entering the deferrable safe-snapshot wait.
Backpatch to all supported versions.
Bug: #19441
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/19441-ec29f3b1363b4a68@postgresql.org
Backpatch-through: 14
The buildfarm member akepa reported a failure in the
031_recovery_conflict.pl test.
The test checked pg_stat_database_conflicts immediately after detecting
a recovery conflict in the standby log. However, the conflict counter is
flushed by the canceled backend during backend exit, so WAL replay
completion and the log message did not guarantee that the updated
statistics are visible yet. So, previously, the test could see a conflict
counter of 0 even though the conflict had already occurred, triggering
the test failure.
Fix this by polling for the expected conflict counter instead of reading
it only once, handling the asynchronous pgstats update.
Per buildfarm member akepa.
Backpatch to v17, where this test is enabled and has the same race.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwHmiLNRfvJDAR=PmxQgf7DbzPSO1M-1RoqO8oy=t2G5KA@mail.gmail.com
Backpatch-through: 17
Not many buildfarm members run this test, since it's gated behind
PG_TEST_EXTRA=xid_wraparound, but of those that do, the slower ones
not infrequently fail. The reason seems to be that the test expects
an INSERT command to either succeed or fail, but there's a window
where it can succeed while issuing a warning about impending
wraparound. poll_query_until treats nonempty stderr as a failure,
so it loops an extra time until the INSERT succeeds with no warning.
There seems no reason to treat this behavior as wrong, so adjust
the test to accept it.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Diagnosed-by: Alexander Lakhin <exclusion@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/7a68bee2-91bd-481d-be44-160e6beebc83@gmail.com
Backpatch-through: 17
When recovery starts from a backup without a signal file, pg_subtrans
is not started at the beginning of recovery and remains unstarted
throughout recovery. However, previously, a restartpoint run by
the checkpointer during recovery nevertheless tried to truncate
pg_subtrans, triggering the assertion failure:
TRAP: failed Assert("TransactionIdIsValid(initial)")
This commit fixes this by tracking whether pg_subtrans has been
started during recovery, and have the checkpointer check this flag
before truncating pg_subtrans at restartpoints.
Backpatch to all supported versions.
Reported-by: Imran Zaheer <imran.zhir@gmail.com>
Author: Imran Zaheer <imran.zhir@gmail.com>
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA+UBfamzfEReT0VOGRUW_=_AecCYQ-CNKLR_T4Q+YEmJAH3fdg@mail.gmail.com
Backpatch-through: 14
When reduce_outer_joins reduces an outer join to an antijoin, any IS
NULL qual on a Var from the antijoin's nullable side is necessarily
true. Previously, such quals were discarded later in
distribute_qual_to_rels, mainly to avoid bogus selectivity estimates.
But that discard was incomplete: the qual remained in the jointree,
while its Vars were not counted in attr_needed. Since commit
2ebf25e7d, join removal edits the jointree and expects it to contain
no other references to a removed rel, so it could remove a rel that
such a discarded qual still references, and then trip an assertion on
the qual's stale Var.
To fix, move this processing to an earlier phase: such quals are now
removed from the jointree by reduce_outer_joins itself. This way
later phases see a consistent query tree, and
check_redundant_nullability_qual is no longer needed, so remove it.
Back-patch to v16, as with commit 2ebf25e7d.
Reported-by: Tender Wang <tndrwang@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAHewXNk8b0TsSy4dL=CO7FXL2W3WBm0BcdP-zwNJePa-Qj4HzA@mail.gmail.com
Backpatch-through: 16
Several invocations decoded and printed the full WAL range even though
the test only needs to confirm that a command form works or that
decoding reaches a specific error. Limit those checks to one record,
and start the fall-off-the-end checks near the end of the generated WAL.
This change reduces the IPC overhead overall, particularly on Windows,
without reducing coverage.
I am usually hesitant to backpatch such changes as this is only an
improvement, but the gains are too good in terms of IO and runtime, for
both the CI and the buildfarm. Based on the numbers provided, the CI
takes 30% less time to run the test with this change on Windows (worst
case shown on the lists). These tests have been introduced in
96063e2836.
Author: Sehrope Sarkuni <sehrope@jackdb.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAH7T-araSEdsNpxiKaMOW8kr_ZHhekCLHx5yzm2ksq4FdgULbA@mail.gmail.com
Backpatch-through: 17
The VALUES reference page said that an AS clause was required when
VALUES is used in a FROM clause. This was imprecise because AS is
optional when specifying an alias.
In v14 and v15, a table alias is still required in this case, so state
that explicitly instead. In v16 and later, commit bcedd8f5fc made table
aliases for subqueries in FROM clauses optional, so state that the
table alias is optional there. In all branches, continue to recommend
explicit column aliases as good practice.
Backpatch to all supported versions.
Author: Ian Barwick <barwick@gmail.com>
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAB8KJ=jrvZdNLLhYSiCgbjLTz2LKEjYw+-HVQkaF+kgk61KtHA@mail.gmail.com
Backpatch-through: 14
