If a proposed join has lateral references to rels outside it,
join_is_legal() checks that those rels can still be joined to from
outside. To do that it collects the rels that must end up on the
inner side of an outer join with the proposed join, directly or
indirectly, and rejects the join if any of its lateral references
point to one of them.
That search followed only outer joins, from min_lefthand to
min_righthand. It failed to notice that a rel that laterally
references the proposed join, or any rel found by the search, cannot
supply a parameter to the proposed join either, and that the same
holds for rels on the inner side of an outer join with such a rel. As
a result, we could approve a join whose lateral references could never
be satisfied.
Such a join can never appear in a complete plan. Worse, as the
existing comment warns, its apparent legality could mislead the
clauseless-join heuristics into thinking that some other join rel need
not be formed. In particular, have_join_order_restriction() would not
force a clauseless outer join if one of its input rels appeared to
have a legal clause join elsewhere, and if that outer join was
required, we would fail with "failed to build any N-way joins".
To fix, also include in the search any input rel of the current join
search that laterally references a rel already found. The input rels
are taken from root->initial_rels rather than from the query's base
rels, because an input can be the joinrel of a sub-joinlist, as
happens for a full join or when from_collapse_limit or
join_collapse_limit prevents flattening. Such a joinrel is joined as
a unit, so it has to be included as a whole.
Back-patch to all supported branches.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: wenhui qiu <qiuwenhuifx@gmail.com>
Discussion: https://postgr.es/m/CAMbWs4_p6iWbsjNcYpd6-6mWFnog3UJ2fZKrxWAQTsyNoyeQTw@mail.gmail.com
Backpatch-through: 14
A WAL sender tracks its WAL reader and logical decoding context as two
static variables, which were left set after being freed for a
START_REPLICATION command. The WAL reader is used for in a physical
replication command, both the WAL reader and the logical decoding
context are used for a logical replication command.
Both variables are now cleared once their contents are released at the
end of each replication command, to keep the release of the resources
and the maintenance of the pointers close to each other. Some more
cleanup is done by WalSndErrorCleanup(), as well, once the WAL reader is
clear.
The assertion added by 879ad9f90e is moved before the execution of a
replication command, checking now that both the WAL reader and the
logical decoding context are never set before the execution of a new
command.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAJTYsWW+hdmmVt_A4hvGv2_5RWK8-kaOf5ncT65=5i_hyD=BHg@mail.gmail.com
Backpatch-through: 14
CheckTablespaceDirectory() did not close the directory opened with
AllocateDir(), leaving its resources held during subsequent recovery,
potentially indefinitely on a standby. Add the missing FreeDir() call.
Backpatch to all supported versions.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/66A784B7-55C1-4613-8C24-B4C2A8D75414@gmail.com
Backpatch-through: 14
Building extended statistics of type "ndistinct" calls
ndistinct_for_combination() once for every combination of columns. Each
call allocates arrays for the whole sample and never frees them, with
memory accumulating until the whole statistics object is built. The
sample has 300 multiplied by target rows. With large statistics targets
(default_statistics_target up to 10k), this can add up to gigabytes
worth of memory, useless once a combination is computed.
ndistinct_for_combination() is now called in a dedicated memory context,
reset after each combination is computed. We only need the estimated
number of distinct values, so there is no need to copy anything out of
the memory context.
Similar fixes have been done in 83772cc78e and ad8a166ca8 for other
areas of the extended stats computations.
Author: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Discussion: https://postgr.es/m/20d6689e-f490-4ca7-a92f-e92f691ba07e@tantorlabs.com
Backpatch-through: 14
extract_jsp_bool_expr applied De Morgan's laws incorrectly when
considering an AND operator underneath a NOT. If one AND input
was implementable as a GIN indexqual while the other was not,
we derived an incomplete indexqual, leading to some rows not
being returned that should be.
extract_jsp_bool_expr also failed to check for methods being
applied to literals, so that for example '$.x == "42".double()'
was treated as '$.x == "42"' not '$.x == 42'. The indexqual
thus selected the wrong rows, all of which would fail qual
recheck, leading to no rows being returned.
While we're here, improve the comments a trifle.
Reported-by: Chinmay Kanchi <cgkanchi@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAJqPDh9Cm=x8eYZ1Jf=_avd2WEqc=0t_f7_SO4igo-CKPi25_Q@mail.gmail.com
Discussion: https://postgr.es/m/CAJqPDh-tu5KY66KU8cvAEg3DQrZLzDZwjkLLa42zD+_Xm1WJAg@mail.gmail.com
Backpatch-through: 14
A hash index scan whose target bucket is still being populated by a
split must read both buckets of the split: forward scans read the bucket
being populated (skipping the tuples that the split copied there) and
then the bucket being split, and backward scans read them in the
opposite order. _hash_readnext and _hash_readprev make the transition
from the first bucket to the second, but _hash_next did not: on reaching
the end of the first bucket, it ended the scan, so every matching tuple
in the second bucket was overlooked.
To fix, teach _hash_next to make the same transition, using helper
functions shared with _hash_readnext and _hash_readprev.
_hash_next only missed the transition when the last page that it read in
the first bucket had a match (otherwise _hash_readpage's loop made the
required transition). Getting wrong answers also needed a matching
tuple to be inserted into the bucket being populated _after_ the split
began. Forward scans then lost the older tuples in the bucket being
split. An incomplete split would keep the buckets in this state until
an inserter finished the split at some indeterminate later time. Note
that wrong answers didn't require an incomplete split.
Although this fix treats the bug as the omission of a required step in
_hash_next, the real problem is the way that page stepping works in
general. The logic for stepping to the next page appears both in
high-level routines like _hash_next and in low-level routines like
_hash_readnext and _hash_readprev. If anything, _hash_next already knew
too much about page stepping (cf. nbtree's _bt_next function). All of
the logic around stepping really belongs in one routine, modeled on
nbtree's _bt_steppage function, but that approach is much too invasive
for a backpatched fix.
Claude Code found this problem. The committed test case is a simplified
version of the one that it wrote to demonstrate this bug.
Oversight in commit 7c75ef57, which introduced page-at-a-time processing
of hash index scans.
Author: Peter Geoghegan <pg@bowt.ie>
Author: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Vignesh C <vignesh21@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=4CJK9ysZzbzxBGGTkgAg9ib8cJyCx=9q_d1r8tf0ang@mail.gmail.com
Backpatch-through: 14
_bt_pagedel only builds a search stack when it needs one to locate the
downlink of the page that it's about to mark half-dead (and when it
doesn't already have one from an earlier sibling deletion that took
place during the same _bt_pagedel call). _bt_pagedel won't build a
search stack for the page its caller passes whenever it happens to be
one that an earlier interrupted VACUUM left half-dead (since there is no
downlink to remove). We nevertheless asserted that page deletion could
only build a search stack for that initial caller-passed page.
To fix, remove the assertion, which spuriously failed when a half-dead
leaf page was passed by the _bt_pagedel caller and its right sibling
page was empty/had to be considered for deletion during the same call
(necessitating building a search stack to locate the right sibling's
downlink).
Oversight in commit cd8c73a38, which refactored nbtree page deletion's
INCOMPLETE_SPLIT check.
Author: Peter Geoghegan <pg@bowt.ie>
Reported-by: Iliia Khaprov <i.khaprov@gmail.com>
Discussion: https://postgr.es/m/CAFO+O_p5So4MXwQ1VEcL9+9vGgODu11TTWcYq9RDbxnira8o2w@mail.gmail.com
Backpatch-through: 14
As of a32b8dc98 and friends, amcheck will complain about bpchar
indexes that claim to allow de-duplication. This causes our
cross-version-upgrade tests to fail when upgrading from a version
that lacks those commits, since there are some such indexes in the
core regression tests. Drop such indexes from the upgrade source
database. (oidvector indexes would be problematic too, but
there aren't any user-defined indexes on oidvector in our tests.)
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAH2-Wzk-oO3HjLaByo8Vpse6Tpha1mqAW8DFShh-kS-Wo59ugA@mail.gmail.com
Backpatch-through: 14
bpchar equality ignores trailing spaces, so equal values in a column
with no length specifier can have different images. Similarly,
oidvector equality intentionally pays no attention to the array lower
bound field. We nevertheless allowed nbtree deduplication to be used
with indexes built with bpchar_ops, bpchar_pattern_ops, or
oidvector_ops. A posting list split could therefore place a TID in an
index tuple whose key was equal to, but not bytewise identical to, the
corresponding table value. This allowed index-only scans to return
subtly wrong results.
To fix, drop support function 4 from the affected opclasses (we can't
change catalog contents on branches 14 through 18, so we just disable
deduplication by adding hard-coding to the relevant support functions).
Deliberately leave the oidvector opclass alone on branches 14 through
18, though. Disallowing oidvector deduplication on these branches would
make pg_amcheck report that pg_proc_proname_args_nsp_index has a corrupt
metapage, needlessly alarming users.
System catalog oidvector values always have a lower bound of 0, so the
issue can't cause harm there; a user can only hit it by indexing an
oidvector column that stores the output of a function like trim_array().
It seems very unlikely that any user would ever do that.
Users should REINDEX any B-Tree index on a bpchar column.
Author: Shihao Zhong <zhong950419@gmail.com>
Co-authored-by: Peter Geoghegan <pg@bowt.ie>
Reported-by: Ke <kehan5800@gmail.com>
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Bug: #19749
Discussion: https://postgr.es/m/19749-d46adf7c7d910b07@postgresql.org
Discussion: https://postgr.es/m/CAGRkXqQQ311Lg=62x5UdZtPuRFgpT6XpPMw4X9E3tgbTu162Mw@mail.gmail.com
Backpatch-through: 14
seg.sgml claims that a seg can store 7 significant digits, but this
is nonsense. The underlying float4 data type is not (quite) that
precise, and seg_out() intentionally rounds off to at most FLT_DIG
decimal places. With IEEE float arithmetic, FLT_DIG will be defined
as 6, or at least that's the case on all platforms I know of.
AFAICT this thinko goes clear back to the introduction of seg
(commit a3694b420), but it wasn't questioned until now.
Bug: #19744
Reported-by: Ke <kehan5800@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19744-8c700582bbb1d075@postgresql.org
Backpatch-through: 14
COPY FROM runs its WHERE clause through eval_const_expressions(),
which among other things replaces NOT above an operator with the
operator's negator, if it has one (e.g., NOT (a > 0) becomes
a <= 0). The node it builds for the negator has no opfuncid, and
since COPY FROM never calls fix_opfuncids() on the clause, nothing
fills it in before the executor needs it. So
COPY tab FROM stdin WHERE NOT (a > 0)
fails with "cache lookup failed for function 0".
To fix, call fix_opfuncids() on the clause, as the relcache does
for index predicates.
Oversight in commit 31f3817402.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/asPZRTci4oBvxR9A%40nathan
Backpatch-through: 14
pull_up_simple_subquery() rechecks is_simple_subquery() after pulling
up the subquery's own subqueries, but it flattened join alias Vars in
the subquery's targetlist only after that, and never in its quals. A
join alias Var within the subquery, such as a whole-row Var of a join,
can expand to an expression containing lateral references to the outer
query once the join's inputs have been pulled up, and until it is
flattened is_simple_subquery() cannot see them. We could thus pull up
a LATERAL subquery whose targetlist or quals reference rels outside
the lowest outer join above it. This leads to assertion failures in
distribute_qual_to_rels() or "wrong phnullingrels" errors, and in some
cases to wrong query results, where rows that should have been
preserved by an outer join above the subquery are discarded.
To fix, flatten join alias Vars in the targetlist, and in the jointree
quals if the subquery is LATERAL, before the recheck. If the recheck
fails, the flattened copy is discarded along with the rest of the
modified subquery, so no harm is done.
Back-patch to all supported branches, as this can lead to wrong query
results.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tatsuya Kawata <kawatatatsuya0913@gmail.com>
Discussion: https://postgr.es/m/CAMbWs48GFZ=3Bjc1ug9JRsB0Qupg+CGcb9js5vmTM6kY2OGWOg@mail.gmail.com
Backpatch-through: 14
pg_blocking_pids() assumes that members of the same lock group
never block each other, but that hasn't been true for relation
extension locks since v13. This commit teaches the function to
report lock group members as blockers for relation extension locks.
Since parallel workers are reported with their leader's PID, the
result may now include the blocked session's own PID.
Oversight in commit 85f6b49c2c.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Shihao Zhong <zhong950419@gmail.com>
Discussion: https://postgr.es/m/apTpV8h%2BCYHf%2B1PJ%40bdtpg
Backpatch-through: 14
It's a bad idea to sleep while holding locks, because you might block
another process that wants to acquire the same lock. Furthermore,
it's not safe to call ProcessConfigFile() in a critical section or
while holding locks. So avoid calling vacuum_delay_point() while
holding locks, and also make vacuum_delay_point() return quickly
without sleeping if it's called while interrupts cannot be processed.
To find the vacuum_delay_point() calls that were made while holding
locks, I added "Assert(INTERRUPTS_CAN_BE_PROCESSED())" in
vacuum_delay_point() and ran the regression tests. I didn't include
that Assert in this commit, because there might be more places that do
that that are not covered by the regression tests, including
extensions.
In ginInsertCleanup(), there was already another vacuum_delay_point()
call later in the loop, while not holding any locks, so just remove
the other call that was made while holding the lock.
In hashbucketcleanup(), move the vacuum_delay_point() call up the
stack to its caller. The call in hashbucketcleanup() was not able to
handle interrupts because it held a lock, and because there were no
vacuum_delay_point() or CHECK_FOR_INTERRUPTS() calls in the caller's
loop, the whole hash index vacuuming phase was uninterruptible by
pending shutdown or query cancel. Now it can be interrupted between
buckets. Unfortunately, hashbucketcleanup() has to process all the
bucket's pages in one go without pausing; fixing that would require
changing how hashbucketcleanup()'s lock chaining works.
In acquire_sample_rows(), move the vacuum_delay_point() call in the
loop to between pages, to a time where we're not holding the buffer
lock.
In master, also remove a misleading CHECK_FOR_INTERRUPTS() call from
pgstat_write_statsfile(). It was a no-op because we're always holding
a lock on the current dshash partition at that point. A call that
does nothing is harmless, but it makes you think that the loop is
interruptible when in reality it's not.
Author: Kevin Rocker <me@kevinrocker.com>
Author: Andrey Borodin <amborodin@acm.org>
Author: Mostafa <mostafa.nabil.nafie@gmail.com>
Reported-by: Greg Burd <greg@burd.me>
Reported-by: Sergei Kornilov <sk@zsrv.org>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Neil Chen <carpenter.nail.cz@gmail.com>
Tested-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/19628-c2b17d358181a1ea@postgresql.org
Discussion: https://postgr.es/m/492c6247-43d3-477b-8981-fb0c56767b38%40app.fastmail.com
Backpatch-through: 14
preprocess_expression skips flatten_join_alias_vars for TABLESAMPLE
clauses, on the assumption that they cannot contain Vars of the
current query level. That's not true once a LATERAL subquery
containing a sampled relation has been pulled up: the relation's
TABLESAMPLE clause then becomes a lateral reference of this level, and
if it referred to a join alias Var, that Var is left unexpanded.
extract_lateral_references then pulls the join alias Var out as a
lateral reference and fails with "no relation entry for relid N".
To fix, treat such clauses like lateral function and VALUES RTEs:
introduce EXPRKIND_TABLESAMPLE_LATERAL, used for the TABLESAMPLE
clause of a lateral relation RTE, and keep skipping the alias
expansion only for the non-lateral kind.
Back-patch to all supported branches, as this dates back to the
introduction of TABLESAMPLE.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAMbWs4834TwOtyyycyvja3MuOBkfpXy3dFAL0zt_vcM1KtqV-Q@mail.gmail.com
Backpatch-through: 14
If a subquery has been flattened into its parent, the joinaliasvars
entries of a join above it can be arbitrary expressions rather than
plain Vars, so expanding a reference to such a join alias may insert a
SubLink into a lower-level subquery. flatten_join_alias_vars_mutator
detects that and sets the subquery's hasSubLinks flag, but only in the
single-column code path; the whole-row path just asserted in a comment
that its recursive call would handle this, which is true only when the
alias entry is itself a Var referencing another join.
Hence a whole-row reference to such a join appearing in a sub-select
left that sub-select's hasSubLinks false, so preprocess_expression
skipped SS_process_sublinks for it, and the unprocessed SubLink
reached code that is not prepared for one: this produced "cannot
handle unplanned sub-select" from cost_qual_eval, an assertion failure
in preprocess_aggrefs, or "unrecognized node type" at execution,
depending on where in the sub-select the SubLink ended up.
To fix, make the same check in the whole-row expansion path.
Back-patch to all supported branches.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49PgeNFhQTZ2gSzwatF0_LXyegMPGt++jSfxUN7NZuQ5Q@mail.gmail.com
Backpatch-through: 14
In an in-tree build, "make coverage-html" invoked lcov with two
"-d ." options, because $(srcdir) is just ".". With lcov 2.0
(and perhaps earlier versions?) that results in scanning every
foo.gcda file twice and producing counts twice what they should
be. With later lcov versions (at least 2.3), it results in
"duplicate file" errors.
In VPATH builds we do need two -d options to cover both regular
and generated .c files. But in in-tree builds it suffices to
specify "-d .", so suppress the second switch.
Author: Narek Galstyan <narek.galstyan@enterprisedb.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAHsn6_xCDQWe8_vVFhtFk27_xTdyVV+Dr0yWzaooBZ6+-VH-5w@mail.gmail.com
Backpatch-through: 14
dumpIndex() emits ALTER INDEX .. SET STATISTICS for per-column
statistics targets, but dumpConstraint() never did. Targets set on the
expression columns of an exclusion constraint's index were silently
lost on dump/restore and pg_upgrade.
The same code can be used in the index and constraint dump code to
generate these ALTER INDEX commands. The logic has been refactored so
that both share a new subroutine in charge of generating all these
custom SET STATISTICS, if any.
Oversight in e4fca461ab.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Andrew Krylosov <krylosov.andrew@gmail.com>
Discussion: https://postgr.es/m/CAN4CZFMd9riOuV5LkM+uV=uF-HYp49C8Fh22xWyMxptGLBCZ3A@mail.gmail.com
Backpatch-through: 14
FetchStatementTargetList() dereferenced the portal named by a
FetchStmt without checking that it still exists. With the extended
query protocol, a client can sent a Parse "FETCH n FROM c" while cursor
"c" is open (which caches a result's TupleDesc), issue a CLOSE on cursor
"c", and then Describe the prepared statement or a portal bound to it.
Describe would re-resolve the target list through the named cursor, and
crash. In a non-assert builds, this causes a NULL pointer dereference.
An assertion was triggered in assert builds slightly before the pointer
dereference.
This commit changes the describe of a FetchStmt to match with the
ExecuteStmt case: if the named cursor's portal is invalid, fail rather
than assume that the portal should always exist.
A test case is added to libpq_pipeline, backpatchable all the way down.
Author: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/7DE248EB-7509-4F8A-9F37-4694A40BBAB7@gmail.com
Backpatch-through: 14
There have been several recent complaints that << and >> don't behave
the same way on all supported platforms. There is a risk that <<
overflows the signed integer type, which is undefined behavior, and there
are platform-dependent behaviors from shifting by numbers of bits >= the
bitwidth of the type, and also from shifting by negative values.
Document this to warn unsuspecting users of these facts.
Author: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAApHDvr0O6ZF8NO=Rq9Q+Sz=FHT=NZ1kkNTXZDcNph-OWuk7TQ@mail.gmail.com
Backpatch-through: 14
Canada's Northwest Territories will move to permanent UTC-06 on
2026-11-01. For now, assume their TZ abbreviation will be CST from
that time forward. This affects only America/Inuvik as the rest of
the territory is covered by America/Edmonton, for which the
equivalent change was released in 2026c.
Manitoba (America/Winnipeg) will move to permanent UTC-05 on
2026-10-31. For now, assume their TZ abbreviation will be EST
from that time forward.
(Upstream notes that more changes may be needed soon if smaller
jurisdictions choose to break away from what these have done,
but for now the plan is wait-and-see.)
Historical corrections for Colombia, Iran, and Ireland.
The backward-compatibility zone names EST5EDT, CST6CDT, MST7MDT, and
PST8PDT now conform better to POSIX. For example, EST5EDT now always
uses the abbreviation "EST" for standard time (now always 5 hours
behind UTC) and "EDT" for daylight saving time, whereas it formerly
had different UT offsets before standard time was introduced and
sometimes used abbreviations like "LMT", "EWT" and "EPT", all contrary
to POSIX. Also, though not specified by POSIX these names now use US
federal rules rather than rules of specific places like New York,
reverting to 2024a behavior. This change affects only timestamps
before 1966-10-30 at 01:00 standard time.
Backpatch-through: 14
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
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>
Discussion: https://postgr.es/m/6b50233fcc0f7d6f4845f083777fa9702d9612e4.camel@j-davis.com
Backpatch-through: 14
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.
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
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
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.
Reinit was already documented in a comment as dead code, and it was
removed in a later branch. To minimize the surgery there, just add an
unused attribute to silence the warning.
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
