psql's \pset display_true/false settings, added by commit 645cb44c54,
affect normal query output, but not \crosstabview. As a result, boolean
values used anywhere in crosstab output were always shown as "t" or "f",
which is inconsistent. Change \crosstabview so that the configured
values are displayed instead.
While at it, make \crosstabview print the \pset null string, if any, in
cells for which the query produces a NULL value. Cells for which the
query produces no value continue to have the empty string. This is an
oversight in the aboriginal \crosstabview commit, c09b18f21c.
Add a regression test covering all of this.
Author: Chao Li <lic@highgo.com>
Reported-by: Chao Li <lic@highgo.com>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Backpatch: none needed
Discussion: https://postgr.es/m/B5E6F0A5-4B48-46D0-B5EB-CF8F8CC7D07D@gmail.com
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
Commit 681daed931 introduced a new EXPLAIN option "IO", but the docs
did not explain what information was added to the output. Expand the
description a little bit, similarly to the other EXPLAIN options.
While at it, fix a typo in the first sentence.
Author: Tomas Vondra <tomas@vondra.me>
createPartitionTable() passed the partitioned parent's reltablespace straight
to heap_create_with_catalog(), bypassing the default_tablespace GUC fallback
that DefineRelation() applies for CREATE TABLE ... PARTITION OF. When the
parent had no explicit tablespace (reltablespace = 0), the new partition
unconditionally landed in the database default, even if default_tablespace
was set to something else; merging or splitting a set of partitions that all
lived in a non-default tablespace produced a new partition in the database
default.
Mirror DefineRelation()'s logic: take parent's reltablespace if set,
otherwise check GetDefaultTablespace() (which reads default_tablespace
and normalises pg_default / MyDatabaseTableSpace to InvalidOid). Also
add the CREATE ACL check on the resolved tablespace and the pg_global
rejection, matching DefineRelation()'s behavior.
Update the documentation for MERGE/SPLIT PARTITION to spell out the
tablespace-selection rule explicitly.
Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Discussion: https://postgr.es/m/ajQTklv8QArzTp3h%40pryzbyj2023
This addresses two gaps in the documentation:
- The function uses an xid, and was not mentioned as an exception in a
section of the docs related to xid8.
- The function returns NULL if a role does not have the privileges of
pg_read_all_stats. The execution is not denied.
Author: Chao Li <li.evan.chao@gmail.com>
Author: Yingying Chen <cyy9255@gmail.com>
Discussion: https://postgr.es/m/CAGGTb65Qmtor2nJP-ATgfWpMpD2qhKrdyO7fmRbbS++nQ=vtMw@mail.gmail.com
The uuid-to-bytea cast just serializes a valid uuid datum into its
fixed 16-byte representation. It does not have an input-dependent
error path so mark its pg_proc entry as leakproof.
Oversight in commit ba21f5bf8a.
Bump catalog version.
Author: Chao Li <lic@highgo.com>
Discussion: https://postgr.es/m/1FAAF426-9205-4F53-8D3B-F2003D96EC37@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
pg_stat_io asserts on unexpected combinations of backend type and IOOp.
These combinations were meant to help detect bugs given our current
understanding of the system -- not serve as a set of rules for what is
allowed. The autovacuum launcher scans catalog tables and may on-access
prune them. This previously wouldn't have led to any extends of the
relation, but now that on-access pruning may pin a page of the
visibility map (4f7ecca84d), scanning tables may lead to
extending the visibility map. This would cause the launcher to trip an
assert. Since there is no reason to forbid the launcher from doing
extends, remove it from the list of backend type pgstat_tracks_io_op
flags for doing IOOP_EXTEND.
Read-only catalog scans still don't let pruning set the VM; doing so
needs table AM API changes and is left for the future.
Reported-by: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/CAON2xHNOyaN9MCZohhD_NL6as3QVhGA0SOn2Hyi9w6+Y-_1bFA@mail.gmail.com
Commit fb80f388f added wal_receiver_timeout as a CREATE/ALTER
SUBSCRIPTION option, but psql tab completion did not include it in the
subscription option lists.
Add wal_receiver_timeout to completion for CREATE SUBSCRIPTION ... WITH
and ALTER SUBSCRIPTION ... SET.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/BBC5628A-63C0-4436-B8F3-90AF59BBEB73@gmail.com
In some corner cases, a new datachecksums worker could be launched
while an old one was still running. If you're really unlucky, the old
worker could set the worker_result in shared memory and mislead the
launcher to think that a newer worker invocation completed
successfully, even though it failed for some reason. That's highly
unlikely to happen in practice as it requires several race conditions
with workers and launchers starting, failing and succeeding and at the
right moments. Nevertheless, better to tighten it up.
To distinguish different worker invocations, assign a unique
'worker_invocation' number every time a new worker is launched. In
the worker, check that the invocation number matches before setting
the worker result. This ensures that the result always belongs to the
latest invocation.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/b283fbb9-298e-4953-9120-eefaf24fae20@iki.fi
Rename the 'success' field in DataChecksumState to 'worker_result'.
That's more appropriate when it's not a simple boolean.
Don't access the field after releasing the lock in ProcessDatabase().
No other process should be modifying it, but if we bother to do any
locking in the first place, let's do it right.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/b283fbb9-298e-4953-9120-eefaf24fae20@iki.fi
It might be left to an old value if the launcher was terminated while
a worker was running. launcher_exit() sends SIGTERM to the worker,
but did not clear 'worker_pid'. Clear it, to be tidy.
Also clear it in ProcessDatabase() before starting a new datachecksums
worker, to be sure we start from a clean slate. The codepath where
WaitForBackgroundWorkerStartup() returns BGWH_STOPPED but
worker_result != DATACHECKSUMSWORKER_SUCCESSFUL didn't clear it, while
all other codepaths did clear or set it.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/b283fbb9-298e-4953-9120-eefaf24fae20@iki.fi
Move DataChecksumsWorkerResult struct to the .c file. It's not used
anywhere else since commit 07009121c2 removed the injection point test
code that the comment referred to.
Mark StartDataChecksumsWorkerLauncher() as static, since it's not
called from outside the .c file. The DataChecksumsWorkerOperation
struct can then be moved into the .c file too.
Clarify the comment on StartDataChecksumsWorkerLauncher(). It said
"Main entry point for datachecksumsworker launcher process", but I
found that misleading. That description would be a better fit for
DataChecksumsWorkerLauncherMain(), which is the process's "main"
function, rather than StartDataChecksumsWorkerLauncher().
Fix comment on WaitForAllTransactionsToFinish() on postmaster death.
The comment claimed that it sets "the abort flag" on postmaster death,
but it actually just errors outs. Improve the comment to explain why
it doesn't just use WL_EXIT_ON_PM_DEATH.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/b283fbb9-298e-4953-9120-eefaf24fae20@iki.fi
This is similar to d3bba04154, batching all the reports of this type
received since the last batch. This covers typos and inconsistencies
for the most part.
The user-visible documentation change impacts only HEAD.
Commit 4e5920e6de disallowed RESPECT NULLS/IGNORE NULLS on
non-window functions, but it also caused the parser to check for
that clause too early in some cases. As a result, calls such as a
nonexistent function with IGNORE NULLS no longer reported the more
helpful "function ... does not exist" error, and aggregate functions
used as window functions reported "only window functions accept ..."
instead of the more accurate aggregate-specific error.
This commit moves the RESPECT NULLS/IGNORE NULLS checks so that
helpful existing errors are preserved where appropriate. This restores
"function ... does not exist" for nonexistent functions, while still
reporting that plain functions are not window functions and that
aggregates do not accept null treatment.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Tatsuo Ishii <ishii@postgresql.org>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAHGQGwH7VY_0GkhycyYZ4czkPGL0uGzDyOxk3uuFOSRR7wFY3g@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
Previously, a parameter specified in the startup packet for a physical
replication connection could encounter an error trying to perform an
ACL check for the setting.
Problem was introduced in a0ffa885e4, but no reasonable back-patchable
solution was found, so fixing only in master.
Bumps catversion.
Discussion: https://postgr.es/m/d8f8e11f06d692fff89e6be0f22732d30cf695a0.camel%40j-davis.com
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: Mark Dilger <mark.dilger@enterprisedb.com>
The final parameter of an ordinary variadic function should be an
array type. CREATE FUNCTION won't accept a declaration that isn't
like that, but it's possible to put an incorrect combination into a
pg_proc.dat entry. Sadly, the opr_sanity test that was supposed to
check that is broken and does not report functions with non-array
final parameters. This allowed exactly such a thinko to sneak into
the recently-added pg_get_*_ddl() functions: their last argument
should be declared text[] but was declared text. (We'd probably
have noticed eventually, when somebody tried to actually pass a
variadic array to one of those functions. But their regression
tests do not do that.)
Fix those functions, and fix the opr_sanity test so we'll notice
next time. Bump catversion for new pg_proc contents.
Author: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/D41A334E-ED9E-42EE-830D-28D4D36E9317@gmail.com
Since d51697484, psql's scanner treats CREATE SCHEMA as a command that
may contain SQL-standard routine bodies, so that semicolons inside
BEGIN ATOMIC ... END blocks do not terminate the command too early.
However, the code counted BEGIN/END throughout CREATE SCHEMA, so that
it could be fooled by valid (and previously accepted) code such as
CREATE SCHEMA s CREATE VIEW begin AS SELECT 1;
Improve this by explicitly checking whether each CREATE sub-clause is
CREATE [OR REPLACE] {FUNCTION|PROCEDURE}, and only counting BEGIN/END
within those clauses. Since CREATE FUNCTION/PROCEDURE wasn't allowed
in CREATE SCHEMA before d51697484, this will not risk failure on any
cases that worked before v19.
There remain cases that fool the top-level CREATE FUNCTION/PROCEDURE
heuristic and thus also the CREATE SCHEMA case, for example
CREATE FUNCTION begin () ...
But that's been true all along with no field complaints, so we'll
leave that issue for another day.
In the name of keeping things readable, move the logic supporting
this out of the {identifier} flex rule and into some small new
subroutines. Also rename existing related PsqlScanState fields
to help distinguish them from the added fields.
This patch also fixes what seems to me (tgl) a small bug: \;
would reset BEGIN/END detection even when inside parens or BEGIN.
That's unlike what a plain semicolon would do, and no such effect
is suggested by the documentation.
Author: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/8E03BB8D-003D-4850-9772-5F8015A5A0C7@gmail.com
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
The "identity" column of pg_identify_object() for property graph
objects can be long string of names connected by "of", e.g. "a of l of
e of g". The type of the first named object is given by column
"type". But the types of intermediate objects are not easy to find
from the identity string especially when some of them share the same
name. Some objects, like user mappings or authorization identifier
members, add types of objects other than the first one in the identity
string. Do the same for property graph objects.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/aej1DkLwhyZWmtxJ%40bdtpg
The pages of pg_dump, pg_dumpall and pg_upgrade mentioned that their
--no-statistics and --statistics options did not include the handling of
statistics created by CREATE STATISTICS, which was wrong.
Oversight in c32fb29e97.
Reported-by: Igi Izumi <igi@sraoss.co.jp>
Discussion: https://postgr.es/m/19529-c7eb1e7a0b07eae6@postgresql.org
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
PL/Python set-returning functions can crash with a use-after-free when
CREATE OR REPLACE FUNCTION is executed while the SRF is mid-iteration.
The crash occurs because srfstate->savedargs is allocated in proc->mcxt,
which gets deleted when the procedure is invalidated, leaving a dangling
pointer that PLy_function_restore_args() then dereferences.
The best fix is to use reference counting to prevent destroying the
function state while it's still in use, similar to what PL/pgSQL has
done. Rather than inventing a new wheel, this commit converts
PL/Python to use the funccache.c infrastructure.
The main challenge is that PL/Python uses SFRM_ValuePerCall for SRFs,
where the handler is called multiple times. A naive implementation
would allow the refcount to return to zero between calls, but we need
to hang onto the original state and function body. SQL-language
functions face the same challenge, so this commit follows the same
approach used in functions.c: maintain a per-call-site cache struct
(PLyProcedureCache) in fn_extra that holds both the pointer to the
long-lived PLyProcedure and the SRF execution state.
The use_count is incremented when we first obtain the procedure and is
decremented via a MemoryContextCallback registered on fn_mcxt, which runs
even during error aborts. Cleaning up the per-call SRF state needs more
care: an ExprContextCallback handles the in-query cases, since the
iterator is not guaranteed to run to completion (for example a LIMIT or a
rescan can abandon it early). But unlike SQL functions, whose resources
are released by transaction abort, PL/Python holds Python reference counts
on the iterator and saved arguments that abort will not release, and
ExprContextCallbacks are not invoked during an error abort. The
MemoryContextCallback on fn_mcxt therefore doubles as the backstop that
releases those references when a query errors out mid-iteration.
Since fn_extra is now used for PLyProcedureCache, this commit removes
use of the funcapi.h SRF infrastructure (SRF_IS_FIRSTCALL,
SRF_RETURN_NEXT, etc.) and switches to direct isDone signaling via
ReturnSetInfo, matching how SQL functions handle ValuePerCall mode.
This fixes a longstanding bug, so ideally we'd back-patch it. But
it'd be impractical to back-patch further than v18 where funccache.c
came in. The patch is somewhat invasive, and the bug only arises in
very uncommon usages (which is why it evaded detection for so long).
On the whole, the risk/reward ratio for putting this into v18 doesn't
seem good, so commit to master only.
Bug: #19480
Reported-by: Andrzej Doros <adoros@starfishstorage.com>
Author: Matheus Alcantara <matheusssilv97@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19480-f1f9fdce30462fc4@postgresql.org
The documentation for pg_get_sequence_data() did not match the
function's behavior. It stated that either USAGE or SELECT privilege
was sufficient, but the function returns sequence data only when the
caller has SELECT privilege.
The documentation also did not explain that the function returns a row
containing all NULL values when sequence data cannot be returned, such
as when the sequence does not exist or the caller lacks the required
privilege.
Update the documentation to reflect the actual behavior, including the
required privilege and the result returned when sequence data is
unavailable.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Tristan Partin <tristan@partin.io>
Discussion: https://postgr.es/m/CAHGQGwGNTaXnBKUV510_P1KwhdbHT+kgZ4zU5njBHy7nCqdhzg@mail.gmail.com
When synchronizing sequences for logical replication, a
publisher-side permission failure could be reported as if the sequence
were missing on the publisher, making the real cause harder to
identify.
This happened because pg_get_sequence_data() returns a row of NULL
values when the replication connection lacks permission to read a
sequence. Sequence synchronization treated that the same as a missing
sequence, causing it to emit a misleading "missing sequence on
publisher" warning.
Fix this by distinguishing permission failures from genuinely missing
sequences. The synchronization query now checks whether the
replication connection has the required privilege for each published
sequence, allowing the worker to report permission failures
separately.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Tristan Partin <tristan@partin.io>
Discussion: https://postgr.es/m/CAHGQGwGNTaXnBKUV510_P1KwhdbHT+kgZ4zU5njBHy7nCqdhzg@mail.gmail.com
An out-of-memory failure while initializing the type cache hash tables
would issue an ERROR and leave a backend in a partially inconsistent
state. Without assertions, the server would crash with a NULL pointer
dereference on initialization re-entry when doing a type lookup due to
one or both hash tables missing. An assertion would trigger if these
are enabled in the build.
This commit changes the ordering of the type cache initialization to
become more robust on re-entry after an in-flight allocation failure:
- The two hash tables are initialized first, and can only be initialized
once.
- The initialization is considered as done once the in-progress list is
allocated in the CacheMemoryContext. This is now the last allocation
step.
- Last, the callbacks are registered. These can only fail with a FATAL
error, taking down the process so leaving the process in a non-complete
state is fine.
This is in the same spirit as b85f9c00fb and 29fb598b9c, where
random allocation failures can make the backend go crazy in the code
paths fixed due to the static states becoming inconsistent. Like the
other fixes, this is unlikely going to show up in practice, so no
backpatch is done.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/e77acaac-a1b3-40b3-99ee-5769b4e453e4@gmail.com
In StandbyReleaseXidEntryLocks, a failure in acquiring a lock with
LockAcquire() due to an out-of-memory problem would lead to an
inconsistency with the lock state cached in the startup process,
impacting the list of RecoveryLockXidEntrys. The code is updated here
so as the cached state is updated once the lock is acquired.
This problem is unlikely going to happen in practice. Even if it were
to show up, it would translate to a LOG message for non-assert builds
(assertion failure otherwise), so no backpatch is done. This commit is
in the same spirit as 29fb598b9c, with a problem emulated by injecting
random failures for allocations.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/e77acaac-a1b3-40b3-99ee-5769b4e453e4@gmail.com
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
The cost parameters for the data checksums worker can be updated by the
user issuing a repeated enable checksum command, but the comments on the
struct members hadn't been updated to reflect this and were out of date.
Another part of the same comment needed better wording to be readable.
Also wrap the reading of the parameters in a lock, there is no live
bug due to not using a lock but it's still the right thing to do.
Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reported-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/2176020b-ecbc-438b-9fc3-9c3593d9e6fc@iki.fi
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
Previously, if the subscription used a server,
ForeignServerConnectionString() could raise an error (e.g. missing
user mapping) during DROP SUBSCRIPTION even if the conninfo wasn't
needed at all.
Construct conninfo after the early return, so that if slot_name is
NONE and rstates is NIL, the DROP SUBSCRIPTION will succeed even if
ForeignServerConnectionString() raises an error (e.g. missing user
mapping).
If slot_name is NONE and rstates is not NIL, DROP SUBSCRIPTION may
still encounter an error from ForeignServerConnectionString().
Reported-by: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/OS9PR01MB12149B54DEA148108C6FA5667F52D2@OS9PR01MB12149.jpnprd01.prod.outlook.com
In some cases, effective_multixact_freeze_max_age can be 0, which
presents a division-by-zero hazard for the multixact ID age score
calculation. To fix, bump it to 1 in that case so that we use the
multixact ID age as the score. While at it, also document that
this component score scales due to high multixact member space
usage.
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
Author: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/CAD21AoC6nKeYAjTvJ9dmBea03GZK9222h_O%3DONmcVuxfyO88Bg%40mail.gmail.com
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
This reverts the non-text (custom/directory/tar) output format support
for pg_dumpall added by 763aaa06f0 and its feature-specific follow-ups,
in line with Noah Misch's post-commit review which recommends reverting
and finishing the work through the commitfest.
Scope is deliberately minimal: only the feature itself is removed.
Independent improvements that merely touched the same files, or that were
committed alongside the feature but do not depend on its design, are
preserved.
Reverted (the feature):
763aaa06f0 Add non-text output formats to pg_dumpall
d6d9b96b40 Clean up nodes that are no longer of use in 007_pgdumpall.pl
01c729e0c7 Fix casting away const-ness in pg_restore.c
c7572cd48d Improve writing map.dat preamble
3c19983cc0 pg_restore: add --no-globals option to skip globals
abff4492d0 Fix options listing of pg_restore --no-globals
bb53b8d359 Fix small memory leak in get_dbname_oid_list_from_mfile()
a793677e57 pg_restore: Remove dead code in restore_all_databases()
a198c26ded pg_dumpall: simplify coding of dropDBs()
ec80215c03 pg_restore: Remove unnecessary strlen() calls in options parsing
Preserved (independent of the feature):
b2898baaf7 the check_mut_excl_opts() helper in src/fe_utils/option_utils.c
and its use in pg_dump
7c8280eeb5 pg_dump's conflicting-option refactor (and tests 002/005)
be0d0b457c pg_dumpall's rejection of --clean together with --data-only
(re-expressed directly, since pg_dumpall.c is otherwise
returned to its pre-feature state)
74b4438a70 the dangling-grantor-OID GRANT fix (back-patched through 16)
273d26b75e, d4cb9c3776 independent pg_restore.sgml clarifications
Because the feature restructured pg_dumpall.c and pg_restore.c (pg_restore's
main() was split into restore_one_database() plus a dispatcher) and
interleaved its option checks with the conflicting-option refactor in the
same regions, the cosmetic check_mut_excl_opts() reflow of those two files'
option blocks is inseparable from the feature and comes out with it; the
behavior is unchanged. The reusable helper and pg_dump's use of it are
unaffected.
Discussion: https://postgr.es/m/20260607000218.96.noahmisch@microsoft.com
ALTER TABLE ... MERGE PARTITIONS / SPLIT PARTITION builds a new
partition via createPartitionTable(), but never gives it a TOAST table.
When the source rows carried out-of-line varlena values, the move
into the new partition entered heap_toast_insert_or_update() with
reltoastrelid = InvalidOid: the externalization step is skipped, the
value falls back to inline storage and heap_insert() fails with
"row is too big" error. Also, TOAST table is needed if the new partition
receives out-of-line varlena values after the DDL operation is complete.
Call NewRelationCreateToastTable() right after the new partition is
created in createPartitionTable(), mirroring what DefineRelation()
does for regular CREATE TABLE. NewRelationCreateToastTable() decides
on its own whether a TOAST table is actually required, so partitions
with no toast-eligible columns are unaffected.
Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/ai_c4-v8iLA2kXFV%40pryzbyj2023
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Reviewed-by: Jian He <jian.universality@gmail.com>
If the allocation of Snapshot->subxip fails, a follow-up call of
GetSnapshotData() would see a partially-initialized snapshot, causing a
NULL dereference on reentry when using "subxip" because only "xip" would
be allocated. In the event of an out-of-memory error when allocating
"subxip", "xip" is now reset before throwing an ERROR, so as Snapshots
can be allocated and handled gracefully on retry.
This problem is unlikely going to show up in practice, so no backpatch.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/e77acaac-a1b3-40b3-99ee-5769b4e453e4@gmail.com
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 EXCEPT clause of a FOR ALL TABLES publication tracks each excluded
table by its identity rather than by name. As a result, renaming a table
or moving it to another schema with ALTER TABLE ... SET SCHEMA leaves the
exclusion in place, and the table stays excluded from the publication.
This behavior was not previously documented and could surprise users who
might reasonably expect a schema-qualified exclusion to apply only while
the table remains in that schema. Add a note to CREATE PUBLICATION to make
the behavior explicit.
Author: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAHut+PvQ5BqnawCQd6r1tqqd+iAJC-CuRY8wscuXSrpHGUzofA@mail.gmail.com
