6ed83d5fa made it so tuplesorts could use a bump.c memory allocator for
non-bounded sorts in order to save memory and increase performance.
That commit didn't get the memory accounting correct for datum sorts and
mistakenly passed 0 bytes to tuplesort_puttuple_common(). That was
correct for byval types, but failed to take into account palloc'd memory
for byref types.
Fix by using datumGetSize() when using the bump allocator and by using
GetMemoryChunkSpace() when not.
Backpatch to v17, where 6ed83d5fa was added.
Author: Mario Karuza <mkaruza.pg@icloud.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Joao Detomini <joao.detomini@enterprisedb.com>
Discussion: https://postgr.es/m/e9f9f54337cd86a982837763958518cdbf08863c.camel%40icloud.com
Backpatch-through: 17
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
When a LATERAL UNION ALL subquery is flattened into an appendrel, a
child EquivalenceMember's expression can contain lateral references to
other relations. create_join_clause() builds a clause from such a
member with make_restrictinfo(), which computes clause_relids from the
expression, and so includes the relids of the lateral references along
with those of any outer joins that null them. Commit 03107b4ed made
sure that clause_relids include the child's relids, but it only ever
added relids.
This is a problem when a lateral reference is nullable. The child is
parameterized by its lateral_relids, which do not include the outer
joins that null the lateral references, so the clause fails the
join_clause_is_movable_into() assertion that
get_baserel_parampathinfo() applies to EC-derived clauses.
To fix, set a child clause's clause_relids from its members' em_relids
rather than from their expressions. This matches how
adjust_appendrel_attrs() computes clause_relids for a child's other
join clauses. The lateral references need not be included, since
every path for the child is parameterized by at least its
lateral_relids. Also update num_base_rels to match, which commit
03107b4ed neglected to do.
Back-patch to v16, where outer-join relids were added to
clause_relids. Before that, the relids of a lateral reference are
always part of the child's parameterization, so the problem does not
arise.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: wenhui qiu <qiuwenhuifx@gmail.com>
Discussion: https://postgr.es/m/CAMbWs4-cRxpQNyQJW=PkDagSDGQ=3Rp2t_fSAyROKsDStjKHUQ@mail.gmail.com
Backpatch-through: 16
Buildfarm members adder and drongo failed to observe the expected relation
lock wait in the startup deadlock test. In adder's failure, autovacuum
pruned dead rows before the test's explicit VACUUM FREEZE, canceling the
standby backend too early and releasing the relation lock wait before the
test could observe it.
Fix this by disabling autovacuum for the test node, leaving the explicit
VACUUM commands in control of when pruning occurs. This also prevents
auto-analyze workers on other relations from holding back xmin and
interfering with the snapshot conflict test.
Backpatch to v17, where this test is enabled.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/421c0aee-84c8-4c07-b4b9-263095479755%40gmail.com
Discussion: https://postgr.es/m/714729ea-f536-0faf-353b-cbd5c8f241ea%40gmail.com
Backpatch-through: 17
Buildfarm members olingo and adder reported failures in
031_recovery_conflict.pl where a recovery conflict was counted twice.
olingo saw the snapshot conflict count reach 2 instead of 1, while adder
saw the same for the buffer pin conflict count.
The startup process can signal a standby backend again while it is
reporting a recovery-conflict FATAL error. Each processed conflict can
increment the corresponding statistic. Thus, even when the expected
conflict occurs, its counter can exceed 1. The existing poll for exactly 1
can then time out, and the exact check of the aggregate count can fail as
well.
Fix this by polling for a positive count for each conflict type and
accepting an aggregate count of at least the expected number.
Backpatch to v17, where this test was enabled.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/421c0aee-84c8-4c07-b4b9-263095479755%40gmail.com
Backpatch-through: 17
On Windows, a backend exiting resets its connection, and the client
discards any data not yet read, including the FATAL message of a
terminated backend. isolationtester polls one connection at a time, so
when it waits on a different session the message can get lost, failing
the permutation and the tests that run after it.
wait_cleanup in injection_points (down to v17) and slot_creation_error
in test_decoding (down to v15) have this pattern, and have been failing
randomly in the buildfarm. Let's skip them on Windows.
The cause of the instabilities is proving to be tricky to deal with,
pointing for example at 29992a6a50. This will be tackled later,
perhaps with a backpatch (or perhaps with a HEAD-only solution), but it
is difficult to be sure at this stage of the work. In theory,
a8458f508a should make a revert of 29992a6a50 (revert of a revert!)
safe on WIN32.
Note that 278cdea6b9, that has introduced the isolation test
slot_creation_error in test_decoding, has mentioned the potential output
instability.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Author: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAN4CZFOoiKa9E4Gnh_OzLk3S5Z6RFyg_oY1e_WddCVYhkpTVow@mail.gmail.com
Backpatch-through: 15
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
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>
