add323da40 consolidated visibility map updates into XLOG_HEAP2_PRUNE
records. It assumed that a record setting the visibility map would
always modify the VM page during replay. That is not true when the
requested bits are already set on the standby.
Primary and standby visibility maps can diverge for several reasons.
This issue was exposed by a CREATE DATABASE ... STRATEGY WAL_LOG bug.
After a standby with an out-of-date VM is promoted, VACUUM may set bits
that were already present on the former primary. When the former primary
later replays that record as a standby, visibilitymap_set() is a no-op.
Restore visibilitymap_set()'s former API, which returns the state of the
VM bits before setting the requested flags. Use that result to set the
VM page LSN only when the visibilitymap_set() updates the page.
Reported-by: Rogers Wang <rogers.ww@qq.com>
Discussion: https://postgr.es/m/tencent_2E870046716FD94285045E96505A2D4E2908%40qq.com
Backpatch-through: 19
During a parallel GIN build, each key is serialized into a GinTuple, a
transient representation used only while sorting. _gin_build_tuple()
lays out the whole tuple (the palloc size, the key memcpy, and the
offset of the posting list) from a local variable holding the real key
length, but then stored that length in GinTuple.keylen, which was
uint16. For a key longer than 65535 bytes, the stored length was thus
silently truncated.
On read-back, GinTupleGetFirst() and _gin_parse_tuple_items()
recompute the posting-list offset from the truncated keylen and land
inside the key data, so ginPostingListDecodeAllSegments() decodes
garbage: an assertion failure with assertions enabled, and a read past
the end of the allocation without, which could in turn lead to a crash
or garbage being written into the index. Only parallel builds are
affected, because only they materialize a GinTuple; a serial build of
the same data succeeds, as index_form_tuple() compresses large keys
before the GinMaxItemSize check.
To fix, widen GinTuple.keylen to Size, which is what VARSIZE_ANY()
returns and what GinBuffer.keylen already uses, and use Size for the
local in _gin_build_tuple() too, which was an int that truncated
VARSIZE_ANY() the same way.
Widening keylen moves GinTuple.data. The key value is accessed in
place in data, so data must be MAXALIGN'ed; before, that was only true
by accident of the field layout. Make that explicit with
alignas(MAXIMUM_ALIGNOF) on data, so that the layout stays correct on
32-bit platforms, where Size is 4 bytes and would otherwise leave data
under-aligned when MAXIMUM_ALIGNOF is 8.
Bug: #19545
Author: Ewan Young <kdbase.hack@gmail.com>
Reported-by: Yuelin Wang <1217816127@qq.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/flat/19545-0f25b7e47351e8fc%40postgresql.org
378a216187 set pd_prune_xid on inserts so that on-access pruning could
later set the pages all-visible. heap_multi_insert() skipped this only
when setting the page all-frozen, but it can insert frozen tuples even
when not setting the page all-frozen. Inserting frozen tuples should not
set pd_prune_xid as they introduce no work for on-access pruning. Fix by
only setting pd_prune_xid when inserting non-frozen tuples.
Reported-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CAAKRu_a02pu7U2S0qPZ5L%2BoWb2m2Zp965sOKCCLnKUAKNo-hfg%40mail.gmail.com
Backpatch-through: 19
Commit 977d865c36 established some rules for mentioning special
values (e.g., -1, 0, an empty string) in GUC descriptions. A few
parameters either neglect to mention their special values or
describe them in a way that doesn't follow those rules. Most of
these parameters were added after the aforementioned commit. This
commit fixes them.
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/aqMkIpRr62cVqtCZ%40nathan
Backpatch-through: 19
The command tag is WAIT, but the reference page called the command
"WAIT FOR" in its title, index entry, psql help name and throughout its
prose, and so did the error messages. FOR is a noise word that makes the
grammar read like English, exactly as INTO does for INSERT and FROM does
for DELETE; neither of those is part of the command's name. Settle on
WAIT everywhere the command is named, and keep WAIT FOR LSN wherever the
syntax itself is being shown.
Rename the reference page's id and file name to match, following
cb2c696b1d: v19 is not released, so the URL is not yet baked into a
supported version, and every other page's id matches its title.
Reported-by: Masahiko Sawada sawada.mshk@gmail.com
Author: Xuneng Zhou xunengzhou@gmail.com
Reviewed-by: Alexander Korotkov aekorotkov@gmail.com
Discussion: https://postgr.es/m/CAD21AoBdtiPTbm7T_aNeDON9JpFG9g%3DJDUP4WU-Y8t8_xqvR5Q%40mail.gmail.com
Backpatch-through: 19
Commit 447aae13b0 accepted TIMEOUT values up to INT64_MAX
milliseconds, but computing the deadline multiplies by 1000, which
caused an int64 overflow. And the command reported a timeout at once
instead of waiting.
Parse the value as an int instead, which simplifies the code and is
consistent with other timeout values such as the statement_timeout GUC.
The specified values are now rounded to the nearest millisecond, so a
positive timeout below half a millisecond becomes zero and waits
indefinitely.
Backpatch to v19, where the TIMEOUT option was introduced.
Reported-by: ChangAo Chen <cca5507@qq.com>
Author: ChangAo Chen <cca5507@qq.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/tencent_86B83240785807077600A1778566B5C12908@qq.com
Backpatch-through: 19
To import attribute statistics, postgres_fdw sent a query wity COLLATE
"C" to remote servers without checking their version. Since COLLATE is
only supported since v9.1, this could break the import against older
servers; fix by modifying postgres_fdw to check their version and if
older, fall back to sampling. (This commit still allows importing only
relation statistics from such an older server.)
Also, expand the documentation to mention that attribute statistics are
stored into the local server without applying the foreign table columns'
n_distinct option.
Oversight in commit 28972b6fc.
Reported-by: Fujii Masao <masao.fujii@gmail.com>
Reported-by: Osama Abdul Qader <osamaabdulqader.cs@gmail.com>
Author: Etsuro Fujita <etsuro.fujita@gmail.com>
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwH8%2BuDTwfohpck9v_9L3nwSWACbmC36JYRyqHd5xQN_OA%40mail.gmail.com
Backpatch-through: 19
contain_context_dependent_node_walker() reports a CaseTestExpr as
context-dependent unless it sits under a simple CaseExpr or the
elemexpr of an ArrayCoerceExpr. A JsonConstructorExpr whose RETURNING
type requires a coercion also carries a CaseTestExpr placeholder in
that coercion, so any SQL function called with such a constructor as
an argument was refused inlining.
Teach the walker that a CaseTestExpr is expected within the coercion
of a JsonConstructorExpr, the same way it already handles the elemexpr
of an ArrayCoerceExpr. This is safe now that eval_const_expressions
no longer lets an enclosing simple CASE clobber that placeholder.
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAMbWs48A=VCFbteTkuCoknO1_0-Cu0aMBT0M07dm7vj1QyixDg@mail.gmail.com
Backpatch-through: 19
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
List of commits reverted:
4881981f92 Add infrastructure for pg_get_*_ddl functions
76e514ebb4 Add pg_get_role_ddl() function
b99fd9fd7f Add pg_get_tablespace_ddl() function
a4f774cf1c Add pg_get_database_ddl() function
6c7bce28c8 Fixups for a4f774cf1c1f108fc02e Fix pfree crash in pg_get_role_ddl() and pg_get_database_ddl().
cda0c4c5d6 Reject invalid databases in pg_get_database_ddl()
79fba6ebab doc: Fix missing role attribute in pg_get_tablespace_ddl() description.
c529ee38b9 Convert ddlutils regression tests to TAP tests.
df1bac400f Fix timezone dependence in test_misc/012_ddlutils.pl
5642a0367c Avoid SIGSEGV in pg_get_database_ddl() on NULL tablespace
2af70e9374 Fix incorrect declarations of variadic pg_get_*_ddl() functions.
d6ed87d198 Use named boolean parameters for pg_get_*_ddl option arguments
plus the corresponding release notes entries.
Discussion: https://postgr.es/m/20260827015242.54.noahmisch@microsoft.com
WaitForLSN() samples the current position before checking whether
recovery is still in progress. If recovery reaches the target and ends
between those operations, the promotion path can compare against the
stale sample and incorrectly return NOT_IN_RECOVERY.
Read the position again after observing that recovery has ended, before
deciding whether promotion reached the target.
Author: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/CABPTF7U0gW5%2B-4oL7-qdML-yerZxUb7ku4QXp7JxCYo0qyJ_Tw%40mail.gmail.com
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Backpatch-through: 19
XLogWrite() can advance the flush position when it finishes a WAL
segment, even if its caller requested only a write. When this happens
while AdvanceXLInsertBuffer() recycles a buffer, primary_flush waiters
are not notified and can sleep until an unrelated flush or checkpoint.
Track such progress with a process-local pending flag and perform the
wakeup after releasing WAL write and insertion locks, keeping
WaitLSNLock acquisition outside the contended WAL path. Add a
deterministic injection-point test that forces this path and inspects
the registration directly in shared memory.
Author: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/CABPTF7U0gW5%2B-4oL7-qdML-yerZxUb7ku4QXp7JxCYo0qyJ_Tw%40mail.gmail.com
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Backpatch-through: 19
A backend waiting for WAL replay can retain locks acquired by earlier
statements. If the startup process needs one of those locks, directly or
through another backend, before reaching the target LSN, a deadlock can
arise: startup waits for the backend to release the lock, while the backend
waits for startup to advance replay. The lock manager records the
backend's held locks, but WAIT FOR LSN does not register its dependency on
replay as a lock wait. The deadlock detector therefore cannot see the
complete cycle. With unlimited standby conflict delays and no other
timeout or cancellation, this deadlock can persist indefinitely.
Write and flush waits are restricted as well. Their positions are floored
by the replay position, so without an active walreceiver the startup
process can be their only source of progress. If a held lock blocks
replay, these waits can form the same cycle: the backend waits for replay
to advance, while replay waits for the backend to release the lock.
Streaming does advance them independently, but only while WAL keeps
arriving. If reception stops before the target is reached, a blocked
startup process cannot restart the walreceiver. It also cannot replay
newer checkpoint records needed to advance restartpoints and recycle WAL,
so continued reception can exhaust available space in pg_wal before the
target is reached. An active receiver at the start of the wait therefore
does not guarantee that the wait can finish while replay remains blocked.
Reject an unsatisfied standby_replay, standby_write, or standby_flush wait
while recovery is active when the backend already holds a granted
heavyweight lock. This conservative restriction covers direct
relation-lock cycles and indirect cycles involving advisory locks. It
also rejects some write and flush waits that an active receiver could
satisfy. Requests whose target is observed as already reached are exempt
from this check, as are primary_flush requests and requests issued after
recovery has ended. Existing snapshot and recovery-state checks still
apply.
Report one of the held locks so the user can find it, using the same
description the deadlock report uses.
Add replay-mode tests for relation and advisory locks and for the
already-reached case, and document the restriction along with the
recommended usage pattern.
Author: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/CABPTF7U0gW5%2B-4oL7-qdML-yerZxUb7ku4QXp7JxCYo0qyJ_Tw%40mail.gmail.com
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Backpatch-through: 19
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
REPACK (CONCURRENTLY) suppresses logical decoding of the changes it
applies to the transient heap. For an update, suppression keeps the
tuple data out of the WAL record, but the record itself is still
written, and decoding turned it into a change carrying neither a new
nor an old tuple. An output plugin that asks for the changes made by
heap rewrites therefore outputs an UPDATE with no data at all,
reported under the name of the table being repacked.
Ignore such records, as decoding already does for inserts. Updates on
catalog relations don't carry the new tuple either, so we ignore them
too. For deletes, REPACK sets XLH_DELETE_NO_LOGICAL instead.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Thom Brown <thom@linux.com>
Reviewed-by: Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAA-aLv7L_-dOuHXjLh0Di66dExdOb=uTOzR=jtrqCmV0Wxyd2Q@mail.gmail.com
Backpatch-through: 19
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.
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
Previously, heap_update() honored TABLE_UPDATE_NO_LOGICAL for the main
tuple but passed a hardcoded 0 to the tuple toaster, so the tuples it
wrote to the TOAST relation were logged for decoding anyway. That was
harmless until commit 28d534e2ae gave updates the flag; the insert
path has propagated its own ever since suppression was introduced for
heap rewrites.
REPACK (CONCURRENTLY), the only user of the flag, relies on it to keep
the changes it applies to the transient heap out of the logical
stream. Logical decoding therefore reassembled the TOAST value and
then dereferenced a new tuple that the suppressed record doesn't
carry, crashing the backend. This is reachable only if an output
plugin asks for the changes made by heap rewrites. In core that is
just test_decoding with include-rewrites.
Fix this by passing HEAP_INSERT_NO_LOGICAL down to the tuple toaster
when TABLE_UPDATE_NO_LOGICAL is set.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Thom Brown <thom@linux.com>
Author: Antonin Houska <ah@cybertec.at>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAA-aLv7L_-dOuHXjLh0Di66dExdOb=uTOzR=jtrqCmV0Wxyd2Q@mail.gmail.com
Backpatch-through: 19
This reverts commit a9c350d9ee.
While that intentionally changed our semantics for CREATE SCHEMA
subcommands, it's being argued that the functionality gain does
not justify potentially-subtle compatibility breakage. The most
useful bit of functionality gain came from commit 404db8f9e
("Execute foreign key constraints in CREATE SCHEMA at the end"),
which we're keeping because it's required by SQL spec and doesn't
seem to create a compatibility hazard by itself.
This is not an exact revert, partly because we're keeping 404db8f9e,
and partly because I kept the API changes that allowed passing down
a ParseState (which allows providing an error cursor for many of the
errors thrown in CREATE SCHEMA).
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20260904180712.cc.noahmisch@microsoft.com
Backpatch-through: 19
This reverts commit d516974840,
along with parts of commit 049b742daa
("psql: Tighten heuristics for BEGIN/END within CREATE SCHEMA").
While there's nothing particularly wrong with d51697484 in itself,
it depends on a9c350d9e ("Don't try to re-order the subcommands of
CREATE SCHEMA"), and concerns have been raised that the compatibility
impact of that outweighs the benefit of allowing more object types
within CREATE SCHEMA.
It's not possible to revert 049b742da verbatim, because the
CVE-2026-6464 patches 3045a25ba ("Teach psql to skip in-line COPY
... FROM STDIN data after a failure") and cf754f741 ("Save/restore
more lexer state when skipping text due to \if") depend on
infrastructure it added. Instead, rip out just the bits specifically
needed to parse CREATE FUNCTION within CREATE SCHEMA. This results in
lexer code that matches v18-and-earlier as modified by CVE-2026-6464.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20260904180712.cc.noahmisch@microsoft.com
Backpatch-through: 19
The replica identity determination was "flawed": it fell back to the
primary key if the marked index was dropped, which is a defensible
choice, but logical decoding has a differing opinion and fails to
provide usable tuple identity data in this case. "Fix" REPACK by
refusing to use the primary key in that case.
This is arguably broken behavior in RelationGetIndexList, but I'll
refrain from changing that at this stage.
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Author: Matthias van de Meent <boekewurm+postgres@gmail.com>
Author: Ewan Young <kdbase.hack@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/apCBRKCH8jwKiaSY@nathan
ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY leaves the partition
marked as detach-pending if its wait is interrupted, and only DETACH
PARTITION ... FINALIZE clears that mark. In that state pg_class still says
relispartition while get_partition_ancestors() already reports nothing,
which RelationBuildPublicationDesc() was not ready for. It took
relispartition to mean that the ancestor list is not empty and asked for
its last element, which is an assertion failure, or a NULL pointer
dereference without assertions. CheckCmdReplicaIdentity() needs the
descriptor for every UPDATE and DELETE of a publishable relation and
builds it on first use, so no publication has to exist for this, and a
plain UPDATE crashes the backend.
Treat such a partition as a standalone table, as after the detach is
finalized. It is then published by FOR ALL TABLES publications, but cannot
be added to an EXCEPT clause until the detach completes, which is now
reported with a hint to run DETACH PARTITION ... FINALIZE.
Oversight in fd366065e0, which added the exclusion.
Author: Mikhail Nikalayeu <mihailnikalayeu@gmail.com>
Author: shveta malik <shveta.malik@gmail.com>
Author: Nisha Moond <nisha.moond412@gmail.com>
Reviewed-by: shveta malik <shveta.malik@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Nisha Moond <nisha.moond412@gmail.com>
Discussion: https://postgr.es/m/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com
Backpatch-through: 19, where it was introduced
The tab-completion query used for ALTER DATABASE ... RESET referenced
pg_db_role_setting and pg_database without schema qualification, so a
same-named relation earlier in search_path could be used instead of the
system catalogs. This could make psql offer misleading RESET candidates.
Commit dbf5a83d46 schema-qualified the unnest() call in this same query,
but the catalog references were still left unqualified. Fix that by
referring to pg_catalog.pg_db_role_setting and pg_catalog.pg_database
explicitly.
Backpatch to 18, where the tab-completion for ALTER DATABASE ... RESET
was introduced.
Bug: #19523
Reported-by: Zhou Digoal <digoal@126.com>
Author: Vismay Tiwari <vismay.t@gmail.com>
Reviewed-by: Paul Kim <mok03127@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/19523-424457118202f570@postgresql.org
Discussion: https://postgr.es/m/CALHMmB84qkCgv3QAR78YawgfqQZCxSPkRhppxzU=e6fg8RA+AA@mail.gmail.com
Backpatch-through: 18
The fast path probes using the referenced index's collation. When that
collation differs from the referenced column's, it can reject valid
references or accept invalid ones. Changing the scan key's collation
is not sufficient, since btree navigation must use the ordering under
which the index was built.
Cache index eligibility lazily after locking the referenced table,
reloading the constraint information, and opening the index. Compare
index and column collations using the held relation descriptors, and
fall back to SPI on a mismatch. Also move the btree eligibility check
here, avoiding a lookup of an index that REINDEX CONCURRENTLY could
drop before the referenced-table lock is acquired.
Cache both acceptance and rejection until the constraint information
is reloaded, so subsequent rows need not repeat the comparisons. Check
before probing a row, and let the caller use SPI if the index is
unsuitable.
Add ICU regression coverage for per-row validation, ordinary inserts,
reordered index columns, and both directions of collation mismatch.
Reported-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAA4eK1Lk1DfgiFPL-HccZm-Nm+exg+FcsODH8KPw6BOBuzfHtA@mail.gmail.com
Backpatch-through: 19
AlterPublicationOptions() can use stale publication state when the
publication is modified concurrently while waiting for the publication
lock. In particular, a concurrent ALTER PUBLICATION ... SET ALL TABLES
can change puballtables, causing the validation to make decisions based
on the old value and leads to assertion failure. Likewise,
CheckAlterPublication() could miss a table added concurrently, letting
SET ALL TABLES EXCEPT (...) leave pg_publication_rel with a mixture of
inclusion and exclusion rows.
Fix by acquiring the lock and re-reading the publication tuple in
AlterPublication() before either path runs, and by calling
CheckAlterPublication() only after that.
Also fix 037_except.pl, which left one computed result untested and
silently reused a subscription created by an earlier test.
Author: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: shveta malik <shveta.malik@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/CALDaNm1r2MkGu6h8zgU1Kj1sX-FcMQ7wGTeLSnhx-5joiyXEvg@mail.gmail.com
Backpatch-through: 19, where it was introduced
Those changes were intended to avoid dependencies on the global libc
locale settings by basing identifier casefolding on the default
locale. They introduced an unintentional behavior difference for the
builtin provider with locale C and a single-byte encoding.
Instead of fixing that edge case, just revert the changes. The
identifier casefolding behavior has always been based on the global
libc locale, and clients make an effort to use the same rules. Until
we have a plan to change that, provider-specific identifier
casefolding doesn't make sense.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260826225510.10.noahmisch@microsoft.com
Backpatch-through: 19
The general nature of this interaction has been understood (at least
by me) since before the original commit, but I failed to explicitly
document it. I also left behind an XXX in the README which I intended
to remove before commit. Fix those things.
Reported-by: Noah Misch <noah@leadboat.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Commit ac58465e06 moved the discussion on clustering from CLUSTER's
refpage to REPACK's, but somehow introduced a mistake regarding when
exactly can the seqscan-and-sort strategy be used with respect to btrees
(spoiler: it was 100% backwards). Fix that, but also restructure and
reword the whole subsection more fully to try and make it more coherent.
Author: Chao Li <li.evan.chao@gmail.com>
Author: Álvaro Herrera <alvherre@kurilemu.de>
Reported-by: Antonin Houska <ah@cybertec.at>
Backpatch-through: 19
Discussion: https://postgr.es/m/69CA7636-0359-42B6-B971-16A036698187@gmail.com
Explain that returning false for TM_SelfModified follows ExecLockRows(),
which skips such tuples. This differs from TM_Updated, which is
unexpected in READ COMMITTED with FIND_LAST_VERSION.
Also remove "fall through to error" from the TM_Updated comment, since
the code calls elog(ERROR) directly.
Reported-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAA4eK1Lk1DfgiFPL-HccZm-Nm+exg+FcsODH8KPw6BOBuzfHtA@mail.gmail.com
Backpatch-through: 19
Commit b7b27eb41a added batching to the direct-index fast path for
foreign key checks introduced by 2da86c1ef9. Instead of probing the
referenced index once per row, it accumulated referencing rows and
checked them in groups, using SK_SEARCHARRAY for single-column foreign
keys.
The batching requires state to survive across trigger invocations and
to be flushed at the end of each trigger-firing cycle. Follow-up work
has had to define how that state interacts with nested trigger firing,
subtransactions, deferred constraints, and SET CONSTRAINTS.
Failure to handle one of those cases can leave a buffered check
unperformed, allowing a transaction to commit a permanent foreign key
violation without reporting an error. With PostgreSQL 19 close to
release, there is not enough time to gain confidence that all relevant
trigger and transaction states have been covered.
Remove the batching and its after-trigger callback infrastructure,
including the per-batch RI cache and associated subtransaction cleanup.
Restore the early exit in AfterTriggerFireDeferred(), since batch
callbacks can no longer queue additional deferred triggers after
afterTriggerInvokeEvents() returns.
Remove tests that exercise only the batching implementation and its
callback and cache lifetime machinery. Retain tests that continue to
exercise the underlying per-row fast path, including validation,
scan-key construction, cross-type rechecks, deferred checks, and
metadata invalidation.
Keep the underlying per-row fast path. It performs each check
synchronously and retains no pending checks across trigger invocations.
Also retain the fixes made to the per-row probe path, including support
for domain-typed referencing columns, restriction to btree referenced
indexes, concurrent index replacement, metadata invalidation, and
nullable referenced keys.
This removal applies only to REL_19_STABLE.
Discussion: https://postgr.es/m/CA+HiwqEDcCSDoYTtoXH4Ta_BpGN+revMZSLLj6GRz2_5ybwE1g@mail.gmail.com
ri_FastPathCheck() acquired its scan snapshot before opening the
referenced relation. If it then waited for the relation lock in READ
COMMITTED mode, a referenced row committed during the wait would not be
visible to the old snapshot. The check could consequently report a
foreign key violation even though the referenced row existed.
The batched path acquires its scan snapshot after opening the relations,
so it does not have the lock-wait problem. The per-row path remains
reachable during constraint validation and checks re-entered from a
batch flush.
Take the snapshot after opening the referenced relation and reloading
the constraint information. This also agrees with the SPI path, which
acquires the referenced-relation lock before selecting the snapshot
used for the check.
Also make the scan snapshot active for the duration of the check, so
STABLE cast and equality functions use the same snapshot as the index
scan. Otherwise, a STABLE cast can miss rows inserted by earlier AFTER
triggers and cause a valid foreign key to be rejected. Add a
regression test comparing this case with the SPI path.
Finally, also advance the command counter after acquiring the
referenced-relation lock and before reloading the constraint
information, so local invalidations are processed under the lock, as in
the SPI path.
Add an injection-point test that pauses a per-row check before locking
the referenced relation, commits the missing referenced row in another
session, and verifies that validation succeeds. Force per-row
validation by using a role without SELECT privilege on the referenced
table, so the test exercises this path even with batching enabled.
Add isolation-test coverage for the visibility of a referenced row
committed after the referencing transaction has executed an earlier
command. A later command can see such a row in READ COMMITTED, but not
in REPEATABLE READ or SERIALIZABLE.
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqEhm+_=bs=2wavAJz-UqC+1KebD31++mapJQQGweE8iQQ@mail.gmail.com
Backpatch-through: 19
Commits 630706ced0 and 9c8de15969 lost some encoding-specific nuances:
in libc with EUC encodings, or in ICU with any non-UTF8 encoding, we
cannot easily classify characters, so we must assume they are
case-varying. Restore these behaviors.
Previously, ILIKE using an index scan could cause wrong results.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260826221526.89.noahmisch@microsoft.com
Backpatch-through: 19
pg_stat_progress_cluster translates REPACK into VACUUM FULL when
pg_stat_progress_repack.repack_index_relid is zero, and into CLUSTER
otherwise.
Previously, repack_index_relid was set only when an index scan was used.
When REPACK used a sequential scan and sort to order the table by an index,
the field remained zero even though index ordering had been requested. As a
result, pg_stat_progress_cluster incorrectly reported the command as
VACUUM FULL instead of CLUSTER.
Fix this by setting repack_index_relid to the OID of the requested ordering
index when progress reporting starts, regardless of the scan method. This
allows pg_stat_progress_cluster to correctly report REPACK as CLUSTER when
index ordering is requested.
Also update the documentation to clarify the meaning of the index columns
and how REPACK is translated into CLUSTER or VACUUM FULL.
Suggested-by: Fujii Masao <masao.fujii@gmail.com>
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://postgr.es/m/CAN4CZFMzy2V_wvRCBW5K8=wVCk1-C7nq=8otXO=u+s_1KZHqyA@mail.gmail.com
Backpatch-through: 19
Previously, when another session ran REPACK, CLUSTER, or VACUUM FULL and
the user lacked permission to view its progress details,
pg_stat_progress_cluster incorrectly reported the command as CLUSTER
instead of NULL.
pg_stat_progress_cluster is based on pg_stat_progress_repack and
translates REPACK into CLUSTER or VACUUM FULL depending on whether index
ordering is requested. However, this translation defaulted to CLUSTER,
incorrectly converting NULL commands returned for insufficient privileges
into CLUSTER.
Fix this translation to preserve a NULL command when the user lacks
permission to view the progress details.
Bump catalog version.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Chao Li <lic@highgo.com>
Discussion: https://postgr.es/m/CAN4CZFMzy2V_wvRCBW5K8=wVCk1-C7nq=8otXO=u+s_1KZHqyA@mail.gmail.com
Backpatch-through: 19
A clause connecting the same two EC members can be requested from the
EC machinery in different contexts. When building parameterized index
paths for a relation, generate_implied_equalities_for_column derives
an indexable clause with parent_ec set, to mark it as redundant with
other join clauses derived from the same EC. Meanwhile, at a join
where the relations of a multi-relation EC member first come together,
generate_join_implied_equalities emits a clause equating the same two
members with parent_ec unset, since that clause is the sole enforcer
of its member there and must not be dropped as redundant. Because
create_join_clause treats parent_ec as part of its cache-lookup key,
such requests can yield two different RestrictInfos for the same
condition, carrying different rinfo_serials.
This breaks the assumption that one condition has exactly one serial
number, which create_nestloop_path relies on to drop join clauses that
are already enforced within a parameterized inner path. As a result,
the same condition could be enforced twice at different plan levels,
wasting execution effort and applying the clause's selectivity twice,
underestimating the join's row count.
To fix, make create_join_clause copy the rinfo_serial from an existing
clause that connects the same two members with the opposite parent_ec
marking, so that the two clauses keep their distinct markings but are
recognized as the same condition.
As a result, a duplicate join filter disappears from one of the
existing regression plans (the "cardinal_number" query in join.sql);
the other plan changes are join-order differences between plans of
essentially equal cost.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAMbWs4-3yZeBMGQDpmYRPdV-sL_=6H2AN=MoeneOT+VUk28PPg@mail.gmail.com
Backpatch-through: 19
Commit f836b688f ensured that at most one clone of a join clause is
chosen for a parameterized path, so long as the clone variants are
parse-tree distinct. Two ways remained to end up enforcing the same
condition more than once, which wastes execution effort and applies
the clause's selectivity multiple times, underestimating the row
count.
If a commuting outer join nulls no Var referenced by a particular
qual, the clone variants of that qual are parse-tree identical, and
the incompatible_relids test cannot tell them apart. To fix, enforce
just the first surviving variant, identified by rinfo_serial.
The other case is in get_joinrel_parampathinfo, where the
clause-recovery pass for dropped EquivalenceClasses queries the EC
machinery with a context that overlaps the preceding loop's, so the
derived-clause cache can hand back a clause that that loop already
accepted. To fix, skip any clause that is already present in the
collected list.
Also add assertions verifying that the clauses to be enforced at a
join or within a parameterized path contain no duplicate rinfo_serial,
to catch any remaining or future violations. These assertions are
added in master only, since hard prevention of duplicate clauses is a
new goal and other unfixed cases may remain.
Back-patch to v19, as with commit f836b688f.
Author: Richard Guo <guofenglinux@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAMbWs4_Ezb1Lnj7BqcTQSysC6B5UFZxfhib8xr6rqpu5uqr8dA@mail.gmail.com
Backpatch-through: 19
Without CHECK_FOR_INTERRUPTS(), backends might not respond to query
cancellations in a timely fashion, especially for JOIN_ORDER()
specifications, where matching against each join can take O(n^2) time
in the length of the join order list in degenerate cases.
Recursive functions should check_stack_depth().
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Backpatch-through: 19
The SIMD path for this command reads ahead via CopyLoadInputBuf()
whenever fewer than sizeof(Vector8) bytes remain in the input
buffer, even if those bytes hold a complete end-of-copy marker.
If the input is a pipe whose writer has sent the marker but not
closed the pipe, that read blocks, and COPY waits for data it will
never use. The scalar loop asks only for the bytes it needs, so
it stops at the marker without reading any further.
To fix, teach CopyLoadInputBuf() to decline a speculative load
when the caller's remaining bytes contain a backslash, which in
text mode might begin such a marker. (CSV mode doesn't treat
\. as special, so it is unaffected.) The SIMD path then hands
those bytes to the scalar loop, as it already does when a load
comes up short. Checking for the marker in the SIMD helper itself
would be more direct, but a call there costs the compiler
registers on every line, which measurably slowed COPY of short
lines in testing.
Oversight in commit e0a3a3fd53.
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CAN55FZ1qFb4Yo3-MWeQfGQnu1_Ksx4xcFC2m3hyw8a--%2BeHQYQ%40mail.gmail.com
Backpatch-through: 19
Commit 86f7c82cf1 reported the user catalog table restriction as a
detail, but the other checks in check_concurrent_repack_requirements()
were still a mix: some were hints, and the details repeated
"REPACK (CONCURRENTLY)", which the primary message already names.
Report them as details with the same "This operation ..." wording. The
one actionable hint, ALTER TABLE ... REPLICA IDENTITY USING INDEX,
stays.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Suggested-by: Alvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/apBIFWzHYOaG0auN%40nathan
Backpatch-through: 19
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
Previously, running REPACK (CONCURRENTLY) on a materialized view failed
with a confusing "has no identity index" error.
REPACK (CONCURRENTLY) replays changes decoded from WAL, but a
materialized view produces none, since it cannot be modified by DML and
REFRESH rewrites it through a transient heap that is not logically
decoded.
Fix this by reporting a clear error for materialized views upfront,
alongside the existing checks for system catalogs and TOAST tables, and
document the limitation.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CALj2ACXPvy4_LkQ0nHKgbZjAGK9fkDqRYUvXA3hgwxPm_X7Eng@mail.gmail.com
