BackendKeyData length was increased from 4 bytes to a variable-length
length (up to 256 bytes) in a460251f0a. However, pqTrace still traces
it as a 4 bytes key, leading to a "mismatched message length" warning
message. The same issue impacts the tracing of CancelRequest.
This patch fixes the issue by using pqTraceOutputNchar instead of
pqTraceOutputInt32 in both cases.
Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Discussion: https://www.postgresql.org/message-id/CAO6_Xqo6gTv9=76H=k2qDRFU+KHuBiY2S=bQynEr6J8gS7L6xA@mail.gmail.com
Backpatch-through: 18
Commit fb421231da extended \df to include procedures, but its tab
completion continued not to show procedures.
Update \df tab completion to include procedures as well.
Backpatch to all supported versions.
Author: Erik Wienhold <ewie@ewie.name>
Reviewed-by: Surya Poondla <suryapoondla4@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/10fbfdfe-80f6-4ef9-b8b3-f7be0eb53a50@ewie.name
Backpatch-through: 14
Previously, MultiXactId wraparound hints suggested dropping stale
replication slots. While that advice is appropriate for transaction ID
wraparound, where replication slots can hold back XID horizons,
it was misleading for MultiXactId wraparound. Following it could lead
users to drop replication slots unnecessarily without helping resolve
the MultiXactId wraparound condition.
MultiXact cleanup is not directly delayed by replication slots.
Instead, it depends on whether old MultiXactIds can still be seen
as live by running transactions.
This commit removes the replication slot advice from MultiXactId
wraparound hints, and documents that stale replication slots are
normally not relevant to resolving MultiXactId wraparound problems.
Backpatch to all supported branches.
BUG #18876
Reported-by: Haruka Takatsuka <harukat@sraoss.co.jp>
Author: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/18876-0d0b53bad5a1f4c1@postgresql.org
Backpatch-through: 14
The jsonpath .decimal(precision[, scale]) method built its numeric
typmod by calling numerictypmodin() through DirectFunctionCall1(), which
can throw a hard error for an incorrect set of precision and/or scale
vaulues. This breaks the silent mode supported by this function, that
should not fail.
Most of the jsonpath code uses the soft error reporting to bypass
errors, which is what this fix does by avoiding a direct use of
numerictypmodin(). Its code is refactored to use a new routine called
make_numeric_typmod_safe(), able to take an error context in input.
numerictypmodin() sets no context, mapping to its previous behavior.
The jsonpath code sets or not a context depending on the use of the
silent mode. This result leads to some nice simplifications:
numerictypmodin() feeds on an array, we can now pass directly values for
the scale and precision.
Oversight in 66ea94e8e6.
Author: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/CAON2xHMaigKABiyPBBq3Sjd3gp7uWMJXnnMHt=s85V1ij3KP1w@mail.gmail.com
Backpatch-through: 17
The float4 and float8 btree_gist opclasses compared keys with raw C
operators (==, <, >). IEEE 754 makes every comparison involving NaN
false, so GiST disagreed with the regular float comparison operators
and with the btree opclass, which uses float[4|8]_cmp_internal()
(so that all NaNs are equal and NaN sorts after every non-NaN value).
In addition, the penalty and distance functions were not careful
about NaNs, and the penalty functions could also misbehave for IEEE
infinities. Wrong answers from the penalty functions would probably
do no more than make the index non-optimal, but the distance mistakes
were visible from SQL.
To fix, make the comparison functions rely on the same NaN-aware
comparison functions the core code uses, and rewrite the penalty
and distance functions to follow the rules that NaNs are equal
but maximally far away from non-NaNs. The penalty_num() code was
formerly shared between integral and float cases, but I chose to make
two copies so that the integral cases are not saddled with the extra
logic for NaNs and infinities/overflows. I also rewrote it as static
inline functions instead of an unreadable and uncommented macro.
The float penalty functions were previously unreached by the
regression tests, so add new test cases to exercise them.
There's no on-disk format change, but users who have NaN entries
in a btree_gist index would be well advised to reindex it.
Bug: #19501
Bug: #19524
Reported-by: Man Zeng <zengman@halodbtech.com>
Reported-by: Yuelin Wang <3020001251@tju.edu.cn>
Author: Bill Kim <billkimjh@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19501-3bff3bbc97f1e7c9@postgresql.org
Discussion: https://postgr.es/m/19524-9559d302c8455664@postgresql.org
Discussion: https://postgr.es/m/CAMQXxcgbtD2LXfX0tpgvOizxP-XxrCHV2ZDy4By_TZnJMsxXWQ@mail.gmail.com
Backpatch-through: 14
Previously, if a base backup failed after it had started streaming
files, pg_stat_progress_basebackup could continue to show a stale
progress entry even though the backup was no longer running. This could
be observed when the client kept the replication connection open after
the error. It is normally not observable when using pg_basebackup,
because the client disconnects after the error.
The problem was that progress reporting was cleared only after
successful completion.
This commit moves the progress reporting cleanup into the progress
sink's cleanup callback so that it is cleared after both successful
and failed backups.
Backpatch to v15. v14 has the same issue, but the fix does not apply
cleanly because it lacks the base backup sink infrastructure. Since
the bug does not affect the backup itself and is normally not
observable when using pg_basebackup, skip the v14 backpatch.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/EA1A6CD2-EFA6-462B-9A02-03003555AB4A@gmail.com
Backpatch-through: 15
off_t is most likely a 64-bit integer, so casting it to a 32-bit type
for output could lose data. There are more issues like this in the
tree, but this is an instance where this could actually happen in
practice, since base backups are routinely larger than 4 GB. So this
is separated out as a bug fix.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/20ce62fa-47fc-457b-b504-12f3c1651726%40eisentraut.org
Commit 4035cd5d4 added LZ4 compression for full-page writes in WAL, and
retained "on" as a backward-compatible way to specify the builtin PGLZ
method. Document this meaning of "on" and update postgresql.conf.sample
to make the equivalence clear.
Author: Christoph Berg <myon@debian.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/akJDHRtXwGLTppsQ@msg.df7cb.de
Backpatch-through: 15
Previously, if an unlogged sequence was created on the primary and
replicated to a standby, reading the sequence after promoting the
standby (for example, with nextval()) could trigger the following
assertion failure:
TRAP: failed Assert("((const PageHeaderData *) page)->pd_special >= SizeOfPageHeaderData")
In non-assert builds, the same operation could instead fail with an
error such as:
ERROR: bad magic number in sequence
The problem was that seq_redo() updated the init fork page in shared
buffers but did not flush it to disk. During promotion,
ResetUnloggedRelations() recreates the main fork of unlogged
relations by copying the init fork from disk, bypassing shared
buffers. As a result, the main fork could be recreated from a stale
init fork instead of the WAL-replayed page.
Fix this by introducing a helper to flush init fork buffers
immediately, and make seq_redo() use it. As a result, the main fork
of an unlogged sequence is recreated from the up-to-date init fork on
disk, allowing the unlogged sequence to be read successfully after
standby promotion.
Backpatch to v15, where unlogged sequences were introduced.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwH1Ssze3XM6wjoTjSLVOR041c6xP+vsdLP951=w8oG8bA@mail.gmail.com
Backpatch-through: 15
Treat copy_file_range() return value of zero as an error: it indicates
that no bytes could be copied (perhaps the source file is shorter than
expected), and the existing retry loop would otherwise spin forever
since nwritten would never reach BLCKSZ.
The other uses of copy_file_range() in the tree don't have this
problem.
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Yingying Chen <cyy9255@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/3208cf7a-c7f3-41eb-92f6-33cbeff4df40%40eisentraut.org
PL/Python and its hstore and jsonb transforms build SQL values from
Python containers by calling Python C API functions that can return
NULL, and in several places the result was used without first checking
it.
On the sequence side, PySequence_GetItem() is used when converting a
returned sequence into a SQL array or composite value, when reading
the argument list passed to plpy.execute() or plpy.cursor(), and when
reading the list of type names given to plpy.prepare(). On the
mapping side, the hstore and jsonb transforms call PyMapping_Size()
and PyMapping_Items() and then index the result with PyList_GetItem()
and PyTuple_GetItem().
All of these return NULL (or -1), with a Python exception set, for a
broken object: for example one whose __getitem__() or items() raises,
or which reports a length that disagrees with what it actually yields.
The unchecked result was then dereferenced, crashing the backend.
Fix this by checking the result of each call and reporting a regular
error if it failed, so that the underlying Python exception is
surfaced instead of taking down the session.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49BKM9wP6m8bCXEpHwQKp7usvOGV6Jf=J7FYr_BCpxLqg@mail.gmail.com
Backpatch-through: 14
If a "char" column has a statistics histogram, scalarineqsel()
would fail with "cache lookup failed for collation 0". Avoid
the failing lookup by acting as though the collation is "C".
Prior to commit 06421b084, this code didn't fail because
lc_collate_is_c() intentionally didn't spit up on InvalidOid.
It did act differently though: it would take the non-C-collation
code path and hence apply strxfrm using libc's prevailing locale.
But that seems like the wrong thing for a non-collatable comparison,
so let's not resurrect that aspect.
Author: Feng Wu <wufengwufengwufeng@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CACK3muq6s-O1Wc3w4dRL1Fe8YQ-Fz1zJbezeQwhuLgNxGNEFiA@mail.gmail.com
Backpatch-through: 18
The read stream callback apw_read_stream_next_block() advances p->pos
through the block_info array. When processing the last block, it
increments p->pos to prewarm_stop_idx before returning. The callback
itself is safe because it checks bounds before accessing the array.
However, the caller assigned blk from block_info[i] at the end of the
loop body, before the loop condition was re-evaluated. When i equaled
prewarm_stop_idx, this accessed memory beyond the allocated DSM segment,
causing a segfault.
Restructure the loop to check bounds at the top and assign blk at the
beginning of the loop body, where it is always safe. This avoids the
need for an explicit bounds check at the end.
Backpatch to 18, where the bug was introduced by commit 6acab8bdbc.
Author: Matheus Alcantara <mths.dev@pm.me>
Reported-by: Glauber Batista <glauberrbatista@gmail.com>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Backpatch-through: 18
Discussion: https://www.postgresql.org/message-id/CAO%2B_mTQgQyTYwDh%3DU8iTnsDmOGyWsZJjUV31SmEYwmw6_xY6Bw%40mail.gmail.com
When compiling a DECLARE section containing a union nested
inside a struct, ecpg passes a null value for struct_sizeof to
ECPGmake_struct_type. I (tgl) didn't foresee that case in
commit 0e6060790, and wrote an unprotected mm_strdup() call.
Reported-by: iMSA (via Jehan-Guillaume de Rorthais <jgdr@dalibo.com>)
Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20260625114849.34b2148e@karst
Backpatch-through: 18
Commit 1ef917e3a6 fixed the re-indexing of ModifyTable's FDW arrays
when initial runtime pruning removes result relations, but it did so
by adding a new mt_fdwPrivLists field to ModifyTableState. Although
the field was placed at the end of the struct to keep the offsets of
existing fields stable, it still enlarges sizeof(ModifyTableState),
which the ABI compliance check flags on the buildfarm (e.g. crake).
The field existed only so that show_modifytable_info() could recover the
re-indexed fdw_private after executor startup; the executor-side fix in
ExecInitModifyTable() that actually prevents the crash does not depend on
it. Remove the field and have show_modifytable_info() instead look up
each kept relation's fdw_private from the original, pre-pruning
node->fdwPrivLists, which is parallel to node->resultRelations and left
intact by pruning. When nothing was pruned the lookup is a direct index;
otherwise it matches on the range table index.
This is applied to REL_18 only; master keeps the mt_fdwPrivLists field
and is unaffected, so the two diverge slightly here.
Reported on the buildfarm (member crake).
Per a suggestion from Tom Lane.
Reviewed-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqEhe7-v5Q0-oOoW3RaO4voYcGK-JfinbYEWXwutDGSOtQ@mail.gmail.com
In get_perl_array_ref(), for a PostgreSQL::InServer::ARRAY object, we
look up its "array" key with hv_fetch_string() and then inspect the
returned SV. However, hv_fetch_string() returns a NULL pointer when
the key is absent, and the code dereferenced that result without first
checking whether the pointer itself was NULL. As a result, a plperl
function returning a forged PostgreSQL::InServer::ARRAY object that
lacks the "array" key would crash the backend with a segmentation
fault.
Fix this by checking the pointer returned by hv_fetch_string() before
dereferencing it, matching how other callers in this file already
guard the result. With the check in place, such an object falls
through to the existing error report instead of crashing.
Author: Xing Guo <higuoxing@gmail.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CACpMh+DYgcnqZwQLXXuxQcehJTd7T8UmKWSLsK4mFBEp9G2ajA@mail.gmail.com
Backpatch-through: 14
ExecInitModifyTable() rebuilds the per-result-relation lists after
dropping result relations removed by initial runtime pruning. The
re-indexing was done for withCheckOptionLists, returningLists,
updateColnosLists, mergeActionLists and mergeJoinConditions, but
fdwPrivLists and fdwDirectModifyPlans were missed. As a result, a
kept foreign result relation could be handed the wrong fdw_private,
or ri_usesFdwDirectModify could be set from the wrong plan index,
leading to wrong behavior or a crash in BeginForeignModify() and in
the direct-modify path.
show_modifytable_info() had the same problem: it indexed the
plan-ordered node->fdwPrivLists with the post-pruning executor
position, so once initial pruning removed a result relation it
could read a different relation's fdw_private (often a NIL entry),
producing wrong EXPLAIN output or a crash.
Fix by re-indexing fdwPrivLists and fdwDirectModifyPlans alongside
the other lists, saving the re-indexed private lists in
ModifyTableState.mt_fdwPrivLists and reading from there in both
nodeModifyTable.c and explain.c.
Reported-by: Chi Zhang <798604270@qq.com>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Author: Rafia Sabih <rafia.pghackers@gmail.com>
Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com>
Reviewed-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Discussion: https://postgr.es/m/19484-a3cb82c8cde3c8fa%40postgresql.org
Backpatch-through: 18
When ALTER TABLE ... ATTACH PARTITION matches partition indexes to the
parent table's indexes, invalid indexes are skipped. This commit
improves the documentation to describe what e90e9275f5 has changed:
invalid indexes are skipped, and only valid indexes are considered for a
match.
Author: Mohamed Ali <moali.pg@gmail.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/CAGnOmWpAMaE-BOkpwM6mJnHcpS2QZ8yLSSaqmz+vryEsbCWWWA@mail.gmail.com
Backpatch-through: 14
This routine acts as a wrapper of a new pgstat_drop_entry_ext(), used in
the core code with a missing_ok argument.
This includes an update of .abi-compliance-history, removing the latest
entry that has documented the change of pgstat_drop_entry(). This
change is applied across v15~v18. HEAD keeps pgstat_drop_entry() as
single entry point, with the new missing_ok.
Per discussion with Álvaro Herrera and Lukas Fittl. This is a follow-up
of 850b9218c8.
Discussion: https://postgr.es/m/ajZz_sVJVX7pmPHo@alvherre.pgsql
Backpatch-through: 15-18
This function called the resource-kind-specific ReleaseResource()
method for each item before deleting that item from the resowner.
That's backwards from the ordering in ResourceOwnerReleaseAllOfKind,
and it's not very safe. If ReleaseResource throws an error then the
subsequent abort cleanup will come back here and try to release that
item again, possibly leading to a double-free or similar crash,
and in any case risking an infinite error cleanup loop. This mistake
explains why the pgcrypto bug just fixed in 80bb0ebcc led to a crash
rather than something more benign.
Remove the item from the resowner, then call ReleaseResource,
matching the way things were done before b8bff07da. If there
is a problem of this sort, we'd prefer to leak the item than
suffer the other likely consequences.
Per further analysis of bug #19527.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/646741.1782157515@sss.pgh.pa.us
Backpatch-through: 17
Raising an error within a function using an OSSLCipher object led
to a complaint from ResourceOwnerForget and then a double-free crash,
because ResOwnerReleaseOSSLCipher forgot to unhook the OSSLCipher
object from its owner. (The sibling logic for OSSLDigest objects got
this right, as did every other ReleaseResource function AFAICS.)
Oversight in cd694f60d.
Bug: #19527
Reported-by: Yuelin Wang <3020001251@tju.edu.cn>
Author: Yuelin Wang <3020001251@tju.edu.cn>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19527-6e7686960c6dce78@postgresql.org
Backpatch-through: 17
When left-join removal deletes a relation, remove_rel_from_query()
updates the relid sets attached to RestrictInfos and
EquivalenceMembers, and the canonical PlaceHolderVar held in each
PlaceHolderInfo, but it does not rewrite the PlaceHolderVars embedded
in clause and EquivalenceClass member expressions. That has been
fine, because later processing consults those relid sets rather than
the embedded PlaceHolderVars.
However, such an expression may afterwards be translated for an
appendrel child and have its relids recomputed from scratch by
pull_varnos(). If the embedded PlaceHolderVar's phrels still mentions
the removed relation, pull_varnos() folds it back in, so the rebuilt
clause's relids reference a no-longer-existent relation. That yields
a parameterized path keyed on the removed relation, tripping the
Assert on root->outer_join_rels in get_eclass_indexes_for_relids().
Fix by stripping the removed relids from the PlaceHolderVars in
surviving rels' baserestrictinfo and in EquivalenceClass member
expressions, keeping them consistent with the canonical
PlaceHolderVars.
This is only reachable on v18 and later, where
match_index_to_operand() began ignoring PlaceHolderVars; before that,
the wrapping PlaceHolderVar prevented the index match that exposes the
stale relids.
Reported-by: Alexander Kuzmenkov <akuzmenkov@tigerdata.com>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/CALzhyqwryL2QywgO03VQr_237Sq3MEVgTTT2_A9G3nGT5-SRZg@mail.gmail.com
Backpatch-through: 18
pg_mkdir_p creates each missing path component with a stat() followed
by mkdir(). If the stat() reports the component as absent but another
process creates it in the window before this process's mkdir(), mkdir()
fails with EEXIST and pg_mkdir_p treated that as a hard error -- unlike
"mkdir -p", which is meant to be idempotent and race-tolerant.
This shows up when several processes concurrently create paths that
share an ancestor directory: for example, parallel initdb runs whose
data directories live under a common temporary directory. One process
wins the race to create the shared ancestor and the others fail with
could not create directory "...": File exists
Fix this race condition by first trying mkdir() and only attempting
stat() if it fails with EEXIST.
On Windows, there's an additional problem: stat() opens a file handle
and participates in share-mode locking, which means it can transiently
fail on a directory another process is concurrently creating. Use
GetFileAttributes() instead: it requests only FILE_READ_ATTRIBUTES
and is exempt from share-mode denial, so it reliably sees a
concurrently-created directory.
I (tgl) also chose to back-patch 039f7ee0f's effects on this function,
so that pgmkdirp.c remains identical in all live branches.
Author: Andrew Dunstan <andrew@dunslane.net>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/3ca004de-e49b-4471-b8aa-fd656e70f68c@dunslane.net
Backpatch-through: 14
The JIT deforming code contains an optimization that determines which
columns are guaranteed to exist in the tuple. That's used to allow
skipping of reading the tuple's natts when the code only needs to deform
attributes that are guaranteed to always exist in all tuples. 83ea6c540
missed updating this code to account for VIRTUAL generated columns.
These are stored as NULLs in the tuple, but may be defined as NOT NULL.
This could result in the code thinking more columns are guaranteed to
exist than actually do.
Author: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Backpatch-through: 18
Discussion: https://postgr.es/m/1151393.1781734980@sss.pgh.pa.us
Newer gcc warns that this "actual_arg_types" variable may be used
uninitialized, but visual inspection indicates there's no bug. To
silence the warning, initialize the variable to zeros.
Bug: #19485
Reported-by: Hans Buschmann <buschmann@nidsa.net>
Tested-by: Erik Rijkers <er@xs4all.nl>
Tested-by: Hans Buschmann <buschmann@nidsa.net>
Reviewed-by: Tristan Partin <tristan@partin.io>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/19485-2b03231a775756f1%40postgresql.org
Discussion: https://postgr.es/m/6c52a1a6612948519468d46cb224a8c4%40nidsa.net
Add CHECK_FOR_INTERRUPTS() to the while loop in plperl_to_hstore()
that dereferences chains of Perl references, so that a circular
reference (e.g. $x = \$x) can be cancelled by the user instead of
spinning indefinitely. (We looked at detecting such circular
references, but it seems more trouble than it's worth.)
This is a follow-up to da82fbb8f, which fixed the same issue in
SV_to_JsonbValue() in jsonb_plperl.
Author: Aleksander Alekseev <aleksander@tigerdata.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAJ7c6TPbjkzUk4qJ5dHvDNEz0hBuFue3A-XWz_=897z+BC+z8A@mail.gmail.com
Backpatch-through: 14
Commit 6678b58d78 fixed a wrong "Prev" link by changing the link
generation code to use [position()=last()] instead of [last()] in
the predicate on the union of reverse axes. Unfortunately, that
caused documentation builds to take much longer. To fix, combine
the "preceding" and "ancestor" steps into one "preceding" step and
one "ancestor" step, and revert the predicate back to [last()].
The smaller union evades the libxml2 bug while avoiding the build
time regression.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Tested-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/1132496.1781718007%40sss.pgh.pa.us
Backpatch-through: 14
As noted in the commit message of 850b9218c8, this function has gained
an extra called "missing_ok". All the callers of this routine should be
in core in the v15-v17 range. For v18, I have found one custom stats
kind that would be impacted by this change.
Discussion: https://postgr.es/m/ajOE3uRxVgSlPRcw@paquier.xyz
Backpatch-through: 15-18
drop_local_obsolete_slots() continued to dereference local_slot after
calling ReplicationSlotDropAcquired(). Once the slot is dropped, its
entry in the slot array can be reused by another backend, so later reads
of local_slot->data could observe a different slot's name or database
OID, leading to an incorrect unlock and log message.
Save the slot name and database OID before performing the drop, and use
the saved values for the subsequent UnlockSharedObject() call and the log
message. While at it, emit the "dropped replication slot" message only
when a slot was actually dropped, rather than unconditionally.
Author: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Backpatch-through: 17, where it was introduced
Discussion: https://postgr.es/m/TY4PR01MB177184FF9EE916F577E1F554194082@TY4PR01MB17718.jpnprd01.prod.outlook.com
pgstat_drop_entry_internal() generates an ERROR if facing a pgstats
entry already marked as dropped. With a workload doing a lot of
concurrent CALL and DROP/CREATE PROCEDURE, it could be possible for
AtEOXact_PgStat_DroppedStats(), that wants to do transactional drops, to
find entries that are already dropped, after a commit record has been
written. In this case, ERRORs are upgraded to PANIC, taking down the
server.
This issue is fixed by making pgstat_drop_entry() optionally more
tolerant to concurrent drops, adding to the routine a missing_ok option
to make some of its callers more tolerant (spoiler: some of the callers
want a strict behavior, like replication slots and backend stats).
pgstat_drop_entry_internal() cannot be called anymore for an entry
marked as dropped, hence its error is replaced by an assertion.
Functions are handled as a special case in core; this problem could also
apply to custom stats kinds depending on what an extension does.
track_functions is costly when enabled (disabled by default), which is
perhaps the main reason why this has not be found yet.
A similar version of this patch has been proposed by Sami Imseih on a
different thread for a feature in development. This version has tweaked
here by me for the sake of fixing this issue.
Reported-by: zhanglihui <zlh21343@163.com>
Author: Sami Imseih <samimseih@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/19520-73873648d44793cf@postgresql.org
Backpatch-through: 15
The call-count test in 001_server.pl runs into a recent upstream
regression in Curl:
https://github.com/curl/curl/issues/21547
The symptom is high CPU usage on some platforms during OAuth HTTP
requests. But it looks like the fix is on track for a June 2026 release,
as part of Curl 8.21.0, so just skip the test if we happen to be using
the broken version.
Reported-by: Andrew Dunstan <andrew@dunslane.net>
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Tested-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAOYmi%2B%3DyrwMSsHuNJ1V14isA4iSix5Xb3P3VEp1X0BS61MdV4A%40mail.gmail.com
Backpatch-through: 18
When debugging an OAuth trace, it's helpful to know what version of Curl
is in use. The SSL library that Curl is using (which may not be the one
in use by libpq) is also relevant, and it's just as easy to get, so
print that too.
This is being added post-feature-freeze, with RMT approval, in order to
fix some tests in the face of an upstream Curl regression. A subsequent
commit will make use of it in oauth_validator. Backpatch to 18 as well.
Tested-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAOYmi%2B%3DkP86t%2BZFFXNQ9G6K4ht7utdmB%3DCzhP%3DZ2wvuBymOTtQ%40mail.gmail.com
Backpatch-through: 18
If the call count test fails, you'll reasonably want to know what the
network trace looked like, but that information is currently swallowed.
Print it out instead.
Backpatch-through: 18
Add check_stack_depth() to Jsonb_to_SV, SV_to_JsonbValue,
PLyObject_FromJsonbContainer, and PLyObject_ToJsonbValue. Without
this, deeply nested JSONB values can crash the backend with SIGSEGV
instead of raising a proper error.
Also add CHECK_FOR_INTERRUPTS() to the while loop in SV_to_JsonbValue
that dereferences chains of Perl references, so that a circular
reference (e.g. $x = \$x) can be cancelled by the user instead of
spinning indefinitely. (We looked at detecting such circular
references, but it seems more trouble than it's worth.)
Author: Aleksander Alekseev <aleksander@tigerdata.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAJ7c6TPbjkzUk4qJ5dHvDNEz0hBuFue3A-XWz_=897z+BC+z8A@mail.gmail.com
Backpatch-through: 14
The current form of the catalog query picks up partitioned tables
with expression indexes that lack statistics. However, since such
indexes never have statistics, there's no point in analyzing them.
To fix, adjust the relevant part of the query to skip partitioned
tables with expression indexes. While at it, remove the nearby
stainherit check; entries for index expressions always have
stainherit = false.
Author: Baji Shaik <baji.pgdev@gmail.com>
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CA%2Bfm-RPE1tEc6CUUPDyRbYTz9tF5Kw47nnk-Zq%3DyYvanbsxyCQ%40mail.gmail.com
Backpatch-through: 18
Several calls of pgstat_count_io_op_time() have been used as data to
count negative values returned by pg_pread() or pg_pwrite(), leading to
an incorrect count reported, casting them back to uint64.
Most of the problematic calls updated here are adjusted so as we do not
report buggy negative numbers anymore. In xlogrecovery.c, the spot
updated still counts short reads. In xlog.c, after a WAL segment
initialization, I/O numbers are aggregated only after checking that the
operation has succeeded.
issues introduced by a051e71e28.
Reported-by: Peter Eisentraut <peter@eisentraut.org>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/0db864e6-4477-4eba-b2be-d3523cc86564@eisentraut.org
Backpatch-through: 18
The test did not wait for the standby to be connected to the primary.
This breaks one assumption at the beginning of the test, where the
primary is stopped to ensure that all its records are flushed to both
standbys before moving on with its next steps.
If standby_1 finishes ahead of standby_2, the test would be able work
fine as the former waits for the latter. The opposite is not true,
standby_2 getting ahead of standby_1 would cause the test to fail on
timeout when standby_1 attempts to connect to standby_2.
This commit adds an additional polling query after the two standbys are
started, checking that both standbys are connected to the primary before
processing with the initial steps of the test.
Like 7185eddf05, backpatch down to v14.
Author: Sergey Tatarintsev <s.tatarintsev@postgrespro.ru>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/fea4190e-f8b5-4432-a52d-bcbee5f34366@postgrespro.ru
Backpatch-through: 14
The error path in ReorderBufferProcessTXN was not freeing
(reorderbuffer.c's representation of) a speculative insertion record
correctly. In assert-enabled builds, this leads to an assertion
failure. In production builds, I see no effect; there may be a small
transient leak, but in an improbable code path such as this, such a leak
is not of any significance. For users running with assertions enabled,
the crash is annoying.
Fix by having ReorderBufferProcessTXN() free the speculative insert
ahead of freeing the rest of the transaction, and no longer try to
handle that insert as a separate argument to ReorderBufferResetTXN().
This code came in with commit 7259736a6e (14-era). Backpatch all the
way back.
In branches 14-16, also backpatch the assertion that originally fails in
the problem scenario, which was added by dbed2e3662 (originally
backpatched to 17), that at the end of ReorderBufferReturnTXN() the
in-memory size of the transaction is zero.
Author: Vishal Prasanna <vishal.g@zohocorp.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Backpatch-through: 14
Discussion: https://postgr.es/m/19c7623e882.4080fd5426212.311756747309556767@zohocorp.com
The previous approach introduced by 0dd93de69e was weak in terms of
name matching, as an --index=foo could match with a table with the same
name but from a different schema, pulling in more data than necessary.
For example, imagine the following case:
CREATE SCHEMA s1;
CREATE SCHEMA s2;
CREATE TABLE s1.foo (id int);
INSERT INTO s1.foo SELECT generate_series(1,100);
ANALYZE s1.foo;
CREATE TABLE s2.bar (id int);
CREATE INDEX foo ON s2.bar(id);
INSERT INTO s2.bar SELECT generate_series(1,100);
ANALYZE s2.bar;
A targetted pg_restore --index=foo would grab the relation and attribute
stats of s1.foo on top of the index s2.foo, which is incorrect. This
commit fixes this scenario by relying on a lookup of the dependencies of
a STATISTICS DATA TOC entry, checking if a TOC entry depends on an index
or another relkind before matching with the names of the objects wanted
for the restore.
Discussion: https://postgr.es/m/ajDBwpxs-otl585H@paquier.xyz
Backpatch-through: 18
The expression (len_diff * 10 * (an + 1)) used as the return value of
ltree_compare() is computed at int32 width. With LTREE_MAX_LEVELS =
65535, the product can exceed INT32_MAX once an ltree has more than
~14,653 levels, which causes the result to wrap and invert its sign.
That corrupts btree ordering as well as the "magnitude" consumed by
ltree_penalty() for GiST page splits.
To fix, split ltree_compare() into two functions. The new
ltree_compare_distance() function returns a float, which won't
overflow. It's used by the ltree_penalty() caller. All the other
callers only care about the sign of the return value, i.e. which of
the arguments is greater, so change ltree_compare() to not multiply
the result with (10 * (an + 1)), which avoids the overflow for those
callers.
Existing btree or GiST indexes on ltree columns containing values with
more than ~14,653 levels may be corrupt and should be REINDEXed.
Add a regression test based on the reporter's PoC.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reported-by: 王跃林 <violin0613@tju.edu.cn>
Discussion: https://www.postgresql.org/message-id/AI6AnABgKW93Qbx1jVzi84r9.8.1781322625756.Hmail.3020001251%40tju.edu.cn
Backpatch-through: 14
The schema_only_with_statistics test scenario was referenced in
002_pg_dump.pl, but was associated to no command sequence since
0ed92cf50c.
Issue discovered while investigating a different bug. Perhaps this
cleanup is not worth backpatching, but there is also an argument in
favor of reducing noise when touching this area of the code in stable
branches.
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/ai-y0S7Z25NlrG_n@paquier.xyz
Backpatch-through: 18
Attempting to restore a schema, a table or an index with
--only-statistics skipped all the statistics of the objects wanted.
Like for pg_dump, statistics should be included, so this created an
assymetry between dump and restore.
A second set of problems existed for --table and --index, where the
presence of --statistics skipped the restore of the stats of the
object(s) targetted.
This issue has been reported originally as related to an inconsistency
with the way extended stats restore is handled in Postgres v19, but the
issue is related to the restore of relation and attribute statistics in
v18. Some TAP tests are added to cover all these cases.
Reported-by: Chao Li <li.evan.chao@gmail.com>
Author: Chao Li <li.evan.chao@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/66E80CAB-527C-42B1-BB65-3F82CF4AD998@gmail.com
Backpatch-through: 18
Our handling of quoting within replication commands was pretty
sloppy, typically looking like
appendStringInfo(&cmd, " SLOT \"%s\"", options->slotname);
This is fine as long as options->slotname doesn't contain a double
quote mark, but what if it does? In principle this'd allow injection
of harmful options into replication commands, in the probably-unlikely
case that a slot name comes from untrustworthy input. We ought to
clean that up.
Moreover, even the places that were trying to be more careful
generally got it wrong, because they used quoting subroutines
intended for SQL commands rather than something that will work
with the replication-command scanner repl_scanner.l. For example,
several places naively use PQescapeLiteral() to quote option values
for replication commands. If the string contains a backslash,
PQescapeLiteral() will produce E'...' literal syntax, which
repl_scanner.l doesn't recognize. Another near miss was to use
quote_identifier() to quote identifiers. That function won't quote
valid lowercase identifiers unless they match SQL keywords ... but in
this context, replication keywords are what matter. Neither of these
errors seem to risk string injection, but they definitely can cause
syntax errors in replication commands that ought to be valid.
We can clean all this up by using simple quoting logic that just
doubles single or double quotes respectively.
Or at least, we could if repl_scanner.l handled doubled double quotes
in identifiers, but for some reason it doesn't! So the first step in
this fix has to be to fix that. (The fact that we'll later reject
slot names containing double quotes is very far short of justifying
this omission.)
Having done that, this patch runs around and applies correct
quoting in all places that generate replication commands containing
strings coming from outside the immediate context. Probably some
of these places are safe because of restrictions elsewhere, but it
seems best to just quote all the time.
This was originally reported as a security bug, which it could be
if replication slot names or parameters were to originate from
untrustworthy sources. But the security team concluded that that
was a very improbable situation, so we're just going to fix this
as a regular bug.
Reported-by: Team Dhiutsa
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/1648659.1781287310@sss.pgh.pa.us
Backpatch-through: 14
Presently, the "Prev" link on the page for background workers sends
you to the middle of the previous chapter instead of the actual
previous page. This appears to be caused by a libxml2 bug, but
regardless, a minimal fix is to change the link generation code to
use [position()=last()] instead of [last()] in the predicate on the
union of reverse axes.
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/aim4AZorFKaC7Wrf%40nathan
Backpatch-through: 14
Late-model clang complains that these functions should be labeled
with "format(printf, 2, 3)", and it's right. But let's go a bit
further and also make use of varargs, to remove duplication and
allow these functions to be used with non-integer input values.
Since no good deed goes unpunished, I had to also adjust a couple
of call sites. They weren't wrong as-is, since the size_t-sized
arguments were coerced to int on the way into diag3(). But
without that, we have to adjust the format strings.
The point of this is to suppress compiler warnings, so back-patch
into branches containing pg_bsd_indent, even though there's no
functional change.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/1645041.1781283554@sss.pgh.pa.us
Backpatch-through: 16
If a query has more than 7498 params, the ParameterDescription message
exceeds the 30000 byte limit on messages that are not specifically
marked as possibly being longer than that (VALID_LONG_MESSAGE_TYPE).
To fix, add ParameterDescription to the list.
Author: Ning Sun <classicning@gmail.com>
Discussion: https://www.postgresql.org/message-id/dbfb4b65-0aa8-470a-8b87-b6496160b28a@gmail.com
Backpatch-through: 14
This commit reduces the number of expected output files for the "xml"
test from three to two (well, mostly one, see below for details).
xml_2.out existed to handle some differences in output due to libxml2
2.9.3, due to some error context missing (085423e3e3). This file is
removed, by tweaking the XML inputs to trigger the same error patterns
for the problematic 2.9.3 and other libxml2 versions. This part is
authored by Tom Lane.
xml_1.out (no libxml2 support) is reduced in size by adding an \if query
that exits the test early. This still checks NO_XML_SUPPORT() through
xmlin(). The rest of the test is skipped if XML input cannot be
handled by the backend. This part has been written by me.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/aiu6CXO67q-s70n5@paquier.xyz
Backpatch-through: 14
Commit 2f70fdb06 removed the deprecated containment operator
~(aclitem[],aclitem) from the catalogs, but missed removing its entry
from the documentation. (Arguably the blame should fall on c62dd80cd,
which added this entry in contravention of the longstanding policy
that we don't document deprecated aliases in the first place.)
Author: Shinya Kato <shinya11.kato@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAOzEurQSyR5psWukyhUz1LtxyO55C2Vfp0Fmt8w2jGKxhszQmQ@mail.gmail.com
Backpatch-through: 14
bt_normalize_tuple() uses VARSIZE() to get the size of varlena, even though
it's not yet known, that it has a 4-byte header. Fix this by replacing a
accessor with a universal VARSIZE_ANY().
Backpatch to all supported versions.
Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/7ckc7oka4bvafkf5bwlqs6ygrhlsbhz25ppozfch7zbuxcx3rf%40e4pr4oqenalc
Author: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Backpatch-through: 14
