Commit 0e1f1ed157 taught seg_out() to print the certainty indicator
on an interval's upper boundary, but it was back-patched only as far
as v14. When upgrading from an older release, the old server prints
the one test_seg row exercising that case ('4.6 .. ~7.0') without the
indicator, so the pre- and post-upgrade dumps do not match. Make
AdjustUpgrade.pm delete just that row; seg's comparison function does
distinguish the certainty indicators, so the otherwise identical row
'4.6 .. 7.0' is unaffected.
Back-patch to all supported branches.
Per buildfarm members crake and fairywren.
Discussion: https://postgr.es/m/5ccbdbde-6467-4a10-bf4d-0be73a05ce8d@dunslane.net
Similar to commit 3692a622d3, for a slightly different code pattern in
psql.
No backpatch to avoid disrupting translation in stable branches.
Author: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/airjxKXx7aTG8kfE@alvherre.pgsql
read_local_xlog_page_guts has the same race as logical_read_xlog_page:
RecoveryInProgress() can return true during promotion, impacting the
availability of the operations doing WAL page reads with this callback.
This problem is similar to eb4e7224a1 that has addressed the issue for
logical replication, impacting more areas of the code where this WAL
page callback can be used (same narrow window during promotion, same
availability issue):
- pg_walinspect.
- Slot advance (SQL function).
- Slot creation.
Repack workers (v19~) and 2PC files (since forever) can also use this
callback, but they are irrelevant as far as I know. A test is added
with the SQL lookup functions. This part relies on injection points,
and is backpatched down to v18, like the test added for eb4e7224a1.
This issue could probably be fixed as well in v14 and v15 for
pg_walinspect. However, I also feel that there is a conservative
argument about consistency here due to the support of logical decoding
on standbys, so let's limit ourselves to v16 for now. pg_walinspect is
used less in the field compared to the two other operations, making
addressing this problem less attractive in these two older branches.
Reported-by: Xuneng Zhou <xunengzhou@gmail.com>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/7daef094-abf3-4672-bc23-3df4763b16a3%40gmail.com
Backpatch-through: 16
The FK fast-path batching added in b7b27eb41a buffers rows in a
transaction-lived cache (ri_fastpath_cache) keyed by constraint OID.
Running user-defined cast and equality functions during a batch flush,
together with the cache's lifetime and iteration, exposed two defects
reachable by an unprivileged table owner.
First, on subtransaction abort ri_FastPathSubXactCallback discarded the
entire cache. An entry's batch holds rows buffered by the enclosing
transaction, not just the aborting subxact -- the cache is keyed by
constraint, so a single entry can mix rows from multiple subxact levels.
An internal subxact abort during after-trigger firing (e.g. a PL/pgSQL
BEGIN ... EXCEPTION block) therefore dropped buffered rows of the outer
transaction without running their FK checks, letting orphan rows commit
behind a constraint that still reported itself valid. The discard also
left relations opened by the batch unclosed, producing "resource was not
closed" warnings.
Second, ri_FastPathEndBatch flushes by iterating the cache with
hash_seq_search. If flush-time user code inserts into a different
fast-path FK table, a new entry is added to the cache mid-scan; it may
land in a bucket the scan has already passed and never be reached, and
ri_FastPathTeardown then destroys the cache without flushing it,
silently dropping that check.
Cleanly unwinding the cache on subxact abort would require tracking the
originating subxact of each buffered row, since rows from different
levels share an entry (the cache is keyed by constraint) and deferred
constraints cannot be flushed early at a subxact boundary. Rather than
add that bookkeeping, confine batching to the top transaction level: in
RI_FKey_check, when GetCurrentTransactionNestLevel() > 1, use the
per-row fast path (ri_FastPathCheck) instead of buffering. Rows checked
inside a subtransaction are then verified immediately and roll back
cleanly with their subtransaction, and the cache only ever holds
top-level rows. With the cache confined to the top level, a
subtransaction abort has nothing of its own to discard, so
ri_FastPathSubXactCallback is removed along with its registration.
For the second defect, add a cache-wide flag (ri_fastpath_flushing) set
while ri_FastPathEndBatch iterates the cache. A re-entrant FK check
arriving while the flag is set takes the per-row path rather than adding
an entry to the cache being scanned, so no entry can be missed and torn
down unflushed. The flag is cleared in a PG_FINALLY so a flush that
throws (a reported violation or an error from user code) does not leave
it stuck. As defensive insurance it is also cleared in
ri_FastPathXactCallback() at transaction end.
The per-row fast path still bypasses SPI and stays well ahead of the
pre-19 SPI-based check. A fuller fix that preserves batching across
subtransactions -- whether by tracking the originating subxact of each
buffered row or by per-subxact cache stacks merged into the parent on
commit -- is left for a future release.
The subtransaction-abort case is covered by a new regression test. The
mid-scan cross-table case depends on hash bucket placement and so is not
reliably reproducible in a portable test, but the flag prevents it by
construction.
Reported-by: Nikolay Samokhvalov <nik@postgres.ai>
Reviewed-by: Nikolay Samokhvalov <nik@postgres.ai>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/m/CAM527d9exRCdWrhJOnAxk_vACg7sr_yPoaJp_+uCFY0qP8v=aw@mail.gmail.com
Commit a70bce43fb added instructions on how to recover if PostgreSQL
refuses to issue new transaction IDs because of imminent wraparound,
but when describing how to find replication slots that should be dropped,
it referred to pg_stat_replication where it should have referenced
pg_replication_slots.
In passing, decorate references to views with <structname> tags.
Backpatch to all supported versions.
Reported-By: Sanjaya Waruna <sanjaya.waruna@gmail.com>
Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Robert Treat <rob@xzilla.net>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/176767268098.1084085.10345048667224193115@wrigleys.postgresql.org
Backpatch-through: 14
The FK fast-path batching added in b7b27eb41a wrote the incoming row
into the batch array before checking whether the array was full:
fpentry->batch[fpentry->batch_count] = ExecCopySlotHeapTuple(newslot);
fpentry->batch_count++;
if (fpentry->batch_count >= RI_FASTPATH_BATCH_SIZE)
ri_FastPathBatchFlush(fpentry, fk_rel, riinfo);
batch_count is reset to zero only at the end of ri_FastPathBatchFlush(),
so it remains at RI_FASTPATH_BATCH_SIZE throughout a full-batch flush.
A flush runs user-defined cast functions and equality operators; if that
user code performs DML on the same FK table, ri_FastPathBatchAdd()
re-enters with batch_count == RI_FASTPATH_BATCH_SIZE and writes one past
the end of the array, corrupting the adjacent batch_count field. This
is reachable by an unprivileged table owner via an implicit cast with a
PL/pgSQL function and causes a SIGSEGV in assert-enabled builds.
Fix by bounds-checking the write into the batch array so a re-entrant
add can never write past the end, and by adding a "flushing" flag to
RI_FastPathEntry that routes re-entrant ri_FastPathBatchAdd() calls on
a busy entry to the per-row path (ri_FastPathCheck) instead of touching
the mid-flush batch array. The flag is set around the probe in
ri_FastPathBatchFlush() and cleared in a PG_FINALLY, which also resets
batch_count, so the entry is left empty and reusable if a flush error
(including a reported FK violation) is caught by a savepoint.
Add regression tests for both the re-entrant flush and reuse of an entry
after a flush error caught by a savepoint.
Reported-by: Nikolay Samokhvalov <nik@postgres.ai>
Reviewed-by: Nikolay Samokhvalov <nik@postgres.ai>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/m/CAM527d9exRCdWrhJOnAxk_vACg7sr_yPoaJp_+uCFY0qP8v=aw@mail.gmail.com
xpath() attempted to call xmlCopyNode() and xmlNodeDump() on a
XML_NAMESPACE_DECL, finishing with a confusing error:
=# SELECT xpath('//namespace::foo', '<root xmlns:foo="http://127.0.0.1"/>');
ERROR: 53200: could not copy node
CONTEXT: SQL function "xpath" statement 1
xpath() is changed so as it goes through xmlXPathCastNodeToString()
instead, that is able to handle namespace nodes. xml2 uses the same
solution. This issue has been discovered while digging into
9d33a5a804.
Author: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/aioT7ui_ZJ9RMlfM@paquier.xyz
Backpatch-through: 14
When amcheck validates that a B-Tree metapage's allequalimage flag
matches _bt_allequalimage(), it could fail to report corruption
unless one of the index key columns used interval_ops. As a result,
pg_amcheck could silently miss this corruption on other opclasses,
incorrectly reporting the index as valid.
The mistake was that bt_index_check_callback() kept ereport(ERROR)
inside the loop that scans key attributes for INTERVAL_BTREE_FAM_OID,
even though that loop is only needed to decide whether to add
the interval-specific hint. This commit moves ereport() out of the loop
so allequalimage mismatches are always reported, while still emitting
the hint for affected interval indexes.
Back-patch to v18, where d70b17636d introduced this regression
while moving the check into bt_index_check_callback().
Author: Chao Li <lic@highgo.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/011ACC9C-CB87-4160-ACE7-4ED57AB86E15@gmail.com
Backpatch-through: 18
MD5 authentication warnings are queued during authentication, before
startup options and role/database settings have been applied. The code
checked md5_password_warnings at queue time, so settings such as
ALTER ROLE ... SET md5_password_warnings = off did not suppress the
warning, even though the established session showed the GUC as off.
Keep the connection-warning infrastructure generic by allowing each
queued warning to carry an optional filter callback. Evaluate that
callback when warnings are emitted, after startup options and
role/database settings have been processed.
Use this for MD5 authentication warnings, while leaving password
expiration warnings unchanged. Add test coverage for an MD5-authenticated
role with md5_password_warnings disabled.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Japin Li <japinli@hotmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/AE46E42D-5966-4D76-9E64-95EAB01B9FB5@gmail.com
Since this is trying to add a SharedInvalRelSyncMsg rather than
a SharedInvalRelcacheMsg, it should use rs rather than rc.
This makes no difference as things stand, because the two structure
definitions are identical (except for the capitalization of "relid"),
but it's still a good idea to fix it.
Co-authored-by: Stolpovskikh Danil <d.stolpovskikh@ftdata.ru>
Co-authored-by: Robert Haas <rhaas@postgresql.org>
Discussion: http://postgr.es/m/bd6a5735b72b4afe99af49c3c62901d6@localhost.localdomain
In a few places, we were constructing translatable strings consisting of
elements list by adding one element at a time and separately a comma.
This is not great from a translation point of view, so rewrite to append
the comma together with the corresponding element in one go.
Author: Peter Smith <smithpb2250@gmail.com>
Author: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAHut+Pvp7jYcaiZ3pXedXgLcWZWDBLXFUK05JtZpGv3Mj=UOjw@mail.gmail.com
transformJsonParseArg() was not careful enough on generation of
transformed expressions when starting from expressions that are not
coercible to text but are in the string type category: it failed to
verify that coerce_to_target_type() succeeds, and returned a NULL
pointer. This leads to a later NULL dereference and crash at executor
time.
This escaped noticed because it cannot happen for built-in types, all of
which have casts to text. Only user-created types are potentially
problematic.
Fix by raising an error when a cast to text doesn't exist.
This mistake came in with commit 6ee30209a6.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reported-by: Chi Zhang <798604270@qq.com>
Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Backpatch-through: 16
Discussion: https://postgr.es/m/19491-7aafc221ec63f288@postgresql.org
When parsing expressions like (old).colname and (old).* in a RETURNING
list, the parser would lose track of the intended varreturningtype,
and therefore return incorrect results.
The root cause was code using GetNSItemByRangeTablePosn() to find a
namespace item from its rtindex and levelsup, without taking into
account returningtype, which would return the wrong namespace item.
Fix by adding a new function GetNSItemByVar() that does take
returningtype into account.
Backpatch to v18, where support for RETURNING OLD/NEW was added.
Bug: #19516
Reported-by: Marko Grujic <markoog@gmail.com>
Author: Marko Grujic <markoog@gmail.com>
Suggested-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/CAOvwyF2cO_5mAt=w=y-dFnaG5UkZ+3H8nSDoKF_iuWZHsU2ARg@mail.gmail.com
Backpatch-through: 18
When printing the upper boundary of a seg interval, seg_out() decided
whether to emit the certainty indicator ('<', '>' or '~') by testing the
upper indicator (u_ext) for '<' and '>', but mistakenly tested the lower
indicator (l_ext) for '~'. This is a copy-and-paste slip from the
symmetric code that prints the lower boundary a few lines above.
The consequences for valid input were:
* A '~' on the upper boundary was dropped on output, e.g.
'1.5 .. ~2.5'::seg printed as '1.5 .. 2.5'.
* When the lower boundary carried '~' but the upper boundary had no
indicator, the wrong test matched and sprintf(p, "%c", seg->u_ext)
wrote a NUL byte (u_ext == '\0'), which truncated the result string
and silently lost the entire upper boundary, e.g.
'~6.5 .. 8.5'::seg printed as '~6.5 .. '.
Certainty indicators are documented to be preserved on output (they are
ignored by the operators, but kept as comments), so this broke the
input/output round-trip for the affected values.
The bug has existed since seg was added. It went unnoticed because the
existing regression tests only exercised certainty indicators on
single-point segs, which are printed by a different branch of seg_out().
Add tests that place indicators on both boundaries of an interval.
Author: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAON2xHPYeRRCEVAv8XfE18KsEsEHCiYcJ5fOsoxFuMEfpxF1=g@mail.gmail.com
Backpatch-through: 14
During promotion, there is a window where RecoveryInProgress() returns
true but the WAL segments of the old timeline have already been removed.
A logical decoding could pick up the old timeline in this window when
reading a page, failing with the following error:
ERROR: requested WAL segment ... has already been removed
This issue does not lead to any data correctness issue, as retrying to
decode the data works in follow-up decoding attempts. It impacts
availability, though. Other WAL page read callbacks have a similar
issue, this commit takes care of what should be the noisiest code path:
logical decoding with START_REPLICATION in a WAL sender.
A TAP test, based on an injection point waiting in the startup process
after the segments have been removed/recycled, is added. This part is
backpatched down to v17.
This issue has been causing sporadic failures in the buildfarm, and
was reproducible manually. This issue happens since logical decoding on
standbys exists, down to v16.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/7daef094-abf3-4672-bc23-3df4763b16a3@gmail.com
Backpatch-through: 16
The subscription option max_retention_duration accepts an integer value
representing a timeout in milliseconds, where zero means unlimited
retention (no timeout). Negative values have no useful meaning, but were
silently accepted and stored in the subscription catalog.
A negative value causes should_stop_conflict_info_retention() to always
return true, because TimestampDifferenceExceeds() treats a negative
threshold as already exceeded. This stops dead tuple retention
immediately rather than honoring the configured timeout.
Fix by rejecting negative values for max_retention_duration during CREATE
SUBSCRIPTION and ALTER SUBSCRIPTION.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/9232401A-DEEE-49E1-9D11-D14A776DB82B@gmail.com
pgxmlNodeSetToText() passed nodeTab[i]->doc to xmlNodeDump() without
checking the node type, which could cause a crash as a
XML_NAMESPACE_DECL maps to a xmlNs struct. The passed-in code would
then be dereferenced in xmlNodeDump().
This commit switches the code to render XML_NAMESPACE_DECL nodes with
xmlXPathCastNodeToString(), like xpath_table(). Some tests are added,
written by me.
Author: Andrey Chernyy <andrey.cherny@tantorlabs.com>
Co-authored-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20260611031436.5afde3cb@andrnote
Backpatch-through: 14
In pursuit of removing a Valgrind-detected leak, I inserted
"pfree(pq_mq_handle);" into mq_putmessage's recursion-trouble-recovery
code path, failing to notice that shm_mq_detach would have pfree'd
that block just before (i.e., this particular code path did not leak).
So now that was a double pfree. We didn't notice because the
recursion scenario isn't exercised in our regression tests, but
Alexander Lakhin found it via code fuzzing.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/b8b40954-e155-41b3-9af8-ad4f261a1b64@gmail.com
GetBufferDescriptor() was called before checking if the buffer is local.
Such buffers have a negative ID, meaning that we could call
GetBufferDescriptor() with a wrapped-around uint32 value causing a
potential out-of-bound access to the BufferDescriptors array.
This is harmless in the existing code for the current uses of
MarkBufferDirtyHint(), but the author has found a way to make that
buggy while working on a different patch set, and the order of the
operations is wrong.
Oversight in 82467f627b. No backpatch is required, as this is new to
v19.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://postgr.es/m/CAExHW5uzRMYVZsXXS3HXXT0fG_sNrpUhUqwP4NorhaCqH9JDhA@mail.gmail.com
Commit 257c8231bf changed pg_buffercache_pages() to materialize its output
directly into a tuplestore. As a result, the function ended up trusting
a caller-supplied RECORD descriptors. That could lead to crashes
if the supplied row definition did not match the actual returned values,
for example by passing bool Datums to tuplestore_putvalues() with
an incompatible descriptor.
Fix this by constructing the correct tuple descriptor for
pg_buffercache_pages() and assigning it to
rsinfo->setDesc after InitMaterializedSRF(). This restores the executor's
tupledesc_match() verification, so incompatible caller-supplied
row definitions are rejected with an error, as before commit 257c8231bf.
Bug: #19508
Reported-by: Nikita Kalinin <n.kalinin@postgrespro.ru>
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Ashutosh Sharma <ashu.coek88@gmail.com>
Discussion: https://postgr.es/m/19508-e5f188183279219b@postgresql.org
EnableLogicalDecoding() sets xlog_logical_info to true, emits a
procsignal barrier, sets logical_decoding_enabled to true, and then
writes a WAL record. If the activating backend is interrupted between
these steps, a PG_ENSURE_ERROR_CLEANUP() callback runs to undo the
partial activation.
The previous callback asserted that logical_decoding_enabled was still
false and then cleared xlog_logical_info. Both actions were unsafe
when a second backend was concurrently activating: the peer backend
might have already observed xlog_logical_info as true, set
logical_decoding_enabled to true, and written the activation WAL
record before our callback fired, causing the first backend to hit the
assertion failure.
Fix this by having the abort callback call
RequestDisableLogicalDecoding(), allowing the checkpointer to undo the
partial activation in the same manner as a normal deactivation. This
simplifies the logic by unifying the activation abort and deactivation
paths. While this approach now wakes up the checkpointer when an
activation is interrupted, this should not be a serious issue in
practice since such interruptions are rare.
Add a test case to 051_effective_wal_level.pl.
Reported-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/788B5B8A-BC22-48D8-818E-7B00416CF84E@gmail.com
Nothing is to be gained from using pgrepack outside of REPACK
(CONCURRENTLY), and it leads to assertion failures in assertion-enabled
builds, and to crashes due to bogus memory lifetime in production
builds. Reject attempts to do that with a clean error report.
Clean up the nearby code a tad while at it. The only functional changes
in that are that the output_writer_private context is allocated and
partially filled by the pgrepack output plugin; and that ->relid therein
is now always present (rather than only in assertion-enabled builds).
Other than that it's just minor code rearrangement and added comments.
Author: Álvaro Herrera <alvherre@kurilemu.de>
Reported-by: Nikita Kalinin <n.kalinin@postgrespro.ru>
Suggested-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Antonin Houska <ah@cybertec.at> (older version)
Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com> (older version)
Discussion: https://postgr.es/m/19500-38a02529a69353a5@postgresql.org
A view definition with GRAPH_TABLE depends upon the property graph it
references as well as the properties and labels referenced in it. We
recorded the dependency on the property graph, but did not record
dependency on labels and properties. This allowed properties or
labels referenced by a view to be dropped, resulting in a cache lookup
error when such a view was accessed. Fix this bug by handling
GraphPropertyRef and GraphLabelRef in find_expr_references_walker().
The dependency on the data type of property does not need to be
recorded separately as it is recorded indirectly via a dependency on
the property graph property itself.
Note that a property or a label associated with individual elements
can still be dropped as long as there are other elements that are
associated with that property or label, since they do not lead to
dropping the property or the label from the property graph altogether.
Reported-by: Man Zeng <zengman@halodbtech.com>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/tencent_43D9888041FA4FDE498C7BF1%40qq.com
Commit 85c17f6 mistakenly declared a variable storing catalog_xmin as
XLogRecPtr, even though catalog_xmin is a TransactionId.
This caused no functional issue, but the type was clearly incorrect.
Therefore, this commit fixes it to use the correct type TransactionId
instead, and backpatch to v17 where the issue was introduced.
Author: Imran Zaheer <imran.zhir@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CA+UBfa=mNeLt-4BFjEP4tqdDsnq+oMqqPr7fd9Wji2_9YXmQdA@mail.gmail.com
There previously were a number of issues:
- We'd upload the cache even if we already had a high hit rate. That means we
churn through the available cache space very quickly.
For this we now check if the cache hit ratio is already high, and skip
uploading a new cache in that case.
- We'd generate per-branch caches, even if master's already would suffice,
because the branch doesn't change much
This is solved indirectly by the above.
- The cache key allowed prefix matches based on the branch,
e.g. master-pending would always use master's branch
Replace the cache key element separator of - with :, which is not a valid
part of a branch name.
- When rebasing a feature branch, we'd start with just that branch's cache,
rather than also having the newer cache of master available
This is solved by downloading by master's and the feature branch's cache,
simply overlaying both. That's possible because ccache is content addressed.
- The size of a cache would increase to the max, even though there likely will
be no benefit from old cache entries.
Address this by explicitly evicting old data and also recompressing the
cache before uploading it.
In my testing this utilizes the available cache space (10GB for personal
accounts) much more effectively than before.
The not entirely trivial determination of whether it's worth uploading a cache
entry is moved to a python script. I first had it as shell, but that gets
awkward. This way it'd also be more viable to use ccache for msvc at some
point.
The per-job redundancies are a bit annoying. There's a way around that, by
using composite actions, but I think that might be harder to understand,
without all that much of an improvement.
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/7eugqon2ilnaq6yimtq7prtl5wlia43mhpmwlydzlw4u4wonaz@hh2fagz5bjuu
Strings built by this function are not supposed to be subject to
NLS translation, but commit 6566133c5 missed that memo, so that
object identities like "membership of role %s in role %s" were
translated.
Previously, outlen was miscalculated if case_sensitive was false and
str_tolower() changed the byte length of the string. If outlen was too
large, pnstrdup() would stop at the NUL terminator, preventing
overrun. But if outlen was too small, it would cause truncation.
Fix by just removing outlen. It was only used in a single site, which
could just as well use pstrdup().
Discussion: https://postgre.es/m/1101e1a3afbbabb503317069c40374b82e6f4cac.camel@j-davis.com
Reviewed-by: Tristan Partin <tristan@partin.io>
Backpatch-through: 14
This reverts commit a0b6ef29a5, along with
its follow-up 2e123e3c2b ("Silence compiler
warning from older compilers"), which only adjusted code introduced by
the former.
The change failed with an empty table and an invalid default, and the
best way to deal with that will involve an addition to the TAM API, so
it's not ready for relese 19 now.
Discussion: https://postgr.es/m/7033D663-DDB4-4B35-922C-F33DE53B1502@gmail.com
Commit 0c8e082fba changed the time at which MyBackendType is assigned,
breaking a careful choreography in syslogger to decide when to write
messages to its own log files. Fix by flipping a boolean at the
(approximate) location where previously MyBackendType was set, instead
of depending on MyBackendType directly.
Author: Álvaro Herrera <alvherre@kurilemu.de>
Reported-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/ahP-JT4ZRPyobnLb@paquier.xyz
ExecForPortionOfLeftovers() assumed that any result relation with
ri_RootResultRelInfo should reinsert temporal leftovers through the
root relation. That is correct for partitioned tables, where tuple
routing is needed, but it is wrong for plain inheritance.
When UPDATE/DELETE FOR PORTION OF is run on an inheritance parent and
a child row is split, the leftover rows must be inserted back into the
child relation. Reinserting through the parent can lose child-only
columns and place the leftover rows in the wrong relation.
Fix this by distinguishing partitioned-table routing from plain
inheritance. For partitioned tables, keep using the root leftover
slot and insert through the root relation. For plain inheritance
children, use a leftover slot matching the child relation and insert
directly into the child. Also keep translating the application-time
column attno for child relations, so multiple-inheritance cases with
different attribute numbers are handled correctly.
Added an ExecInitForPortionOf function to set up the ForPortionOfState
for each child table, which keeps most of these decisions localized
instead of spread out through ExecForPortionOfLeftovers. Incidentally
clarified a comment about the rangetype stored in ForPortionOfState.
Add regression tests for UPDATE and DELETE FOR PORTION OF on
inheritance children, including a multiple-inheritance case where the
range column has a different attnum in the parent and child.
Author: jian he <jian.universality@gmail.com>
Co-authored-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/flat/4245F94D-84F1-4E05-BF81-C458A6CF9901%40gmail.com
The operators for array_eq, record_eq, range_eq, and multirange_eq
are all marked oprcanhash, but there's a pitfall: their hash functions
can fail at runtime if the contained type(s) are not hashable.
Therefore, the planner has to check hashability of the contained types
before deciding it can use hashing in these cases. Not every place
had gotten this memo, and noplace at all had considered the issue
for ranges or multiranges. In particular we could attempt to use
hashing for a ScalarArrayOpExpr on a container type when it won't
actually work, leading to "could not identify a hash function ..."
runtime failures.
For the most part we should fix this in the lookup functions provided
by lsyscache.c, to wit get_op_hash_functions and op_hashjoinable.
But there's a problem: get_op_hash_functions is not passed the input
data type it would need to check. We mustn't change the API of that
exported function in a back-patched fix, and even if we wanted to,
its call sites in the executor mostly don't have easy access to the
required data type OID. Fortunately, the executor call sites don't
actually need fixing, because it's expected that the planner verified
hashability before building a plan that requires it. Therefore,
leave get_op_hash_functions as-is and invent a wrapper function
get_op_hash_functions_ext that does the additional checking needed
in the planner's uses.
We also need to fix hash_ok_operator (extending the fix in 647889667).
While at it, neaten up a couple of places in lookup_type_cache where
relevant code for multirange cases was written differently from the
code for other container types.
Note: while this touches pg_operator.dat, it's only to add oid_symbol
macros. So there's no on-disk data change and no need for a
catversion bump.
Reported-by: Andrei Lepikhov <lepihov@gmail.com>
Author: Andrei Lepikhov <lepihov@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/ed221f95-f09b-4a9c-b05b-e1fed621ec87@gmail.com
Backpatch-through: 14
The security team has received a couple of reports about potential
SQL injection via refint's trigger arguments. We discussed this
while preparing CVE-2026-6637 and concluded that forcibly quoting
these arguments is more likely to break working code than to
prevent exploits. Unlike data values, the table/column names come
from trigger arguments, and there is little reason for a trigger
author to put hostile inputs into those arguments. So, let's
document it accordingly.
Reported-by: Nikolay Samokhvalov <nik@postgres.ai>
Reported-by: Alex Young <alex000young@gmail.com>
Reported-by: Satyanarayana Narlapuram <satyanarlapuram@gmail.com>
Suggested-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com>
Reviewed-by: Christoph Berg <myon@debian.org>
Reviewed-by: Satyanarayana Narlapuram <satyanarlapuram@gmail.com>
Discussion: https://postgr.es/m/ahXP7z7nsfGPOZ3T%40nathan
Backpatch-through: 14
Previously, ecpg accepted multiple descriptor header items in GET DESCRIPTOR
and SET DESCRIPTOR, but generated broken C code when they were used.
Although the grammar allowed this syntax, the implementation did not actually
support it.
This commit tightens the ecpg grammar so the header form of GET/SET DESCRIPTOR
accepts only a single header item, matching the implementation and preventing
generation of broken C code.
Also update the documentation synopsis accordingly.
Backpatch to all supported versions.
Author: Masashi Kamura <kamura.masashi@fujitsu.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Lakshmi G <lakshmigcdac@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/OS9PR01MB13174AD7D1829D0644B6BB90E9447A@OS9PR01MB13174.jpnprd01.prod.outlook.com
Backpatch-through: 14
pg_createsubscriber rejected duplicate --publication values while parsing
command-line options, even when the duplicate names referred to
publications in different databases. Since publication names are
database-local objects, the same name is perfectly valid across multiple
databases.
This restriction was not a practical problem before commit 85ddcc2f4c,
which added support for reusing pre-existing publications. After that
change, users who have identically-named publications in multiple
databases (a common convention) could not use the feature without renaming
their publications.
The analogous restriction on --subscription names is intentionally kept as
they are reused as replication slot names, which are cluster-global, so
allowing duplicate subscription names without additional guards could
cause a slot-name collision. That work is left for a future release.
Author: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/B08A7C89-B3DE-4C1D-A671-32AD8BAB7E22@gmail.com
When called from a parallel worker, this function calls initStringInfo()
and pq_beginmessage(), causing a StringInfo allocation to happen twice.
pq_endmessage() frees only the second allocation, with each call leaking
~1 kB into the per-worker memory context. This could cause a few
hundred megabytes worth of memory to pile up until the worker exits (the
message allocations happen in the parallel worker context), with the
situation being worse the longer a parallel worker runs.
Oversight in f1889729dd.
Author: Baji Shaik <baji.pgdev@gmail.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Tristan Partin <tristan@partin.io>
Discussion: https://postgr.es/m/CA+fm-RMopta1Dmq8udiU5sp+zwTvhUf4+xfbr3rZDfczH+p-xw@mail.gmail.com
Backpatch-through: 17
When a table's columns are narrower than the record header line, the
expanded aligned format produced misaligned output because the data
column width was not adjusted to match the record header width, leading
to output like:
+-[ RECORD 1 ]-+
| a | 10 |
| b | 20 |
+---+----+
This commit adjusts the output so as the column width match with the
header line, giving:
+-[ RECORD 1 ]-+
| a | 10 |
| b | 20 |
+---+----------+
Author: Pavel Stehule <pavel.stehule@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CAFj8pRCzGpsr9zTHbtTd4mGh2YPJqOEgLgt8JLiopuYA9_1xGw@mail.gmail.com
Backpatch-through: 14
In AlterSubscription(), when the SET clause includes both
retain_dead_tuples and origin options, the origin branch was using
assignment (=) rather than bitwise-or assignment (|=) when setting
check_pub_rdt. This meant that if retain_dead_tuples had already set the
flag to true in the same command, the origin branch would silently
overwrite it. As a result, the publisher-side retain_dead_tuples check
could be incorrectly skipped.
Fix by changing the assignment to |= so that the flag accumulates across
both option branches within the same ALTER SUBSCRIPTION command.
Author: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAHg+QDfe7WPOhVGKzv83ZB+BmXM88r=KPQn1sa_ZXMMChcNo=A@mail.gmail.com
TupleDescFinalize() failed to take into account virtual generated
columns, which are always stored as NULL in tuples. TupleDescFinalize()
didn't check for this, and that could result in attcacheoff being set for
and beyond virtual generated columns. Also, the TupleDesc's
firstNonGuaranteedAttr could also be set incorrectly, which could result
in the tuple deformation function deforming without checking for NULLs,
and deforming using incorrectly cached offsets.
This could result in tuples being deformed incorrectly, which could
result in incorrect results, ERRORs or possibly a crash.
This has been broken since c456e39113.
Author: Chao Li <li.evan.chao@gmail.com>
Reported-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: ChangAo Chen <cca5507@qq.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/A4BC563C-0CA3-4EF3-952A-EA41F9E5BF1E%40gmail.com
heap_force_common() declared a boolean array indexed with an
OffsetNumber for a size of MaxHeapTuplesPerPage. OffsetNumbers are
1-based, so an input TID whose offset number equals MaxHeapTuplesPerPage
wrote one byte past the end of the stack array, crashing the server.
Like heapam_handler.c, this commit changes the array so as it uses a
0-based index, substracting one from the OffsetNumbers.
Reported-by: Wang Yuelin <violin0613@tju.edu.cn>
Reviewed-by: Ashutosh Sharma <ashu.coek88@gmail.com>
Discussion: https://postgr.es/m/20260604002256.40f1fd544@smtp.qiye.163.com
Backpatch-through: 14
Document that OAuth validators can return an authenticated identity
in the authn_id member. The server records the identity value before
checking if the connection is authorized, so it may appear in
connection-authentication logs (even if the connection later fails
authorization).
Also remove outdated wording saying that all result parameters are
ignored when a validator returns false since validators may provide
error_detail.
Patch by Chao Li with some additional wordsmithing by me.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Reported-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/0281836A-F5FF-41A5-9EE1-656C1FAAC6B2@gmail.com
With TLS 1.3 the concept of curves was renamed to groups. Update
our wording to use groups instead of curves to make it clear what
the underlying GUC can support.
This was extracted from a slightly larger patch which also renamed
variables to match the new terminology. Given that we are in beta
this portion was however left as a future excercise.
Author: Evan Si <evsi@amazon.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/23C40DD6-1C47-46FC-A746-8A1D8530AD3E@amazon.com
Backpatch-through: 18
Presently, refint stores plans in a per-backend cache to avoid
re-preparing in each call. This has a few problems. For one,
check_foreign_key() embeds the new key values in its cascade-UPDATE
queries, so a cached plan reuses the values from preparation.
Also, the cache is never invalidated, so it can return stale
entries that cause other problems. There may very well be more
bugs lurking.
We could spend a lot of time trying to address all these problems,
but this module is primarily intended as sample code, and by all
indications, it sees minimal use. Furthermore, there is a growing
consensus for removing refint in v20. However, since we'll need to
support it on the back-branches for a while longer, it probably
still makes sense to fix some of the more egregious bugs.
Therefore, let's just remove refint's plan cache entirely. That
means we'll re-prepare on every call, but that seems quite unlikely
to bother anyone. On v17 and older versions, the regression test
for triggers fails after this change, so I've borrowed pieces of
commit 8cfbdf8f4d to fix it.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAJTYsWXU%2BfhuzrEd_bnrxyGH3%2Bny8QRQC2QHf3ws6s9iki3c2Q%40mail.gmail.com
Backpatch-through: 14
