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
Previously, CheckPointReplicationSlots() ran at the start of
CheckPointGuts(), while WAL cleanup occurred much later in
CreateCheckPoint() and CreateRestartPoint(), after the buffer write
and ProcessSyncRequests() phases. During a spread checkpoint, this gap
could be several minutes.
During that time, active replication slots could advance their
restart_lsn. However, replicationSlotMinLSN had already been
computed from the older saved values. As a result, KeepLogSeg() could
retain WAL segments that were no longer needed, causing unnecessary
pg_wal growth until the next checkpoint or restartpoint.
Fix this by moving CheckPointReplicationSlots(),
CheckPointSnapBuild(), and CheckPointLogicalRewriteHeap() to just
before CheckPointTwoPhase(), after the buffer write and
ProcessSyncRequests() phases. This makes WAL retention decisions use
the latest replication slot state. The logical snapshot and rewrite heap
cleanup decisions also benefit from the updated saved restart_lsn.
Author: Ants Aasma <ants@cybertec.at>
Author: Hüseyin Demir <huseyin.d3r@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CANwKhkPCBcTQ_pk06MD5W5YYNnuYHp8dLNuOUz8-5pMBMPY1Bw%40mail.gmail.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
3592e0ff9 added caching logic to ExecFindPartition() to allow faster
partition lookups for LIST and RANGE partitions when we hit the same
partition multiple times in a row. The last_found_part_index field was
added, but it was never used, so remove it. Caching works by caching the
last matching index into the PartitionBoundInfo.datums, not by caching the
last found partition index.
Update comments to reflect this, and perform other general improvements to
the comments in this area.
Author: Aleksander Alekseev <aleksander@tigerdata.com>
Author: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAJ7c6TMRxO6vo1tNgLZs4nTNizpjCSWdUviB8Mf3nkYiNHNhzg@mail.gmail.com
When VACUUM or ANALYZE is run without a table list, we scan
pg_class to build the list of relations to process, and we
check the privileges on each relation we find. Since we don't
take any locks on the relations at this point, it's possible
for one to be concurrently dropped, in which case the privilege
check fails with an ERROR such as the following:
ERROR: relation with OID 16388 does not exist
This unnecessarily aborts the entire command. To fix, use
pg_class_aclcheck_ext() for the privilege check so that we can
detect concurrently-dropped relations and silently skip them.
There's no need to emit a WARNING for such relations because
they weren't explicitly named, and a drop at this point is no
different than one that happened before our pg_class scan
began. Note that concurrent drops that occur later on are
already handled gracefully by vacuum_open_relation().
The new missing_ok behavior is limited to get_all_vacuum_rels().
All other callers of vacuum_is_permitted_for_relation() should
hold a lock on the relation that prevents it from being
concurrently dropped, so this commit also adds an assertion to
that effect.
Oversight in commit a556549d7e.
This is a bug fix and could be back-patched, but given the
presumed rarity of the race condition and the lack of field
reports, I'm not going to bother.
Reported-by: ChangAo Chen <cca5507@qq.com>
Author: ChangAo Chen <cca5507@qq.com>
Co-authored-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Surya Poondla <suryapoondla4@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/tencent_F9D483523BB0D082C2EFDA80142F192DBC07%40qq.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
Code using GetNamedDSHash() to retrieve a dshash table may also want to
limit its size, but there is actually no easy way to do so because the
dsa_area of a hash table is hidden within dshash.c.
This commit adds dshash_get_dsa_area() to close the gap, which returns
the dsa_area for a given dshash_table. The DSA area retrieved can then
be passed to dsa_set_size_limit(), to limit its size. This accessor
routine also opens the door for allocating additional objects in the
same DSA area as a hash table, which may be useful for some.
Author: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/CAA5RZ0tKfCVqFnMZtavM42H63ha2Haf_C4mbJNWqkaW30cPW1w@mail.gmail.com
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
The glossary entry for data checksums workers incorrectly stated that
they were auxiliary processes, but they are implemented as background
workers. Fix, and while there, simplify the entry by combining the
worker and launcher into a single glossary term. Backpatch down to
v19 where online checksums were introduced.
Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwEv-C9ia+rBYyePzO8F=5FVvS412ZqcOupazuOb5RafNg@mail.gmail.com
Backpatch-through: 19
Three code paths checking the size, timeline ID and system identifier
stored in a manifest now check for an empty value. Values are always
expected in these parts of a backup banifest. A couple of tests are
added to validate this behavior
Additionally, precheck_tar_backup_file() checked that "endptr" is NULL.
Based on the C standard, strtoul() never sets an "endptr" to NULL when
given a value (that is the case here), returning a pointer to the
original value if there is nothing to convert. The pre-tar validation
code is adjusted to do so.
Author: Tristan Partin <tristan@partin.io>
Discussion: https://postgr.es/m/DKBS2Z9CGARC.2T07O6TJYSE8B@partin.io
Since 999dec9ec6, pg_stat_io can show read time with zero reads
for an IO Context: a foreign IO is counted as a read only in the
initiating backend, while other waiters record only the wait time. That
violates pgstat_bktype_io_stats_valid(). Relax the check to allow time
without a matching operation count, since we want to count read wait
time even in backends that did not initiate the read. This also enables
future accounting of waits on IO resources (e.g., AIO handles) in
backends that didn't start the IO.
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/ak5lccE4qiQpOBHn@pryzbyj2023
Backpatch-through: 19
CreateHardLinkA reports ERROR_INVALID_FUNCTION if the target file
is on a filesystem that doesn't support hard links. _dosmaperr
maps that to EINVAL, which confuses zic.c into failure. zic.c is
expecting ENOTSUP if the filesystem lacks link support, and will
properly fall back to making a physical copy if it gets that.
Hence, add code to map ERROR_INVALID_FUNCTION to ENOTSUP.
(We could instead teach _dosmaperr to do that, but it's far from clear
that this would be appropriate as a global behavior: intuitively
it seems like EINVAL should be appropriate most of the time.)
We didn't need this before commit aeb07c55f, because the tzcode
version we were using before that didn't have this particular
error-handling logic. Hence, no back-patch for now; but if we
decide to back-patch tzcode 2026b or later, we'll need this too.
Author: Vladlen Popolitov <v.popolitov@postgrespro.ru>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/e6122f9b2eef9096f1f11ecc058bcd91@postgrespro.ru
The slow_down interval parsing code checks explicitly for overflow, but
since it does that after the signed overflow has already occurred, we
end up inviting undefined behavior from the compiler anyway.
Use checked arithmetic instead. set_timer() takes a long int in order to
interface nicely with libcurl, so use an int32 as the interval counter
and clamp to LONG_MAX during conversion to milliseconds.
Backpatch to 18, where libpq-oauth was introduced.
Reported-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/qtclihmrkq67ach3xjxyi4qcksstin5qxwsnkqefkmotxwh4g6%40ae2bj6jvcmry
Backpatch-through: 18
As previously coded, walsummarizer only wants to read WAL from a file
where the TimeLineID in the filename exactly matches the TimeLineID being
summarized. But in some cases, when a timeline switch occurs, the WAL file
from the old timeline is not archived, because it's never completely
filled, so the only way to obtain the contents of that last partial
segment is to read from the first segment on the new timeline. Teach
WAL summarizer to do that, and add a test case to make sure that it
works.
Reported-by: Nick Ivanov <nick.ivanov@enterprisedb.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Tested-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Thom Brown <thom@linux.com>
Discussion: http://postgr.es/m/CA+Tgmobr27GpKDZx3_ezW2+C5_g18i+jSK3sGF_cR-_ESv5N5A@mail.gmail.com
Backpatch-through: 17
We recently removed support for upgrading from pre-v10 servers, so
we no longer need to handle older versions in this helper module.
Oversight in commit 14d8418083.
Discussion: https://postgr.es/m/ak7Ekv2-L-G55-YD%40nathan
adf97c156 allowed expression evaluation to perform hashing, and
subsequently 9ca67658d fixed a memory stomping bug in that commit
that caused unrelated-to-hashing expression op steps to stomp on the
intermediate hash value. The intermediate hash value needs to be
maintained when hashing multiple hash keys. 9ca67658d didn't quite get
things right when in "strict" mode when it aborted hashing early after
encountering a NULL hash key. What was meant to happen was that the
expression returns NULL directly to indicate to the caller the value
hashed to NULL. The problem was that any EEOP_HASHDATUM_FIRST_STRICT or
EEOP_HASHDATUM_NEXT32_STRICT op step that didn't belong to the final
key to be hashed would have its op->resnull and op->resvalue pointing to
the location to store the intermediate hash value. That's correct for
non-NULLs since we bit-rotate the intermediate value and continue hashing,
but with the strict case, when we get a NULL key, we immediately jump to
the "jumpdone" step. The problem is the jumpdone step expects the
ExprState resnull and resvalue fields to be set (as they would be if we
didn't abort hashing early due to the NULL), but when we aborted early,
the ExprState fields never got set. This would result in inserting
records into the hash table that would never match to any join partner,
which is a waste of CPU and memory.
Here we fix this by having EEOP_HASHDATUM_FIRST_STRICT and
EEOP_HASHDATUM_NEXT32_STRICT populate the ExprState resnull and resvalue
fields directly when the value to hash is NULL.
Although Hash Agg and Hashed Subplans do use hashing from ExprStates,
those were unaffected by this bug, as neither of those uses the STRICT op
steps.
Thanks to Tomas Vondra for finding the offending commit.
Reported-by: Dan Stefura <dstefura@bluecatnetworks.com>
Author: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/YQBPR0101MB89738FB972FBD02A3640C6D3D6C92@YQBPR0101MB8973.CANPRD01.PROD.OUTLOOK.COM
Backpatch-through: 18
check_publications_origin_sequences() warns when a subscription with
origin = NONE synchronizes sequence values that may have originated from
another subscription. The existing warning is phrased in terms of
copy_data and copying data, which is appropriate for table synchronization
but misleading for sequence synchronization.
Reword the warning, detail, and hint to describe sequence synchronization
and the associated origin = NONE semantics more accurately.
Also fix a typo ("rathen" -> "rather") in a comment in sequencesync.c.
Reported-by: Noah Misch <noah@leadboat.com>
Reported-by: Peter Smith <smithpb2250@gmail.com>
Author: vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Backpatch-through: 19, where it was introduced
Discussion: https://postgr.es/m/20260710045217.f0.noahmisch@microsoft.com
In the long option error path, the code previously returned BADARG
immediately when optstring[0] == ':' for a missing required argument,
without advancing "optind" or resetting "place". This error handling
was inconsistent with the short option path, where both updates are
performed before returning BADARG (and inconsistent with libc,
additionally..).
This affects platforms where our port version of getopt_long() is used,
a concept that should be limited to WIN32 these days. An argument could
be made in favor of a backpatch, but this could lead to a slight
different error handling, for a report that would only show up when
using incorrect option combinations.
Author: Japin Li <japinli@hotmail.com>
Discussion: https://postgr.es/m/SY7PR01MB10921AF81F18A8BCFA06388BEB6C22@SY7PR01MB10921.ausprd01.prod.outlook.com
Partition pruning for RANGE-partitioned tables could mistakenly prune
the DEFAULT partition in some cases when it was not valid to do so,
which could lead to rows missing from query results.
The only known cases where this could happen is when combining pruning
steps from an IS NOT NULL clause with other steps that matched to the
DEFAULT partition. This could occur due to RANGE partitioned tables
having two distinct internal representations for marking if the DEFAULT
partition should be scanned. The IS NOT NULL steps would mark the
"scan_default" boolean, but other steps created for different purposes
could mark a bound_offset Bitmapset, which would ultimately translate into
also scanning the default partition. This could all fail after multiple
steps were combined with a combine intersect operator, as that will
intersect the bound_offset bits and only set scan_default if all pruning
steps have that flag set. When both input steps to the intersect operator
had different representations of whether to scan the DEFAULT partition,
the resulting intersect step result would contain neither representation.
Here, we fix this by having the IS NOT NULL pruning result mark the
bound_offsets so that it uses both representations to mark that the
DEFAULT partition must be scanned.
Reported-by: Jacob Brazeal <jacob.brazeal@gmail.com>
Diagnosed-by: Jacob Brazeal <jacob.brazeal@gmail.com>
Author: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CA+COZaDXrfTaBjLE=Z79MTaH6Xun1V4PeKxLvCNv8mXS8wn0rw@mail.gmail.com
Backpatch-through: 14
The code paths patched in this commit fix some of remnants not addressed
by 1b105f9472. We should have more holes in the tree that could
benefit from stronger type safety guarantees. These hypothetical holes
could be addressed later; this finishes the job in storage/file/ for the
backend code.
Author: Tristan Partin <tristan@partin.io>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/DKBAWPZ2QDOS.10Y3JDUNKFMXX@partin.io
This fixes some incorrect flattening of nested Result nodes during
create_plan that was introduced by f2bae51df. That commit failed to
maintain the logic that checks for subplans and gating quals from the
nested Result node before flattening, and that could result in the nested
gating qual and subplan being lost, which could produce incorrect results.
Bug: #19579
Reported-by: Viktor Leis <leis@in.tum.de>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/19579-e6296b6c9fc0591c@postgresql.org
Backpatch-through: 19
Commit 6aba42c660 added quit() calls for two background psql sessions
whose slot creation is canceled by pg_cancel_backend(). Both sessions
ran with the default ON_ERROR_STOP=1 and ended their script with \q,
so psql exited as soon as the cancellation error arrived. quit() then
wrote another \q to the already-closed pipe, making the test die with
"ack Broken pipe".
Run both sessions with on_error_stop => 0 and drop the trailing \q, so
that psql stays at the prompt after reporting the error and quit() can
shut it down cleanly.
Discussion: https://postgr.es/m/CAD21AoCZY1fKYgfkvHGWGiXpatUKd23FSLnDCL4m9bWFjdXNZw@mail.gmail.com
Backpatch-through: 19
On standbys, logical decoding can be deactivated while a logical slot
is being created: either by replaying an
XLOG_LOGICAL_DECODING_STATUS_CHANGE record, or by the end-of-recovery
transition upon promotion, which deactivates logical decoding if no
valid logical slot exists. Both could interleave with a check of the
logical decoding status performed before creating a new slot because
the slot invalidation executed as part of the deactivation cannot find
a slot being created.
For regular slot creation on standbys, EnsureLogicalDecodingEnabled()
assumed that logical decoding must still be enabled during recovery
since the caller had already checked it, tripping an assertion failure
if a concurrent deactivation interleaved.
For slot synchronization, the local slot could be created and
persisted based on the remote slot information fetched before the
deactivation was replayed, leaving a valid slot whose restart_lsn
precedes the deactivation.
Fix both paths by re-checking the logical decoding status after the
new slot has been created: regular slot creation raises an error, and
slot synchronization skips persisting the slot. If the deactivation
happens after the re-check instead, it is guaranteed to invalidate the
newly created slot.
Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAD21AoDEB99VtNbQdDrNd=1gQupJNGMfW_5kdnxq03Q82EK3ag@mail.gmail.com
Backpatch-through: 19
When restoring relation stats, pg_restore_relation_stats() rejected
calls with (reltuples < -1.0). But that is insufficient - Infinity and
NaN values both pass that check, and get stored in pg_class verbatim.
This can have various undesirable consequences.
Fixed by rejecting non-finite reltuple values, in the same non-fatal way
as for the existing checks (emit WARNING and skip the update). Adds a
regression test to stats_import for these non-finite values, and to
check the -1.0 special value is still accepted.
Backpatch to 18, where pg_restore_relation_stats() was introduced.
Patch by Jan Nidzwetzki, minor commit message tweaks by me.
Author: Jan Nidzwetzki <jan@planetscale.com>
Discussion: https://postgr.es/m/518BA772-8026-412A-AA8F-A7FE4C6B3717@planetscale.com
Backpatch-through: 18
Index builds update pg_class.reltuples for the table. In parallel GIN
builds, workers track the number of processed rows, and report it to
the leader, who then updates the pg_class with a total. However,
gin_parallel_build_main failed to initialize the bs_reltuples field,
leaving it set to whatever happens to be on the stack (which may be
bogus values like Infinity or NaN, or just impossibly high values).
If such values get reported to the leader and stored in pg_class, that
can have serious consequences. The pg_class.reltuples field is used to
decide when a table is due for autovacuum or autoanalyze, and if it
happens to be set to a bogus value, that may never happen. The field is
also used by the optimizer when calculating costs.
Fixed by initializing bs_reltuples together with the rest of the build
state. The bs_numtuples was initialized later, but it seems cleaner to
just initialize all the fields at once.
After a bogus value gets persisted in pg_class, affected systems are
unlikely to self-heal. That would require an ANALYZE, but preventing
that is one of the consequences. We have considered forcing autoanalyze
in these cases, but there's not a good way to reliably identify bogus
values (except for a small minority like Infitiny/NaN).
A manual ANALYZE on (possibly) affected tables is the only solution.
Backpatch to 18, where parallel GIN builds were introduced.
Reported-by: Jan Nidzwetzki <jan@planetscale.com>
Discussion: https://postgr.es/m/518BA772-8026-412A-AA8F-A7FE4C6B3717@planetscale.com
Backpatch-through: 18
The new test for enabling data checksums with concurrent CREATE
DATABASE calls use the same injection points as a previous test
but accidentally missed detaching the injection point first.
Fix by detaching the injection point in the PG_TEST_EXTRA SKIP
block to make it can be reused. Pointed out by buildfarm member
porpoise which failed with:
die: error running SQL: 'psql:<stdin>:1:
ERROR: injection point "datachecksumsworker-fake-temptable-wait"
already defined'
Backpatch to v19 where online checksums were introduced.
Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Buildfarm member porpoise
Reviewed-by: Jonathan Gonzalez V. <jonathan.abdiel@gmail.com>
Discussion: https://postgr.es/m/28CF6FD9-E1C4-4C04-8270-E3305AC46171@yesql.se
Backpatch-through: 19
