When a table is partitioned, the previous code did not emit NO_GATHER
advice for the parent table, on the theory that doing this for the
child tables was good enough. However, it isn't. I believe what
happened here is that, during development, I was planning to enforce
NO_GATHER by clearing the consider_parallel flag for the children,
which would have been good enough to ensure that parallelism could
not be used at any higher level of the plan tree. Eventually, I
switched course, but failed to realize that this code needed to be
updated as a result. Do that, and add a regression test.
Backpatch-through: 19
Discussion: http://postgr.es/m/CA+TgmoYYHC4i-Gpzc9-JYHqih6pOK+6LTYFiNR-PioXG1nKkOQ@mail.gmail.com
Commit 0d4391b265 taught the planner
to store information about the RTIs of SubqueryScan, Append, and
MergeAppend node elided from the plan using a new ElidedNode data
structure; an hour later, 7358abcc60
taught to save the RTIs of an Append node into another Append
node using a new child_append_relid_sets field. Regrettably,
despite the fact that both commits were written by me and the
fact that both were part of the same patch set, the latter commit
failed to deal with the case where an Append node first acquires
child_append_relid_sets != NIL and then gets elided. I believe that
I thought that was impossible, but the included test case shows that
it isn't.
The proper fix involves updating the definition of ElidedNode to
carry child_append_relid_sets, so this commit does that, breaking
ABI compatibility. That's fine for the master branch, but it's a
bit unfortunate to be doing do that so close to the planned ship
date for v19. However, shipping v19 without fixing this appears
(at least to me) to be even more unfortunate.
Backpatch-through: 19
Discussion: http://postgr.es/m/CA+TgmoYYHC4i-Gpzc9-JYHqih6pOK+6LTYFiNR-PioXG1nKkOQ@mail.gmail.com
Given an Append or MergeAppend node, pgpa_build_scan needs to call
pgpa_make_scan for the node's own apprelids, and also for each element
of child_append_relid_sets. It did that, but it overlooked the fact
that the scan type applicable to the child scans might differ from
the scan type applicable to the parent scan. The problem can only
occur in one direction: an RTE_SUBQUERY (Merge)Append can appear on
top of a RTE_RELATION (Merge)Append, but not the reverse, so the
only failure mode here is to classify a partitionwise scan of a child
appendrel as PGPA_SCAN_ORDINARY rather than PGPA_SCAN_PARTITIONWISE,
leading to failure to include PARTITIONWISE(something) in the
generated advice string.
Backpatch-through: 19
Discussion: http://postgr.es/m/CA+TgmoYYHC4i-Gpzc9-JYHqih6pOK+6LTYFiNR-PioXG1nKkOQ@mail.gmail.com
Commit 7358abcc60 taught the planner to
store information about Append node consolidation in the final plan,
but it forgot that child_append_relid_sets needs to be adjusted
at setrefs.c time just like every other place where we store RTIs
in a plan tree. Fix that.
Although this can cause incorrect plan advice to be emitted in cases
involving multi-level partitioning, it seems more appropriate to put
the test case for it in pg_overexplain, so this commit does that.
Backpatch-through: 19
Discussion: http://postgr.es/m/CA+TgmoYYHC4i-Gpzc9-JYHqih6pOK+6LTYFiNR-PioXG1nKkOQ@mail.gmail.com
bpchar equality ignores trailing spaces, so equal values in a column
with no length specifier can have different images. Similarly,
oidvector equality intentionally pays no attention to the array lower
bound field. We nevertheless allowed nbtree deduplication to be used
with indexes built with bpchar_ops, bpchar_pattern_ops, or
oidvector_ops. A posting list split could therefore place a TID in an
index tuple whose key was equal to, but not bytewise identical to, the
corresponding table value. This allowed index-only scans to return
subtly wrong results.
To fix, drop support function 4 from the affected opclasses (we can't
change catalog contents on branches 14 through 18, so we just disable
deduplication by adding hard-coding to the relevant support functions).
Deliberately leave the oidvector opclass alone on branches 14 through
18, though. Disallowing oidvector deduplication on these branches would
make pg_amcheck report that pg_proc_proname_args_nsp_index has a corrupt
metapage, needlessly alarming users.
System catalog oidvector values always have a lower bound of 0, so the
issue can't cause harm there; a user can only hit it by indexing an
oidvector column that stores the output of a function like trim_array().
It seems very unlikely that any user would ever do that.
Users should REINDEX any B-Tree index on a bpchar column.
Author: Shihao Zhong <zhong950419@gmail.com>
Co-authored-by: Peter Geoghegan <pg@bowt.ie>
Reported-by: Ke <kehan5800@gmail.com>
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Bug: #19749
Discussion: https://postgr.es/m/19749-d46adf7c7d910b07@postgresql.org
Discussion: https://postgr.es/m/CAGRkXqQQ311Lg=62x5UdZtPuRFgpT6XpPMw4X9E3tgbTu162Mw@mail.gmail.com
Backpatch-through: 14
build_grouped_rel() makes a flat copy of the input relation, including
its FDW fields. As a result, when building a grouped join relation
for eager aggregation, add_paths_to_joinrel() called the FDW's
GetForeignJoinPaths for it. postgres_fdw then failed with "Aggref
found where not expected" on the partial Aggrefs in the target list,
and in any case it would have treated a partially aggregated input as
a plain foreign scan.
FDWs are not prepared to handle grouped relations, so clear serverid,
fdwroutine and fdw_private in build_grouped_rel().
Reported-by: Robert Haas <robertmhaas@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CA+Tgmob7iSM9YkRM44VjUDuaCchW-fY54MV5njpTZTL9uNyV4w@mail.gmail.com
Backpatch-through: 19
When we converted these types' input functions to report errors
through the soft-error mechanism, we missed that stack-overflow
errors can be triggered through bad user input. Convert those
to be reported softly, too.
Bug: #19745
Reported-by: Ke <kehan5800@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19745-39cd7b93a505f9a9@postgresql.org
Backpatch-through: 16
This commit addresses the following issues in commit de28140de:
* When the local transaction was DEFERRABLE, begin_remote_xact() started
a remote transaction in the same mode without checking the remote
server version. Since DEFERRABLE is supported only in v9.1 and later,
this would cause a syntax error on older remote servers. Fix this by
propagating the mode only to remote servers that support it.
* pgfdw_xact_callback() committed remote transactions without
synchronizing the read-only mode. If the local transaction had become
READ ONLY since the last remote operation, this would cause deferred
triggers on remote servers to still run in READ WRITE mode. Fix this
by synchronizing the mode at commit time.
* The documentation did not match the actual implementation. Update the
documentation to reflect the implementation.
Reported-by: Fujii Masao <masao.fujii@gmail.com>
Author: Etsuro Fujita <etsuro.fujita@gmail.com>
Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwE4UMaWGOZq1Z7YUTUSg-CgL4yAzO5YaGwTZe1-6Pjg2g%40mail.gmail.com
Backpatch-through: 19
On Windows, a backend exiting resets its connection, and the client
discards any data not yet read, including the FATAL message of a
terminated backend. isolationtester polls one connection at a time, so
when it waits on a different session the message can get lost, failing
the permutation and the tests that run after it.
wait_cleanup in injection_points (down to v17) and slot_creation_error
in test_decoding (down to v15) have this pattern, and have been failing
randomly in the buildfarm. Let's skip them on Windows.
The cause of the instabilities is proving to be tricky to deal with,
pointing for example at 29992a6a50. This will be tackled later,
perhaps with a backpatch (or perhaps with a HEAD-only solution), but it
is difficult to be sure at this stage of the work. In theory,
a8458f508a should make a revert of 29992a6a50 (revert of a revert!)
safe on WIN32.
Note that 278cdea6b9, that has introduced the isolation test
slot_creation_error in test_decoding, has mentioned the potential output
instability.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Author: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAN4CZFOoiKa9E4Gnh_OzLk3S5Z6RFyg_oY1e_WddCVYhkpTVow@mail.gmail.com
Backpatch-through: 15
When ANALYZE imports statistics from a remote server, it fetched/stored
the statistics while still running as the user executing ANALYZE, not as
the foreign table's owner. This is inconsistent with the sampling path,
and it causes a security issue: a role that owns a foreign table but has
no access to the underlying remote data (eg, no access privileges on the
remote server) could obtain that data by having a privileged user run
ANALYZE on the table; the imported statistics like most_common_vals then
expose sampled values from the remote data, as shown in the reproducer
on the discussion thread.
To fix, switch to the foreign table owner's userid in analyze_rel()
before calling the ImportForeignStatistics() routine, mirroring the
identity used by the sampling path. This not only makes the privilege
handling consistent between the sampling/import paths, but also prevents
statistics import from disclosing data the table owner couldn't
otherwise obtain.
Oversight in commit 28972b6fc.
Reported-by: Fujii Masao <masao.fujii@gmail.com>
Reported-by: Osama Abdul Qader <osamaabdulqader.cs@gmail.com>
Reported-by: Noah Misch <noah@leadboat.com>
Author: Noah Misch <noah@leadboat.com>
Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com>
Reviewed-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Discussion: https://postgr.es/m/CAPmGK16jVk+i2KMkkgR9ajoPdaUAinvcspkk5Bc6urbo2xYTMQ@mail.gmail.com
Backpatch-through: 19
The header comment for this file says it assumes that "the CPU is not
picky about alignment", which we wouldn't really accept for Postgres.
The two functions that have alignment requirements have been hacked in
ways that are inconsistent and yet both ugly. des_setkey just casts
its "const char *" argument to "const uint32 *", relying on the caller
to ensure that that pointer is actually word-aligned, which the caller
does by declaring the buffer as uint32[] and then casting to "char *".
Meanwhile des_cipher carefully memcpy's to and from an internal
uint32[] buffer, which is pretty silly given that what it's passed
must be word-aligned for the benefit of des_setkey. And on top of
that we have a bunch of ugly casting and oddly-written pointer
arithmetic in the caller px_crypt_des. The odd pointer arithmetic
causes warnings about possible buffer overrun when using recent gcc
at -O3 or higher.
Let's clean this up by converting the buffer variable into a union
of a uint8 array and a uint32 array, so that we can remove all these
casts, and also replace the strange loop logic with a simple subscript
variable to satisfy gcc.
Reported-by: Andres Freund <andres@anarazel.de>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Co-authored-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/2005885.1790359097@sss.pgh.pa.us
Discussion: https://postgr.es/m/3nuudxv365kjnmwjhnygdakhxuktpdjvf26rt26eb44esgqdrj@y2x3vomkrfoo
Backpatch-through: 14
The heuristic to avoid setting the VM during on-access pruning when
doing so would emit an extra FPI missed a few cases. First it missed
temp and unlogged tables. Those will never emit an FPI, so they can
always set the VM if the page is all-visible. It also missed that if
hint bits are not WAL-logged, setting only the VM passes REGBUF_NO_IMAGE
for an already WAL-logged heap page, forbidding a heap FPI in the WAL
record. A page that has never been WAL-logged still requires a heap FPI.
The third is more subtle: If the page is all-visible, the new prune xid
will be InvalidTransactionId. On-access pruning only executes when the
current pd_prune_xid is valid and visible. So, when on-access pruning
finds the page all-visible, it will always clear pd_prune_xid, modifying
the page.
Knowing this means we can set the VM without emitting an extra heap FPI
in more cases. When hint bits are WAL-logged, if the heap buffer is
clean, modifying pd_prune_xid will emit an FPI if one is required. There
is no reason to try to avoid an FPI by not setting the VM. So, set the
VM in this case. However, when the heap buffer is already dirty,
modifying pd_prune_xid can avoid an FPI; so if the page hasn't been
logged since the last checkpoint, setting it all-visible will emit an
extra heap page FPI. We will still avoid setting the VM in this case.
This is being backpatched to 19 because since 378a216187 pd_prune_xid
is set on insert, and if we execute a prune cycle and skip setting the
VM because of an incorrect heuristic, we have added new wasted work in
PG 19.
This commit adds some tests covering these cases. It also updates one of
the existing temp table tests to avoid exceeding the pin limit. Setting
the VM and FSM on-access when querying temp tables takes more local pins
and can run into the limit with fewer heap buffers pinned.
Reported-by: Melanie Plageman <melanieplageman@gmail.com>
Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/CAAKRu_amj7qLF4c=9ijd=708Fu2G8gg-2EqwBu=aCdAHU2sPHg@mail.gmail.com
Backpatch-through: 19
The query to get attribute statistics from a remote server applied the
"C" collation to a column of type name without checking the server's
version. Since the name type was made collatable in Postgres 12, this
could break attribute statistics import if the server was older. Fix by
casting the column value to text before collating it if the server is
older.
Also, fix a typo in the if-test in the same function.
Oversights in commit 28972b6fc; the former was reported by Fujii Masao.
Reported-by: Fujii Masao <masao.fujii@gmail.com>
Author: Etsuro Fujita <etsuro.fujita@gmail.com>
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwGvA_3q7Wg7GXQkuugGZj3%3DkxDRjDsKgVvK1uwAxHTopQ%40mail.gmail.com
Backpatch-through: 19
This commit fixes several related problems. All of these problems
escaped testing for the same reason: they do not occur when only
generated advice is supplied, and therefore test_plan_advice was
incapable of detecting these shortcomings.
First, prior to this commit, the advice feedback code never regarded
an unordered sublist of a JOIN_ORDER specification as matching an
unrolled join. As a result, when JOIN_ORDER() advice contained
unordered sublists, the advice feedback tended to be "matched, failed"
even when the advice worked exactly as intended. Only the case where
an unordered sublist was implemented by a plan shape not subject to
unrolling, such as a partitionwise join, worked properly.
Second, prior to this commit, a join order sublist in the initial
position wasn't properly handled. Note that this overlaps with the
problem described in the previous paragraph; JOIN_ORDER({a b} c) was
broken both because of the unordered sublist and because of the
sublist being in the initial position. However, there's more to this
case: JOIN_ORDER((a b) c) means the same as JOIN_ORDER(a b c), but the
advice feedback code didn't know that, and would generate "matched,
failed" for the former case even when everything was working.
Finally, prior to this commit, advice feedback didn't correctly handle
single-element sublists. Those are pretty nonsensical, since by
definition a join involves at least 2 tables, so we could just ban
that case. Instead, at least for now, I've chosen to make advice
feedback handle such cases in the same way that advice enforcement
already does: the extra grouping levels are simply disregarded, so
that JOIN_ORDER({a} ((b))) is enforced in the same way as, and also
gets the same advice feedback as, JOIN_ORDER(a b).
Reported-by: Noah Misch <noah@leadboat.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Up until now, pg_plan_advice has had a feature that allows an advice
target to mention a partition name but omit the partition schema;
that is, something like SEQ_SCAN(foo/bar) forces a sequential scan on
every child of table foo whose partition name is bar, regardless of
the schema in which bar appears. However, that feature turns out to
have a nasty design flaw: while advice enforcement handles this case
just fine, the advice feedback code doesn't know about it and will
mark such advice as "matched, failed" even when everything worked
perfectly. Unfortunately, there seems to be no simple code fix for
this problem.
Since the release of PostgreSQL 19 is imminent, take the conservative
course and revert this feature of pg_plan_advice. In other words,
require the partition schema whenever the partition name is present.
You must now write e.g. SEQ_SCAN(foo/public.bar) rather than just
SEQ_SCAN(foo/bar). This doesn't affect any cases where automatically
generated advice is supplied, since generated advice has always
included the partition schema anyway. Manually written advice will
have to conform to the new, stricter rule. For a later release, we
can consider whether to again relax this restriction in some way,
but now is not the time to design new things.
Reported-by: Noah Misch <noah@leadboat.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
List of commits reverted:
8e72d914c5 Add UPDATE/DELETE FOR PORTION OF
b6ccd30d8f Add isolation tests for UPDATE/DELETE FOR PORTION OF
33db6c4baf Fix DELETE/UPDATE FOR PORTION OF with rules
7b22f15a01 Add psql tab completion for FOR PORTION OF clause
7ca8c94296 Fix FOR PORTION OF with non-updatable view columns
993a7aa0e4 Fix cross-leftover pollution in FOR PORTION OF insert triggers
7ac030d5b1 Require UPDATE permission on FOR PORTION OF column
f0aedc7cb0 Small terminology fixes in comments
7d13b03a2e Fix FOR PORTION OF for inheritance children
a272a58b94 Move FOR PORTION OF volatile check into planner
a40fdf6588 Reject child partition FDWs in FOR PORTION OF
bc3ae886a7 Forbid FOR PORTION OF with WHERE CURRENT OF
c8d49ffb00 Forbid generated columns in FOR PORTION OF
5b5e99047a Forbid FOR PORTION OF on views with INSTEAD OF triggers
c58c83ce50 Fix RLS checks for FOR PORTION OF leftover rows
5454fe4a6e Test what BEFORE UPDATE triggers do to FOR PORTION OF
2a9541ddfd Deparse FOR PORTION OF using the range column's current name.
fd92f74b91 Avoid RETURNING side effects for FOR PORTION OF leftovers.
f3a116a26f Enforce WITH CHECK OPTION on DELETE FOR PORTION OF leftovers
c7e34c31e8 Fix error code for null FOR PORTION OF target
938f4f8f34 Fix assertion failures in DELETE FOR PORTION OF tuple routing
f194032168 Resolve untyped parameters in FOR PORTION OF FROM/TO bounds
98f6984e5c Don't evaluate the FOR PORTION OF target under EXPLAIN
plus release notes entries.
Discussion: https://www.postgresql.org/message-id/62bf70cc-dc33-4188-8070-a8c177bb61de%40eisentraut.org
To import attribute statistics, postgres_fdw sent a query wity COLLATE
"C" to remote servers without checking their version. Since COLLATE is
only supported since v9.1, this could break the import against older
servers; fix by modifying postgres_fdw to check their version and if
older, fall back to sampling. (This commit still allows importing only
relation statistics from such an older server.)
Also, expand the documentation to mention that attribute statistics are
stored into the local server without applying the foreign table columns'
n_distinct option.
Oversight in commit 28972b6fc.
Reported-by: Fujii Masao <masao.fujii@gmail.com>
Reported-by: Osama Abdul Qader <osamaabdulqader.cs@gmail.com>
Author: Etsuro Fujita <etsuro.fujita@gmail.com>
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwH8%2BuDTwfohpck9v_9L3nwSWACbmC36JYRyqHd5xQN_OA%40mail.gmail.com
Backpatch-through: 19
The replica identity determination was "flawed": it fell back to the
primary key if the marked index was dropped, which is a defensible
choice, but logical decoding has a differing opinion and fails to
provide usable tuple identity data in this case. "Fix" REPACK by
refusing to use the primary key in that case.
This is arguably broken behavior in RelationGetIndexList, but I'll
refrain from changing that at this stage.
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Author: Matthias van de Meent <boekewurm+postgres@gmail.com>
Author: Ewan Young <kdbase.hack@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/apCBRKCH8jwKiaSY@nathan
The general nature of this interaction has been understood (at least
by me) since before the original commit, but I failed to explicitly
document it. I also left behind an XXX in the README which I intended
to remove before commit. Fix those things.
Reported-by: Noah Misch <noah@leadboat.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Without CHECK_FOR_INTERRUPTS(), backends might not respond to query
cancellations in a timely fashion, especially for JOIN_ORDER()
specifications, where matching against each join can take O(n^2) time
in the length of the join order list in degenerate cases.
Recursive functions should check_stack_depth().
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Backpatch-through: 19
Commit 86f7c82cf1 reported the user catalog table restriction as a
detail, but the other checks in check_concurrent_repack_requirements()
were still a mix: some were hints, and the details repeated
"REPACK (CONCURRENTLY)", which the primary message already names.
Report them as details with the same "This operation ..." wording. The
one actionable hint, ALTER TABLE ... REPLICA IDENTITY USING INDEX,
stays.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Suggested-by: Alvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/apBIFWzHYOaG0auN%40nathan
Backpatch-through: 19
Previously, running REPACK (CONCURRENTLY) on a materialized view failed
with a confusing "has no identity index" error.
REPACK (CONCURRENTLY) replays changes decoded from WAL, but a
materialized view produces none, since it cannot be modified by DML and
REFRESH rewrites it through a transient heap that is not logically
decoded.
Fix this by reporting a clear error for materialized views upfront,
alongside the existing checks for system catalogs and TOAST tables, and
document the limitation.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CALj2ACXPvy4_LkQ0nHKgbZjAGK9fkDqRYUvXA3hgwxPm_X7Eng@mail.gmail.com
REPACK (CONCURRENTLY) is not an MVCC-safe operation; it doesn't
preserve the visibility information, which logical decoding needs
because it reads user catalog tables under a historic snapshot.
Disallow REPACK (CONCURRENTLY) on user catalog tables. Removing this
check requires making it MVCC-safe and logical rewrite mappings.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/apBIFWzHYOaG0auN%40nathan
Backpatch-through: 19
There's not really any good reason to store an empty advice string
in an advice stash, but it's entirely possible that some people might
do it by accident. If they do that without this fix, restarting the
system will result in failure while reloading pg_stash_advice.tsv.
The root cause is that I (rhaas) did not think clearly enough when
defining pgsa_next_tsv_field(). After returning each field on the
line, it set *cursor to the start of the next field, and after
returning all of them, it set *cursor to the trailing NUL byte. But
when the last field on the line is empty, this is ambiguous: if we're
pointing at the trailing NUL byte, it could be either because it's the
start of the zero-length field or because we're out of fields. To
remove the ambiguity, this commit decides that *cursor will instead
be set to NULL when there are no more fields.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260827171830.68.noahmisch@microsoft.com
Backpatch-through: 19
List of commits reverted:
2f094e7ac6 SQL Property Graph Queries (SQL/PGQ)
cd8844e7db Make some tests more stable by adding more explicit ordering
182cdf5aea pg_dump: Add appropriate version check
c9babbc881 Dump labels in reproducible order
040a56be4b Cleanup users and roles in graph_table_rls test
5282bf535e Fix some typos and make small stylistic improvements
720f0f89d6 Reject consecutive element patterns of same kind
288ae96872 Add a graph pattern variable only once
c5b3253b8a Property references are preferred over regular column references
a0dd0702e4 Fix cross variable references in graph pattern causing segfault
2ff289d039 Check for stack overflow when rewriting graph queries
9082680c34 Fix typos and grammar in graph table rewrite code
ac3bcc041c Fix collation of expressions in GRAPH_TABLE COLUMNS clause
9d2979dd68 pg_get_viewdef() and lateral references in COLUMNS of GRAPH_TABLE
891a57c739 Do not define type for a property graph
dc9e7c9ed9 Handle nodes that may appear in GraphPattern expression trees
f6edd8ed70 Add ORDER BY to test query to stabilize test
1190f858ea doc: Small synopsis wording change for consistency
d0eac3cafb Make spelling consistent
6827de95ee Simplify code in objectaddress.c for some property graph objects
5778fb3eaf Fix typo in error message
72498a8698 Handle element label and label property objects in object address functions
4cb2a9863d Fix LATERAL references in GRAPH_TABLE with multi-label pattern
9d8cdcbe0c Record dependencies on graph labels and properties
2a7e95b659 Readable identity strings for property graph objects
ac0ad6a7c9 Prevent dropping the last label from a property graph element
36aae3d029 Fix handling of dropping a property not associated with the given label
cc9aa7f3a9 Resolve unknown-type literals in GRAPH_TABLE COLUMNS
d868736830 Make property graph object descriptions better translatable
45d4c917ff Fix properties orphaned by dropping a label
33bfad0f3c Remove apparent support for SECURITY LABEL ON PROPERTY GRAPH
ccca2cd81b Fix pg_dump ACL minimization for PROPERTY GRAPH.
7883ea13ff Fix replace_property_refs() ignoring the root of expression tree
b820c623dd Resolve unknown-type literals in property expressions
01c544e1af Prohibit locking clauses on GRAPH_TABLE
8d2beee027 Restore the ability to use | and -> as prefix operators.
d2ac26eb9b Fix missing space before WHERE in GRAPH_TABLE deparse
4054ec5b4f Prohibit GRANT ... ON TABLE on a property graph
77964322d6 doc: Add PROPERTY GRAPH to the access privilege tables
c5e11de2f3 More tab-completion for DROP PROPERTY GRAPH
8ce749f8f6 Disallow aggregates, window functions, and SRFs in GRAPH_TABLE COLUMNS
22d0eebfef Report duplicate property and label names with a proper error
41a300ae3d Make generate_queries_for_path_pattern_recurse() interruptible
58af213874 Fix inferred property graph keys with INCLUDE columns
63f8e9773a Fix pg_event_trigger_ddl_commands for GRANT ON PROPERTY GRAPH
07df2b2532 doc: reformat GRAPH_TABLE examples
1c8c790ec1 Coerce GRAPH_TABLE pattern WHERE clauses to boolean
plus release notes entries and typedefs.list changes.
Discussion: https://www.postgresql.org/message-id/CAAKRu_bEtjWYWhYxSo0o_t3DaZYRQd5PkC0abA0g9Mp4%2BovH0w%40mail.gmail.com
When analyzing a foreign table pointing to a partitioned or inherited
table on the remote server, this method just imported relpages and
reltuples for the parent table from the remote pg_class, causing a
discrepancy or an error in results compared to the sampling method.
* In the partitioning case, while the sampling method sets relpages=0,
this method set relpages=-1. (For reltuples, the results of both
methods match.)
* In the inheritance case, while the sampling method sets
reltuples=(# of rows in the parent and child tables), this method set
reltuples=(# of rows in the parent table). (For relpages, the results
of both methods match.)
As relpages and reltuples are used for costing foreign paths, these
would lead to incorrect plans or plan changes depending on the method.
To fix, do the following:
* For the partitioning case, modify postgres_fdw to set relpages=0 to
match the sampling method.
* For the inheritance case, we could also modify it to set the correct
reltuples value by fetching reltuples for the child tables as well,
but 1) the change isn't small, and 2) table inheritance hasn't been
used that much these days, so fix by disabling this method.
While at it, adjust comments and docs slightly.
Oversight in commit 28972b6fc.
Author: Etsuro Fujita <etsuro.fujita@gmail.com>
Discussion: https://postgr.es/m/CAPmGK173b2Xvyoeq%3DY7L09H%2BajgMgxC2fYe%3D7CCunZz8tGTfFA%40mail.gmail.com
Backpatch-through: 19
FOREIGN_JOIN target sublists must contain at least two relation
identifiers. However, the parser checked only for sublists with
exactly one identifier, so FOREIGN_JOIN(()) was accepted.
Reject sublists with fewer than two relation identifiers, and add
regression coverage.
Author: Chao Li <lic@highgo.com>
Discussion: http://postgr.es/m/BEDC04E0-6732-4310-95BA-6EC34BC1442C@gmail.com
Explicitly make space for the NUL when casefolding.
No known bug in the previous code, because the previous buffer (size
12) was more than large enough for folding any codepoint with enough
room left for a NUL. The builtin provider's limit is 7; ICU's limit
seems to be 7 also; and libc always does 1:1 mappings so the real
limit is MAX_MULTIBYTE_CHAR_LEN + 1 (size 5).
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Rithvika Devisetti <devisettirithvika@gmail.com>
Discussion: https://postgr.es/m/2cfbc37ae8deccd3825e36b7f31c391cbf8ff9b8.camel@j-davis.com
Backpatch-through: 18
Tied hashes, and probably tied arrays, are capable of returning Perl
value pointers that are actually NULL, not the usual pointer to an
undef SV. We were not defending against that everywhere, leading
to possible SIGSEGV. Fix the code to consistently treat a null
pointer returned from hv_iternext or av_fetch like a !SvOK one.
(Note that the large diff in SV_to_JsonbValue is actually quite
trivial, but it required reindenting a chunk of existing code.)
Claude Code found the instance in hstore_plperl, and I found the
others by code auditing. Perhaps the other instances aren't
actually reachable, but I see little reason to assume that.
The known test cases for these errors require perl's Tie modules,
which may not be present, so it doesn't seem worth the trouble
to create regression test cases that would cover them.
Reported-by: Claude Code (via Noah Misch)
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/569769.1786901901@sss.pgh.pa.us
Backpatch-through: 14
Commit 072ee847ad taught logical decoding to discard the changes of a
transaction that is already known to be aborted when it is picked for
eviction. That path reuses ReorderBufferTruncateTXN(), which marks
every subtransaction that still has in-memory changes as
streamed. Since nothing is streamed in that path, and the top-level
transaction is never marked, a subtransaction ends up flagged as
streamed even though the output plugin has never seen it. Decoding the
subsequent abort record then makes ReorderBufferAbort() invoke the
stream_abort callback for that subtransaction.
For pgoutput this sends a Stream Abort ('A') message to a subscriber
that requested streaming = off, and it does so regardless of the
negotiated protocol version, so even a client speaking a version that
predates transaction streaming receives a message it cannot
parse. test_decoding dereferences a NULL pointer and crashes, since it
allocates its per-transaction state in the begin or stream start
callback, neither of which runs for a transaction discarded as
aborted.
This commit fixes this by marking a subtransaction as streamed only
when it has changes and its top-level transaction is already marked as
streamed. All streaming call sites mark the top-level transaction
before truncating it, so their behavior is unchanged, while the
abort-discard path never marks the top-level transaction and therefore
now leaves its subtransactions unmarked.
Backpatch to v18, where commit 072ee847ad was introduced.
Bug: #19616
Reported-by: Tyler Smart <tyler@smarts.io>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/19616-f6153af509910853@postgresql.org
Backpatch-through: 18
plperl_func_handler()'s stanza for handling an arrayref result in
a SETOF function could loop forever (or at least till OOM) when
given a tied array, since av_fetch won't necessarily ever return
a null pointer in that case. Be consistent with the other places
where we traverse a perl array: call av_len() once and use len+1
as the loop limit, silently ignoring any null pointers we get back
from that range of subscripts.
But actually, Perl's preferred locution for this seems to be to
use av_count() not av_len()+1. av_count() seems better since
there's less risk of forgetting to add 1. Also, both of those
functions return Size_t (or SSize_t) not int, creating at least
a theoretical overflow hazard. While we're modernizing this,
let's use the correct variable type where we can, and include an
overflow check where we can't.
Reported-by: Claude Code (via Noah Misch)
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Discussion: https://postgr.es/m/569769.1786901901@sss.pgh.pa.us
Backpatch-through: 14
Simple EXISTS subplans can be converted to hashed ANY subplans as
an alternative implementation. Previously, both the original EXISTS
subplan and the converted ANY subplan used names with the
exists_ prefix, making EXPLAIN output harder to read and
pg_plan_advice targets less clear.
Use the exists_to_any_ prefix for converted ANY subplans so that
their names distinguish them from the original EXISTS alternative.
This only changes the names shown by EXPLAIN and used by plan advice;
it does not affect planner behavior.
Author: Yugo Nagata <nagata@sraoss.co.jp>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: solai v <solai.cdac@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/20260605165641.3950f99ace0aad8f807abe96@sraoss.co.jp
Backpatch-through: 19
Commit 13b935cd changed hash_get_num_entries to return int64 instead
of long. This fixes a few more users of hash_get_num_entries which
were missed in the original commit.
PGSS_FILE_HEADER is left unchanged since this only affects as of yet
unreleased versions of PostgreSQL. Backpatch to v19 where the int64
change was performed.
Author: Karina Litskevich <litskevichkarina@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CACiT8iYTkc33YWaA2D3t51Y5s=GqBO7T1zX7bkpSmet2njcLLw@mail.gmail.com
Backpatch-through: 19
The option added by commit 28972b6fc was named "restore_stats", because
it was calling pg_restore_relation_stats()/pg_restore_attribute_stats()
in place of fetching a remote rowsample. However, those code paths have
been replaced with calls to
import_relation_statistics()/import_attribute_statistics() (cf. commit
54cd6fc83), so the option is now slightly misnamed.
Switch the option to "import_stats", which more closely reflects both
the functions being called internally, and the user's perception of what
the operation is doing.
Back-patch to v19 where commit 28972b6fc went in.
Suggested-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Author: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CADkLM%3DezuspTk3VD-2EM6XtFP-Dfz7cJKtnbn0cB6gocsAgu1g%40mail.gmail.com
Backpatch-through: 19
The previous commit raises an ERROR during PGP operations if OpenSSL
does not support the cipher in use. However, any existing messages
created with faulty encryption will no longer be accessible via
pgp_[sym|pub]_decrypt().
To help users out of this situation, add a new ignore-cipher-failure
option which reverts to the broken behavior during decryption only. A
faulty encryption wrapper, created by an OpenSSL configuration that does
not support the cipher, can then be stripped back off by that same
OpenSSL in order to safely reencrypt it. (Note that when OpenSSL does
support the cipher, corrupted messages will not be decrypted regardless
of the ignore-cipher-failure setting; this is unchanged.)
The new tests add a corrupted Blowfish message for both public- and
symmetric-key decryption, resulting in the following test matrix:
- Blowfish supported, default behavior: fails to decrypt
- Blowfish supported, ignore-cipher-failure: fails to decrypt
- Blowfish unsupported, default behavior: fails to load cipher
- Blowfish unsupported, ignore-cipher-failure: strips faulty encryption
The previous commit's change to the pubkey tests is expanded similarly:
correctly encrypted messages cannot be decrypted by an OpenSSL that does
not support the cipher, regardless of the option's setting, though the
failure mode will change.
Suggested-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Noah Misch <noah@leadboat.com>
Security: CVE-2026-14663
Backpatch-through: 14
PGP encryption was using px_cipher_encrypt without checking if any
error was returned. When OpenSSL is running in FIPS mode, or when
the legacy provider hasn't been loaded, not all ciphers which are
supported by the PGP code are available and fail the init step in
px_cipher_encrypt. Since the PGP encryption failed to notice this
it XORed the non-encrypted block with the plaintext, effectively
disabling the encryption.
This was found due to a report of PGP encryption not respecting
the pgcrypto.builtin_crypto_enabled flag and allowing Blowfish
and DES. This however turned out to be a false positive, since
the PGP code only use ciphers from OpenSSL and not the built in
ciphers.
Bug: #19457
Reported-by: Shishir Sharma <ansh01072001@gmail.com>
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://postgr.es/m/19457-4bab15c17aea36c7@postgresql.org
Security: CVE-2026-14663
Backpatch-through: 14
levenshtein() and levenshtein_less_equal() let the caller specify
the insertion, deletion, and substitution costs, and
fuzzystrmatch's corresponding SQL functions accept any 32-bit
integer for each. Since the distances are calculated with 32-bit
arithmetic, large costs can cause overflows, thereby producing
nonsensical results. Certain inputs to levenshtein_less_equal()
can even cause out-of-bounds writes. To fix, use 64-bit arithmetic
instead, and error whenever the final result won't fit in the
returned 32-bit integer.
We may want to teach these functions to reject negative costs, too,
but that didn't seem appropriate for a security fix, and therefore
it is left as a future exercise.
Reported-by: Ben Morris in collaboration with Claude and Anthropic Research
Author: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Security: CVE-2026-15742
Backpatch-through: 14
REPLICATION users were not previously subject to restrictions on output
plugin paths, so they were able to bypass LOAD-time protections during
logical decoding. Unfortunately, adding the standard LOAD restrictions
now would retroactively require all third-party output plugins to be
installed under the $libdir/plugins directory. This would prevent the
use of dynamic_library_path, introduce a wire incompatibility for
clients, and require all plugin authors to check that their libraries
are safe for use by any unprivileged user; we want to avoid that.
Instead, introduce an output_plugin_libraries GUC so that DBAs can
specify the output plugins that are trusted for use in logical decoding.
For simplicity, superusers are subject to the restriction as well
(though they're free to modify the GUC at will during a session, so no
power is actually lost).
The default setting is 'pgoutput, test_decoding'. If other third-party
plugins are in use, DBAs will need to modify this parameter after they
update. Some pointers have been added to the documentation to assist
with this.
Author: Jacob Champion <jacob.champion@enterprisedb.com>
Reported-by: Vladimir Tokarev <vladimirelitokarev@gmail.com>
Reported-by: Yu Kunpeng <yu443940816@live.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Backpatch-through: 14
Security: CVE-2026-6471
A grantee of amcheck function EXECUTE privilege could execute arbitrary
functions as the owners of expression indexes that depend on the search
path. An expression like (lower(col)) was not vulnerable, because
lower() is resolved at CREATE INDEX time. However, an expression
calling an sql-language or plpgsql-language function often was
vulnerable, even if it used search_path only to find objects in
pg_catalog. The amcheck documentation has been warning about data
disclosure after such a GRANT, not about function execution.
This might cause new amcheck errors when index expressions rely on a
broader search_path. Such indexes have seen errors during auto-analyze
since CVE-2018-1058 commit 582edc369c, and
v17 amcheck always worked this way. Hence, the risk is low.
Leave a comment on the one other sandbox entrance that doesn't empty
search_path. In its case, the choice was valid.
Back-patch to v14 (all supported versions), but v17 was safe already.
Commit 2af07e2f74 (v17) unintentionally
blocked the attack, and commit d70b17636d
(v18) unintentionally removed that protection. Hence, this adds to v17
just a test and a comment. While emptying search_path became more
widespread in commit 2af07e2f74 (v17),
none of its other changes blocked an attack available in v16, even when
considering GRANT. For example, brin_summarize_range() has had an owner
check that GRANT does not override.
Reported-by: 王跃林 <violin0613@tju.edu.cn>
Reported-by: Jacob Brazeal <jacob.brazeal@gmail.com>
Backpatch-through: 14
Security: CVE-2026-14673
The CACHESIGN.sign field is a BITVECP, not a TRGM, so you should not
use GETSIGN() on it. You don't get a compiler warning because the
GETSIGN() macro includes a cast. It resulted in a bogus read beyond
end of buffer, which would cause bad split decisions or a crash if
you're very unlucky.
Reported-by: Mehmet D. INCE <mehmet@mehmetince.net>
Backpatch-through: 14
Security: CVE-2026-14678
