The logic for asynchronous Append assumes that pending requests made for
subplans of an Append are drained during ExecReScanAppend. To ensure
that, commit 9e283fc85 modified postgresReScanForeignScan to drain such
a request if any, but failed to take into account that if such a request
was made for a subplan that is re-scanned with parameter changes or
pruned in the next round by runtime pruning, the postgres_fdw callback
function is called after ExecReScanAppend or never called, respectively.
This would cause such a request to remain even after ExecReScanAppend,
leading to incorrect results, an infinite loop, or an assertion failure.
To fix, modify ExecReScanAppend to, for each of the pending requests,
give the FDW a chance to drain that request using the existing
ForeignAsyncConfigureWait/ForeignAsyncNotify callback functions. This
makes the change made to postgresReScanForeignScan useless, so remove it
as well.
Back-patch to v14 where asynchronous Append was added.
Reported-by: Alexander Korotkov <aekorotkov@gmail.com>
Co-authored-by: Alexander Korotkov <aekorotkov@gmail.com>
Co-authored-by: Gleb Kashkin <g.kashkin@postgrespro.ru>
Co-authored-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Reviewed-by: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
Reviewed-by: Gleb Kashkin <g.kashkin@postgrespro.ru>
Discussion: https://postgr.es/m/CAPpHfduMOTnV5Zj2KGJ7zanL_10QvccZHtPUaDfJvBhsh9axnQ%40mail.gmail.com
Backpatch-through: 14
AfterTriggerFireDeferred() drained its queued events in a loop, then
called FireAfterTriggerBatchCallbacks() once after the loop to flush
the RI fast-path FK-check batches the fired triggers accumulated.
A batch callback runs user-supplied cast or equality functions, whose
DML can queue further deferred trigger events. Because the flush ran
after the loop had exited, such an event was left in
afterTriggers.events with nothing to fire it, since
AfterTriggerFireDeferred() is the last drainer at commit. The deferred
check was skipped and a row violating the constraint committed.
Move the flush inside the loop, after afterTriggerInvokeEvents(), and
drop the "all fired" break, so afterTriggerMarkEvents() re-checks after
each flush and fires events a flush queued at the correct time. The
other FireAfterTriggerBatchCallbacks() callers leave any queued event
for the eventual commit-time AfterTriggerFireDeferred(), so only the
commit-time firing, which has no later drainer, lost events.
Reported-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/20260705210533.ee.noahmisch@microsoft.com
Backpatch-through: 19
ri_FastPathCheck() called index_beginscan() before switching to the
referenced relation's owner for the permission check and probe. For
btree this has no consequence, but starting the scan before the user id
switch sets a poor example for code dealing with out-of-tree access
methods, whose beginscan could observe the wrong user id. Move
index_beginscan() after the SetUserIdAndSecContext() call.
Reported-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/20260705210533.ee.noahmisch@microsoft.com
Backpatch-through: 19
The RI fast-path check probes the referenced index directly and, for
single-column keys, uses SK_SEARCHARRAY. Both assume the index is a
btree. A comment claimed "PK indexes are always btree", but a foreign
key's referenced index need not be a primary key:
transformFkeyCheckAttrs() accepts any unique (or, for temporal keys,
exclusion) index, so an out-of-tree access method advertising
amcanunique could supply a non-btree index reachable by the fast path.
Add pk_index_is_btree to RI_ConstraintInfo, set from the referenced
index's access method when the constraint is loaded, and make
ri_fastpath_is_applicable() return false for non-btree indexes so such
constraints fall back to the SPI path.
Reported-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/20260705210533.ee.noahmisch@microsoft.com
Backpatch-through: 19
The note claiming ri_FastPathCheck() is only used by the ALTER TABLE
validation path is no longer accurate. Rather than reword it, remove
it; the block comment above RI_FKey_check()'s call to the function
already documents when each path is taken, and a duplicated
cross-reference here would only drift out of date again.
Reported-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/20260705210533.ee.noahmisch@microsoft.com
Backpatch-through: 19
When pgstat_init_entry() fails due to an OOM in the DSA allocation,
pgstat_get_entry_ref() cleaned up the shared hashtable but forgot to
remove the local reference that pgstat_get_entry_ref_cached() had
already inserted into pgStatEntryRefHash.
Missing this cleanup would leave a backend with a stale local cache
entry whose entry_ref points to a NULL shared_stats. If
pgstat_gc_entry_refs() runs with this reference still around, it would
crash due to a pointer dereference.
The local reference is now removed before removing the shared entry,
the order being sensitive to pending interrupts.
Oversight in 8191e0c16a.
Author: Niall Newman <nn@turacolabs.com>
Discussion: https://postgr.es/m/2FDAA194-9CF3-4FD7-A450-F1A4BEB125F6@turacolabs.com
Backpatch-through: 15
VACUUM's wraparound failsafe mode exists to reclaim transaction IDs as
quickly as possible. 4830f10243 made the failsafe stop using the
BAS_VACUUM buffer access strategy so that the rest of the vacuum could
make use of all of shared buffers rather than being confined to the
small strategy ring.
However, when 9256822608 made vacuum's first heap pass use the read
stream, this was accidentally disabled. The read stream keeps its own
references to the buffer access strategy, so clearing vacrel->bstrategy
in lazy_check_wraparound_failsafe() no longer had any effect on the
reads issued by the first pass.
Fix this by adding clearing the BufferAccessStrategy reference actually
being used by the ongoing scan -- those in the ReadBuffersOperations
structs themselves.
Two things we accept rather than fix, as neither is worth the added
complexity given how rarely failsafe mode is reached:
- A small amount of read time for IOs that were already in progress when
the strategy was cleared may be attributed to IOCONTEXT_NORMAL instead
of IOCONTEXT_VACUUM. WaitReadBuffers() derives the IOContext from the
(now cleared) strategy, so the wait time of these in-flight IOs is
misattributed. This is bounded by the stream's look-ahead window and
happens at most once per vacuum, when the strategy is first cleared.
- The stream's buffer pin limit stays lower than it would have been had
no strategy been used at all. max_pinned_buffers is capped by the
strategy's pin limit when the stream is created and is not recomputed
when the strategy is cleared. Raising it would mean building a new,
larger ring, which would require first waiting for all in-progress IOs
to complete. That didn't seem worth it.
Reported-by: Jingtang Zhang <mrdrivingduck@gmail.com>
Discussion: https://postgr.es/m/CAPsk3_APRYVLhAJ5TMwdmpSx8W_%3DPHMm%3DPmKAvnC3gBrfNommQ%40mail.gmail.com
Backpatch-to: 18
Adding a property with the same name multiple times to a label on an
element, either within a single PROPERTIES clause or across statements
via ALTER PROPERTY GRAPH ... ADD PROPERTIES, previously failed with a
unique-index violation on pg_propgraph_label_property. The same class
of bug existed for labels: listing the same label multiple times on
one element in CREATE PROPERTY GRAPH, or adding a label to an element
that already has it via ALTER PROPERTY GRAPH ... ADD LABEL, failed
with a unique-index violation on pg_propgraph_element_label. Detect
the duplicates up front and raise a friendlier error in both cases.
For properties, the cross-statement duplicate is caught by a syscache
probe before insertion. The in-clause duplicate could have been
caught by the same probe if we issued a CommandCounterIncrement()
between property inserts, but that forces a catalog invalidation per
property purely to detect a condition we can check for free on the
in-memory target list. The list-based check is only needed when the
properties are listed explicitly; when they are derived from the
table's attributes, names are already unique.
For labels, a single syscache probe on pg_propgraph_element_label
suffices for both the in-clause and cross-statement cases because
insert_element_record() already issues CommandCounterIncrement()
between successive label inserts.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://www.postgresql.org/message-id/flat/20260630173053.51.noahmisch%40microsoft.com
Buildfarm member olingo intermittently failed this test, timing out while
waiting for the subscriber log to report an ERROR because
max_prepared_transactions is zero there. The test captured the log offset
only after issuing the publisher's
BEGIN/INSERT/PREPARE TRANSACTION/COMMIT PREPARED sequence.
Since streaming is enabled, the subscriber can receive and apply the
transaction, and log the expected ERROR, before that publisher SQL command
even returns, i.e. before the test captures the offset. The subsequent
wait_for_log() calls then searched only from a point after the message had
already been written, and timed out waiting for it.
Fix by moving the offset capture to before the publisher's transaction is
issued, ensuring it always precedes the point where the ERROR can appear
in the subscriber log.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Backpatch-through: 16, where test was introduced
Discussion: https://postgr.es/m/c43753d8-5265-4f77-83ff-9b1167276ec5@gmail.com
The code added in c456e3911 added populate_isnull_array() to bulk
populate the slot's tts_isnull array 8 elements at a time. When tuples
don't have an exact multiple-of-eight number of attributes, this will
lead to populating the tts_isnull elements for attributes that don't
exist in the tuple. This is ok as the array is large enough. However,
if we perform tuple deforming in two passes, and on the first pass
deform *some* of the attributes with slot_getmissingattrs() then later
when we deform the remaining missing attributes, the subsequent call to
populate_isnull_array() would overwrite the tts_isnull values previously
set by slot_getmissingattrs(), and since that function only continues
where it left off, it wouldn't reapply the previously set values and those
would be left as NULLs, as populate_isnull_array() would have set them.
Here, we fix by passing the tuple's natts to slot_getmissingattrs()
rather than the attnum we're deforming from. This means we apply all
the missing attribute values each deform pass, so slightly more work,
but deforming several missing values in different deform passes is
likely exceedingly rare. Doing that seems much better than adding
overhead in the happy path to check for this and skip the subsequent call
to populate_isnull_array().
Author: David Rowley <dgrowleyml@gmail.com>
Reported-by: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WznHo4b+6AmAj0GZ0jXqDSK69MfHe8fAQwuY_01y7cVNdw@mail.gmail.com
Backpatch-through: 19
If a subscription is disabled, don't try to build conninfo because
that may generate a confusing error and try to disable an
already-disabled subscription.
Partially addresses finding 5 in report from linked discussion.
Reported-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com
Backpatch-through: 19
seq_search_localized() returns the length of the matching prefix in
*len, but because it internally case-folds the inputs, it gets
confused on the length. The caller expects to get the length of the
prefix in the original string, but what it actually returns is the
length of the prefix after case-folding, which can be different if the
case-folded characters have different byte-length than the original,
or with ICU, if the case-folding changes the number of characters
(e.g. "ß", the German double s).
To fix, once we have determined that we have a match, work harder to
find the match's length in the original string. This adds some
overhead, but the strings are expected to be short.
The function does "case-folding" by converting a string to upper-case,
then to lower-case, which is a little ugly given that we have
dedicated functions for case-folding nowadays. But switching to that
doesn't seem appropriate to backpatch in a security fix, and that's
not available in older stable versions, anyway.
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Reported-by: Xint Code
Reviewed-by: Jeff Davis <pgsql@j-davis.com>
The COLUMNS list of a GRAPH_TABLE query is parsed as an ordinary select
target list, which permits aggregate functions, window functions, and
set-returning functions. GRAPH_TABLE has no machinery to evaluate them,
though: the rewriter copies the COLUMNS target list verbatim into a
freshly built subquery whose hasAggs/hasWindowFuncs/hasTargetSRFs flags
are never set, so the planner builds no Agg/WindowAgg node (and no SRF
expansion) and the Aggref/WindowFunc/SRF reaches the executor. This
triggers an assertion failure ("ecxt_aggvalues != NULL"), or "Aggref
found in non-Agg plan node" on a non-assert build, for otherwise
parser-accepted SQL such as
SELECT max(c) FROM GRAPH_TABLE
(g MATCH (x IS v) COLUMNS (count(*) AS c));
Reject these constructs in transformRangeGraphTable() the same way
subqueries are already handled: save and clear pstate->p_hasAggs,
p_hasWindowFuncs, and p_hasTargetSRFs around the transformation of the
COLUMNS list, and raise a "not supported" error if any of them got set.
This is deliberately a blanket prohibition for now. Once quantified
element patterns such as (a)->{1,5} are supported, aggregates over
property references of higher degree (e.g. count(a) or sum(a.val)) can
be allowed; at that point the check will need to inspect the aggregate
arguments rather than reject all aggregates outright.
Author: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAON2xHOYAmYLkB2jGi6g77d6Fqv8YgOrfV-riQVf0K_7AdxD3w@mail.gmail.com
hashbulkdelete() fetched the relcache's cached hash metapage before
calling read_stream_begin_relation(). During the read stream
initialization, relation lookups may process pending relcache
invalidation messages, which could cause the cached metapage to be
invalidated before the read stream uses it, leading to the failure of a
VACUUM bulk-deletion for a hash index.
This commit reworks the order of hashbulkdelete() so as its read stream
is initialized before fetching the cached metapage, so as pending
invalidation messages do not interfere with the relation scan.
Issue introduced by bfa3c4f106.
Author: Mikhail Nikalayeu <mihailnikalayeu@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/CADzfLwVEJ2_7ioX3ZSB1P7qXW40ghUy_ZCzeFvhDLoa_3Muztg@mail.gmail.com
Backpatch-through: 19
Commit e646450e60 added DETAIL messages for XID and MultiXactId
wraparound warnings that described the reported percentage as the
percentage of IDs available for use. However, the percentage is actually
calculated from the remaining distance to the wraparound limit, not the
stop limit where new IDs are refused. As a result, the wording could be
misinterpreted as meaning that the reported percentage of IDs can still
be allocated.
Update the wording to clarify that the reported percentage represents the
remaining transaction ID space or MultiXactId space before wraparound.
Also update the related XID warning hints to use "transaction ID"
terminology consistently.
Backpatch to v19, where the DETAIL messages were added.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/CAHGQGwHWZTsR6bjdfpa+pOkBPjoXXm2LuJ+5nh+CvdyqEASHcQ@mail.gmail.com
Backpatch-through: 19
Clarify how special wal_sender_shutdown_timeout values affect shutdown
waiting. A value of -1 disables the shutdown timeout and lets the
WAL sender wait for the receiver to catch up, while 0 causes immediate
termination without waiting for catch-up. Positive values bound how long
shutdown waits, so document that they should be high enough for WAL data
to be replicated under normal circumstances.
Update the main documentation, the GUC long description, and
postgresql.conf.sample consistently. While here, fix nearby paragraph
indentation and spacing.
Backpatch to v19, where wal_sender_shutdown_timeout was introduced.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Ian Barwick <barwick@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/AF4FE756-A220-4DA7-87B7-A126F3F307FD@gmail.com
Backpatch-through: 19
Commit c0d9864f5c made all but one caller of visibilitymap_clear() check
its return value, causing Coverity to flag the remaining unchecked call
in heap_page_fix_vm_corruption().
This VM clear is not WAL-logged, so the caller doesn't need the return
value of visibilitymap_clear(). Add an explicit void cast and comment to
document that the return value is intentionally ignored.
Backpatch to 19 when the number of callers discarding the return value
dropped low enough to trigger Coverity's warning.
Discussion: https://postgr.es/m/1065814.1784514869%40sss.pgh.pa.us
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Backpatch-through: 19
heap_force_common() tracked the current position in the caller-supplied
tid[] using OffsetNumber, which is only 16 bits wide, so when the array
held more than 65535 entries, the updated index wrapped around and the
outer loop never reached the exit condition. A SQL call with a
sufficiently large TID array would then run until interrupted.
Fix by tracking the tid[] position using int instead of OffsetNumber.
A regress case based on the report is included.
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reported-by: Yuelin Wang <1217816127@qq.com>
Backpatch-through: 14
Bug: #19607
Discussion: https://postgr.es/m/19607-2f256a66481c514b@postgresql.org
The SELECT privilege can be granted on a property graph, and its
privileges can be examined with psql's \dp command, but property
graphs were missing from both summary tables in the "Privileges"
section: the applicable object types for SELECT in the privilege
abbreviations table, and the per-object-type row in the summary of
access privileges table. Add the missing entries so the tables match
the actual behavior described for the SELECT privilege and in GRANT.
While at it, also add a test for \dp on a property graph.
Author: Shinya Kato <shinya11.kato@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAOzEurScgwLDXQmNFnDZANGAMMiva9GnLH_kO8qGtGVHUvVk2A%40mail.gmail.com
We allowed GRANT ... ON TABLE on sequences for backward compatibility.
We don't need to consider backward compatibility in case of property
graphs since we will be prohibiting its usage on property graph from
the very release which introduced property graphs.
Change regression tests that used GRANT ... ON [TABLE] on property
graphs to use GRANT ... ON PROPERTY GRAPH instead.
While here, add the missing RELKIND_PROPGRAPH cases in
pg_class_aclmask_ext() and in the object-type switch in
ExecGrant_Relation() so that the default ACL and the objtype passed to
restrict_and_check_grant() are correct.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/20260630023308.c7.noahmisch@microsoft.com
Commit f6f0542266 changed getNotify(),
getParameterStatus(), and related libpq message-processing paths to
abandon the connection on out-of-memory errors.
However, getCopyDataMessage() and pqFunctionCall3() did not handle
this new fatal-error state. Both can process asynchronous
NotificationResponse and ParameterStatus messages while waiting for
other responses. If one of those messages triggered a fatal error, these
loops continued processing instead of reporting it immediately.
Fix this by checking for a saved fatal error after processing an
asynchronous message. If the connection has been abandoned, return the
appropriate error immediately instead of continuing to parse input.
Backpatch to v18, where commit f6f0542266 introduced this issue.
Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAO6_XqpGfm+XHE1OzS=_+jroeDOxhhGa11P3cbm9q2gT05yorA@mail.gmail.com
Backpatch-through: 18
Per commit 8bf6ec3ba3, a column can be GENERATED only if it is such in
the whole inheritance tree.
For this reason, ATPrepDropExpression refuses to be called with ONLY on
a partitioned table. To detect this, the current implementation checks
whether recurse is set to false and the rel has direct children.
Recursion is implemented with ATSimpleRecursion, which calls ATPrepCmd
with recurse = false for every node in the tree. Inner nodes (for
example a partition which itself has subpartitions) then fail the check,
accidentally preventing the command from working on inheritance trees of
depth > 2.
This commit fixes it by also checking that we're at the top level of the
recursive calls using the recursing parameter, which is always true when
called through ATSimpleRecursion, always false when invoked on the root
rel.
Also, remove a comment claiming that DROP EXPRESSION could be
implemented with some effort. It cannot, as the commit message for
8bf6ec3ba3 explains.
Author: Alberto Piai <alberto.piai@gmail.com>
Backpatch-through: 14
Discussion: https://postgr.es/m/DHMT78XOD8BK.341V3H87KZ7NO@gmail.com
Fix missing NULL check in MemoryContextAllocAligned(). The underlying
call to MemoryContextAllocExtended() could return NULL when
flags contains MCXT_ALLOC_NO_OOM and the underlying malloc fails.
There are no current callers using MemoryContextAllocAligned() that pass
the MCXT_ALLOC_NO_OOM in core, so no live bug fix in core here. However,
an extension might use this pattern, so we'd better fix.
Fix this so we correctly pass the NULL to the caller rather than trying
to write to a NULL memory address.
This also fixes the same bug in AlignedAllocRealloc(), which is also
unused in core.
Backpatch to v16, where these functions first appeared.
Author: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/07DAC4C3-120D-4F3C-8FEE-BA236F7E9C1D@gmail.com
Backpatch-through: 16
Enabling retain_dead_tuples requires the publisher to run PostgreSQL 19 or
later and to not be in recovery. Previously this was checked only at DDL
time. That forced ALTER SUBSCRIPTION ... ENABLE to connect to the
publisher, so pg_upgrade (which re-enables subscriptions during restore)
failed if the publisher was unreachable. It was also not authoritative,
since the publisher's version or recovery status can change afterwards,
for example after a failover.
Perform the check authoritatively in the apply worker when it connects,
and stop doing it when enabling a subscription. ENABLE is the only command
issued during restore that triggered it, so this also fixes the pg_upgrade
failure. The DDL-time check is kept as a convenience for the other paths,
none of which are issued during restore.
Reported-by: Noah Misch <noah@leadboat.com>
Analyzed-by: Jeff Davis <pgsql@j-davis.com>
Author: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Jeff Davis <pgsql@j-davis.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Backpatch-through: 19, where it was introduced
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch@microsoft.com
We do not allow a DROP SERVER ... CASCADE to implicitly drop a
subscription, because it's in a shared catalog and dropping a
subscription has side effects. Instead we throw an error and the user
must drop the subscription explicitly. Document this behavior and add
a HINT to the error message.
Generalize AcquireDeletionLock()/ReleaseDeletionLock() to use shared
object locks for all shared catalogs, which includes AuthMemRelationId
and now SubscriptionRelationId.
Move error message after AcquireDeletionLock() to avoid an unnecessary
error if there's a concurrent DROP SUBSCRIPTION.
Addresses finding 10 & 15 in report from linked discussion.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch@microsoft.com
Backpatch-through: 19
Commit 6566133c5f added a case for AuthMemRelationId in
AcquireDeletionLock(), but not ReleaseDeletionLock(). The fall-through
case would go to UnlockDatabaseObject(), which would raise a WARNING;
and the lock would be retained until the end of the transaction.
Add the missing branch.
Discussion: https://postgr.es/m/2487ddcd737d4fc8e408e87aa9ad4365eed3bbb3.camel@j-davis.com
Backpatch-through: 16
When enabling checksums cannot process a database, the launcher uses
DatabaseExists to tell a concurrent drop (benign) from a real failure.
Since 1df361e3d8 that check also treats a present, but-invalid, data-
base as non-existent. An interrupted DROP DATABASE flush the invalid
marker before the row and files are removed, so a crash or ERROR can
leave an invalid row whose files remain on disk.
Report a database as existing whenever its catalog row is found to
ensure that checksums cannot be enabled if there are invalid databases.
The AccessShareLock in DatabaseExists already waits out an in-flight
drop, so an invalid-but-present row can only be an interrupted drop
leftover whose files still need checksums; enabling then aborts until
it is dropped.
Backpatch to v19 where online checksums were introduced.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAN4CZFOGdqxtZ5-6gb4apqmvoH=Z+TNH8RKJ3mVtoR1HirKQWg@mail.gmail.com
Backpatch-through: 19
Logging connection information, even at DEBUG1, creates unnecessary
risks. Remove the entire log message because it had no other useful
content.
Addresses finding 14 in report from linked discussion.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch@microsoft.com
Backpatch-through: 14
Commit c8a350a439 attempted to consolidate
code for identify possibly-compressed tar archives by suffix into a new
function parse_tar_compress_algorithm(). Unfortunately, the refactoring
wasn't perfect, and slightly changed the behavior at both existing call
sites.
In CreateBackupStreamer(), the previous code required the filename to
consist of more than just a suffix, so the aforementioned commit had the
effect of allowing pg_basebackup to accept a file from the server whose
entire name was something like .tar.gz -- which should never happen, but
let's reject it as previous releases did.
In precheck_tar_backup_file(), the previous code required the suffix to
be immediately adjacent to the prefix already checked, so the commit
in question allowed pg_verifybackup to accept not only filenames like
base.tar.gz but also filenames like baseFOOBARBAZ.tar.gz. While such
filenames are perhaps unlikely, rejecting them is correct, so let's go
back to that behavior.
Discussion: http://postgr.es/m/CA+TgmoYJY8FkoeYKGF_YF1S6uOK7fd0Bd3zrw0XY_oZXbmVFpQ@mail.gmail.com
Reported-by: Sarath Kumar <Sarath@iitmpravartak.net>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Backpatch-through: 19
get_graph_pattern_def() emitted the pattern-level WHERE keyword as
"WHERE " with no leading space, so reverse-parsing produced output
like "(o IS orders)WHERE (...)". The element-level WHERE deparse in
get_path_pattern_expr_def() already prepends a separating space; the
pattern-level branch was inconsistent with it. Emit " WHERE " to
match.
The output still re-parses to the same tree, so this is cosmetic.
For test coverage, add a whole-pattern WHERE clause to the existing
customers_us view, which is already reverse-parsed with
pg_get_viewdef().
Author: Dhruv Chauhan <chauhandhruv351@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CANWwWcpHb0h7tg6otRnL-FV83jwQpAiyw1bhvv8T78kpwZ-0ow%40mail.gmail.com
When checking whether a sub-select's output columns can produce NULL,
so as to decide whether a NOT IN can be converted to an anti-join,
query_outputs_are_not_nullable() falls back on find_nonnullable_vars()
for targetlist entries that are plain Vars: if the sub-select's own
quals prove the Var non-null, the output is non-nullable. But that
test compared only varno and varattno, without checking varlevelsup.
An outer reference in the targetlist could thus be matched against a
Var of the sub-select's own range table that happens to share the same
varno and varattno, wrongly proving the output non-nullable and
allowing an invalid conversion to an anti-join, which yields wrong
answers when the outer reference is NULL.
To fix, restrict the fallback to Vars of the current query level.
Author: Rui Zhao <zhaorui126@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAHWVJhGuaFFRpmq4j+mcMcm_HC5QOT7LZsC9bf9b7BCBmvbfMA@mail.gmail.com
Backpatch-through: 19
In the not-too-likely scenario where we successfully created a hash
table entry for a TS dictionary, but then failed to make a small
memory context for it, we left the hash entry in existence but with
a garbage value for dictCtx. This confused the code the next time
through, leading to a crash. Rearrange things so that we leave
the hash entry in a well-defined state with dictCtx == NULL, and
then the next try knows it still needs to make a memory context.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/0f3ddeb5-0dbd-479c-9d0e-ae254758e624@gmail.com
Backpatch-through: 14
Allocate CompoundAffix with room for its terminator, initialize the
old-format flag buffer before NIAddAffix(), and reject incomplete or
missing Hunspell AF aliases. None of these errors would be likely to
trigger on real dictionary files, accounting for the lack of previous
reports; but they're certainly bugs.
Bug: #19595
Reported-by: Michael Malis <michaelmalis2@gmail.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19595-7dc18b4e212c4757@postgresql.org
Backpatch-through: 14
Alberta (America/Edmonton) moved to permanent UTC-06 on
2026-06-18, which will affect their clocks beginning on 2026-11-01.
For lack of any clarity on the point, assume their TZ abbreviation
will be CST from that time forward.
Morocco (Africa/Casablanca) will move to permanent UTC+00,
without daylight saving time transitions, on 2026-09-20.
Backpatch-through: 14
