Prior to v18, the total cost of a plan was just one number, a
floating-point value. Commit e222534679
introduced a second component, the number of disabled_nodes at or
below that level of the plan tree. Unfortunately, I (rhaas) failed
to realize that GEQO extracts the cost from each candidate plan and
compares it directly as a measure of fitness.
This means that, at least in principle, it's possible for GEQO to
prefer a plan with more disabled_nodes and lower cost over one with
fewer disabled_nodes and higher cost, which is incorrect. It may be
possible for that to become a practical issue in v18, but it's much
more likely to become an issue in v19 due to pg_plan_advice. Hence,
since the straightforward fix breaks ABI compatibility, back-patch
only to v19.
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
The check_oauth_validator check for an empty validator list didn't test
for a string with only whitespace, which would cause the postmaster to
crash. Instead of testing for the empty string cases, pass the config
value to SplitDirectoriesString unconditionally. If an empty list is
returned then the input string was empty. Since pstrdup cannot handle
NULL, assert that the string ie set. While users cannot set the string
to NULL, it is initialized to NULL so guard against programmer error.
Check the parsed list instead. Assert that the GUC string is non-NULL
before pstrdup(); users cannot set it to NULL, but the C variable is
initialized that way.
This also adds a TAP test that reloads a whitespace-only setting after
pg_hba_file_rules and waits until the existing backend sees the restored
GUC.
Author: Grigorev Jurij <ju.grigorev@ftdata.ru>
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/04fa84f6ebbe400f940e179ebe1070e9@localhost.localdomain
Backpatch-through: 18
The worker connects as the table owner in a new session, so a role- or
database-level lock_timeout and transaction_timeout unhelpfully apply to
it, possibly canceling the wait for older transactions. This cannot be
overridden by the REPACK calling session. Fix by passing the timeout
values from the steering backend via the DecodingWorkerShared struct and
adopting these values in the worker; this way, values from session-local
SET reach the worker too.
In passing, remove a pointless member from the same struct.
Author: Shihao Zhong <zhong950419@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CAGRkXqTYaBjFvjtjPb1+=sZWvt93=2c472bmM+xdG7w9ZvAydA@mail.gmail.com
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
A REPACK background worker can fail to start for various reasons, but
the REPACK steering process was not handling them correctly. For
instance, the steering process waits on a condition variable, but it
would simply hang if the worker doesn't send the signal. (A signal
would be sent by postmaster, but it would wake up the process only to be
ignored and continue sleeping.) Fix this by splitting the wait in two:
first wait for the worker to attach to the error queue, and then wait on
the condition variable as before.
The shutdown sequence can also get stuck, if the message queue gets full
while trying to shut down (this can apparently happen if the log level
is set to very noisy). Previously the worker would send a signal and
then detach; but the steering process can receive that signal, act on
it, then fail to wake up to handle the detach, sleeping indefinitely.
Fix this by having the worker send a Terminate message, which the
steering process can react to.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Author: Shihao Zhong <zhong950419@gmail.com>
Author: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Shihao Zhong <zhong950419@gmail.com>
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Reported-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reported-by: Nikolay Samokhvalov <nik@postgres.ai>
Backpatch-through: 19
Discussion: https://postgr.es/m/CALj2ACVAxA9HxvFe8HSspTJ-UO4Aoz%3DkuQdZBeLrod0gqUxH3g%40mail.gmail.com
Discussion: https://postgr.es/m/apBpOVZOyqrakEr_@nathan
The reference page and the read-your-writes section both presented
WAIT in standby_replay mode as enough for read-your-writes, without
saying which LSN to wait for. Read as written, an LSN taken after the
last statement but before COMMIT would do, and it would not: the standby
position WAIT compares against is the end of the last replayed record, so
a target that falls inside the COMMIT record is satisfied while that
record is still unapplied.
Say that the target must be at or after the end of the write
transaction's COMMIT record, and note in the example that autocommit is
what puts the UPDATE's commit before the call that reads the position.
Author: Sami Imseih <samimseih.pg@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/CAN12%2BYKN7gXb1o9bDWgfA%3D6wjDrqNEgt%2BrXJiAWXgPCXQ5TqpA%40mail.gmail.com
Backpatch-through: 19
The description of NO_THROW covered what the option does but not when an
application would want it. Explain that it suits callers that treat
timeout or not in recovery as expected results to be handled as part of
ordinary control flow, note the obligation to check the returned status,
and give the reason to omit the option.
Be precise about the option's scope: it changes only how those two wait
outcomes are reported. Soften the description's claim that the command
"returns a status string instead of throwing errors" accordingly, and say
that invalid input still raises an error, as does any condition checked
before the wait begins. Name two of those conditions as examples rather
than listing them, since the set grows.
Author: Rithvika Devisetti <devisettirithvika@gmail.com>
Reported-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Satyanarayana Narlapuram <satyanarlapuram@gmail.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/14415b05-16d0-446f-ba7d-c891db8b3406%40eisentraut.org
Backpatch-through: 19
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 pg_publication_rel entry can represent either an explicitly published
relation or a table excluded from a FOR ALL TABLES publication, but the
object address code treated every entry as a published relation without
checking prexcept. EXCEPT entries thus appeared as ordinary
published-table mappings in dependency messages, pg_identify_object(),
and pg_identify_object_as_address().
Report such entries as "publication excluded relation" instead, with
object identities indicating that the table is excluded from the
publication. pg_get_object_address() also accepts this new object type,
so that the output of pg_identify_object_as_address() can still be
passed back to it to look up the same object. "publication relation"
now resolves only non-EXCEPT entries, and "publication excluded
relation" only EXCEPT entries.
Oversight in commit fd366065e0.
Author: Fujii Masao <masao.fujii@gmail.com>
Author: Vignesh C <vignesh21@gmail.com>
Reviewed-by: shveta malik <shveta.malik@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Nisha Moond <nisha.moond412@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwHfESBexa7fq99EvFCf31av=O9h9udnw22ymxmm6LMZzw@mail.gmail.com
Backpatch-through: 19, where it was introduced
Up until now, pg_plan_advice has had a feature that allows an advice
target to mention a partition name but omit the partition schema;
that is, something like SEQ_SCAN(foo/bar) forces a sequential scan on
every child of table foo whose partition name is bar, regardless of
the schema in which bar appears. However, that feature turns out to
have a nasty design flaw: while advice enforcement handles this case
just fine, the advice feedback code doesn't know about it and will
mark such advice as "matched, failed" even when everything worked
perfectly. Unfortunately, there seems to be no simple code fix for
this problem.
Since the release of PostgreSQL 19 is imminent, take the conservative
course and revert this feature of pg_plan_advice. In other words,
require the partition schema whenever the partition name is present.
You must now write e.g. SEQ_SCAN(foo/public.bar) rather than just
SEQ_SCAN(foo/bar). This doesn't affect any cases where automatically
generated advice is supplied, since generated advice has always
included the partition schema anyway. Manually written advice will
have to conform to the new, stricter rule. For a later release, we
can consider whether to again relax this restriction in some way,
but now is not the time to design new things.
Reported-by: Noah Misch <noah@leadboat.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Like in PerformOffsetsTruncation() and e.g. in TruncateSUBTRANS(),
need to step back to previous page at page boundary, to avoid passing
a cutoff page to SimpleLruTruncate() that hasn't been created
yet. Otherwise you can get a bogus 'LOG: could not truncate directory
"pg_multixact/members": apparent wraparound' message in the log.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://www.postgresql.org/message-id/20260827231757.78.noahmisch@microsoft.com
Backpatch-through: 19
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
Previously, during REPACK (CONCURRENTLY), index_rebuild_count in
pg_stat_progress_repack and pg_stat_progress_cluster did not advance
as indexes were rebuilt. This made it impossible for users monitoring the
operation to tell how many indexes had been completed.
Fix this by incrementing the count after each index is built on the new
heap, so that both views report the number of completed index builds.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAHGQGwFUsrBvTurkSU8TEc=DZTunqteUXrasseqNMT7AMBZKRw@mail.gmail.com
Backpatch-through: 19
Previously, when REPACK, CLUSTER, or VACUUM FULL created a transient TOAST
table, index_rebuild_count in pg_stat_progress_repack and
pg_stat_progress_cluster reported 2 before any indexes on the main table
had been rebuilt. This could mislead users monitoring the operation.
This happened because creating the TOAST index writes the CREATE INDEX phase
to the same progress slot used for the index rebuild count of the enclosing
command.
Fix this by passing INDEX_CREATE_SUPPRESS_PROGRESS when creating a TOAST
index, preserving the progress information of the enclosing command and
keeping the count at zero until the main-table indexes are rebuilt.
Author: Adam Lee <adam8157@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/aj4gJQMba0kClQmj@Mac
Backpatch-through: 19
Commit 5f3a49abf7 changed the primary status update message to report the
end of the last inserted WAL record instead of the WAL write position.
However, the protocol documentation still described the WAL position field as
the latest WAL write position, which no longer matches the message sent by
walsender.
Update the description of the WAL position field to indicate the end of the
last inserted WAL record, matching the implementation.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwFn2QfpHDR6cWxd98tqNb5vyguEis8rtqnqidFR36p4QQ@mail.gmail.com
Backpatch-through: 19
Commit 0c5d626961 was confused about which indexes are potentially
problematic to rebuild: we thought only !indisready indexes could fail
to rebuild, and thus would attempt to rebuild !indisvalid ones that are
"ready" (that is, being maintained by DML but not yet completed).
However, that's wrong, because it could be that the validation phase of
such an index failed or was interrupted partway through. So, have
REPACK throw an error with all !indisvalid indexes, not just those
!indisready.
(!indisready implies !indisvalid, so this is an expansion of what
happened with the previous commit.)
Reported-by: shihao zhong <zhong950419@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CAGRkXqRA4YetT8Nu3+1-45+ffTwi2bd1d1eDFME39Y9OPRMytQ@mail.gmail.com
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
List of commits reverted:
8e72d914c5 Add UPDATE/DELETE FOR PORTION OF
b6ccd30d8f Add isolation tests for UPDATE/DELETE FOR PORTION OF
33db6c4baf Fix DELETE/UPDATE FOR PORTION OF with rules
7b22f15a01 Add psql tab completion for FOR PORTION OF clause
7ca8c94296 Fix FOR PORTION OF with non-updatable view columns
993a7aa0e4 Fix cross-leftover pollution in FOR PORTION OF insert triggers
7ac030d5b1 Require UPDATE permission on FOR PORTION OF column
f0aedc7cb0 Small terminology fixes in comments
7d13b03a2e Fix FOR PORTION OF for inheritance children
a272a58b94 Move FOR PORTION OF volatile check into planner
a40fdf6588 Reject child partition FDWs in FOR PORTION OF
bc3ae886a7 Forbid FOR PORTION OF with WHERE CURRENT OF
c8d49ffb00 Forbid generated columns in FOR PORTION OF
5b5e99047a Forbid FOR PORTION OF on views with INSTEAD OF triggers
c58c83ce50 Fix RLS checks for FOR PORTION OF leftover rows
5454fe4a6e Test what BEFORE UPDATE triggers do to FOR PORTION OF
2a9541ddfd Deparse FOR PORTION OF using the range column's current name.
fd92f74b91 Avoid RETURNING side effects for FOR PORTION OF leftovers.
f3a116a26f Enforce WITH CHECK OPTION on DELETE FOR PORTION OF leftovers
c7e34c31e8 Fix error code for null FOR PORTION OF target
938f4f8f34 Fix assertion failures in DELETE FOR PORTION OF tuple routing
f194032168 Resolve untyped parameters in FOR PORTION OF FROM/TO bounds
98f6984e5c Don't evaluate the FOR PORTION OF target under EXPLAIN
plus release notes entries.
Discussion: https://www.postgresql.org/message-id/62bf70cc-dc33-4188-8070-a8c177bb61de%40eisentraut.org
d96f87332b folded COPY FREEZE visibility map updates into
XLOG_HEAP2_MULTI_INSERT records. Unlike the former XLOG_HEAP2_VISIBLE
redo path, multi-insert redo only updated the FSM when the record did
not contain a heap page FPI and the heap page had less than 20% free
space.
This can leave all-frozen pages with missing or stale FSM entries on a
standby. After promotion, vacuum will skip those pages, preventing
their free space from being discovered.
When a multi-insert record sets the page all-frozen, record the heap
page's free space unconditionally.
Reported-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/CAAKRu_bS3nfMJCF2fjE%2Bceb5-t%3D2p_VPMk2PWuVomrOFxg5DGQ%40mail.gmail.com
Backpatch-through: 19
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
