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
Since the retain_dead_tuples feature uses a single cluster-wide
replication slot across all subscriptions, adding a new subscription on a
database with an older active transaction can require
conflict_detection_slot.xmin to move backward to avoid missing dead tuples
needed for update_deleted conflict detection.
Previously, if a newly created subscription had an older active
transaction than the slot's current xmin, the slot update occurred during
worker apply, leading to an assertion failure.
Fix this by having the launcher track the set of databases with active
tuple-retaining subscriptions. When a database joins this set (either via
a new subscription or when retention restarts), the launcher resets the
slot's xmin to the cluster-wide safe decoding horizon before starting any
workers. Because this horizon accounts for all active transactions across
the entire cluster, it serves as a safe seed without requiring retroactive
backward movement by individual apply workers.
In passing, update the retain_dead_tuples documentation to clarify two
operational boundary cases:
(a) Conflict data continues to accumulate even when a subscription is
disabled or its apply worker is stopped.
(b) track_commit_timestamp must be enabled for conflict detection to
function properly.
Reported-by: Nisha Moond <nisha.moond412@gmail.com>
Author: Zhijie Hou <houzj.fnst@fujitsu.com>
Author: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Nisha Moond <nisha.moond412@gmail.com>
Reviewed-by: shveta malik <shveta.malik@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/TY4PR01MB177182F547A62FC2666EC04EC94B72@TY4PR01MB17718.jpnprd01.prod.outlook.com
Backpatch-through: 19, where it was introduced
When outer-join identity 3 permits a join to commute with lower outer
joins, we generate multiple clone versions of its join clause, of
which only one should be applied in any given plan. When building a
join relation's restriction list, subbuild_joinrel_restrictlist
selects the appropriate clone by checking required_relids and
incompatible_relids, but no such selection was made for movable join
clauses pushed down into a parameterized path. As a result, a
parameterized scan or join could enforce more than one clone of the
same condition, shown by EXPLAIN as a duplicate qual. This wastes
effort evaluating the same condition repeatedly. What is worse, it
applies the clause's selectivity multiple times, underestimating the
result's row count.
To fix, in get_baserel_parampathinfo and get_joinrel_parampathinfo,
skip a clone clause if any outer join listed in its
incompatible_relids has already been computed below the point of
evaluation.
Back-patch to v19, but no further. While this bug goes back to v16,
applying the fix in released branches risks destabilizing plans that
users are currently happy with, and there have been no field
complaints.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/CAMbWs4-g5TFF5k=jfe3JVWqtv_cCTOMmQRUp0_E8fApgqcqBng@mail.gmail.com
Backpatch-through: 19
When neither BKPIMAGE_HAS_HOLE nor BKPIMAGE_COMPRESSED is set,
DecodeXLogRecord() checks that bimg_len needs to be equal to BLCKSZ.
However, the associated error message reported the data length
associated to a block (DecodedBkpBlock.data_len), and not the length of
the block (DecodedBkpBlock.bimg_len).
Oversight in 57aa5b2bb1, probably due to some copy-paste from the
surroundings.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAJTYsWVz9ymE7aMt2ZiQvs1bxNvhjfTf+XaJEsdFw7MT5dvtYQ@mail.gmail.com
Backpatch-through: 14
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
If a LATERAL subquery references a PlaceHolderVar that must be
evaluated at an outer join, the subquery's lateral_relids include that
outer join's relid, since the lateral parameter is the outer join's
output. join_is_legal() verified that the rels named by a proposed
join's minimum parameterization could still be joined to from outside,
but it did not consider such outer-join relids. As a result, when
identity 3 permitted a commuted join order, we could approve a join
that includes part of that outer join's required input, even though
the outer join could then be completed only above the proposed join,
leaving the lateral parameter forever unsatisfiable.
In assert-enabled builds this tripped the backstop Assert in
try_nestloop_path(). In production builds the bogus join generated
only paths whose parameterization can never be satisfied, so they
could never appear in a complete plan. This not only wasted planning
effort, but also could mislead the clauseless-join heuristics into
thinking that legality of this join means that some other join rel
need not be formed, and that could lead to failure to find any plan at
all.
Such joins used to be rejected by the have_dangerous_phv() check in
join_is_legal(), which refused any join where a PlaceHolderVar's eval
set overlapped the other side of the join without being contained in
it. Commit a16ef313f removed that restriction, exposing this
oversight.
To fix, teach join_is_legal() to reject a proposed join if its minimum
parameterization includes an outer-join relid whose outer join cannot
be formed strictly outside the join, that is, when any rel needed to
form that outer join must become part of the proposed join's own join
tree.
Back-patch to v18, where commit a16ef313f removed the
have_dangerous_phv() restriction.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49ZveAYx9T5b0=gxn0pnvqq=MbN-Z4KLbLAE+kH06mWVA@mail.gmail.com
Backpatch-through: 18
The retain_dead_tuples feature waits for concurrent transactions to be
applied before advancing conflict_detection_slot.xmin. It gets the WAL
write position from the publisher and waits until the apply remote
position passes it. This ensures that dead tuples, commit timestamps, and
origins needed for conflict detection (update_deleted,
update_origin_differs, and delete_origin_differs) are retained while
those transactions are applied.
However, with asynchronous commit enabled on the publisher, a committed
transaction may not have updated the WAL write position yet. As a result,
the reported WAL position can lag behind the latest committed transaction.
This can cause conflict_detection_slot.xmin to advance prematurely,
allowing dead tuples needed for conflict detection to be removed before
subsequent asynchronously committed transactions are applied.
Report the end of the last inserted WAL record instead. A transaction
inserts its commit record before it is marked committed, so the insert
position cannot be behind a transaction that has already committed.
Transactions that have entered the commit phase but have not yet inserted
their commit record are still accounted for by the in-commit transaction
ID reported in the same message.
In addition to above, add missing trailing period to update_deleted
conflict detail messages which are added for the same feature.
Reported-by: Nisha Moond <nisha.moond412@gmail.com>
Author: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/TY4PR01MB177182F547A62FC2666EC04EC94B72@TY4PR01MB17718.jpnprd01.prod.outlook.com
Backpatch-through: 19, where it was introduced
Previously, a session that had created temporary objects could hang during
exit if its default transaction mode was SERIALIZABLE READ ONLY DEFERRABLE
and a concurrent serializable transaction was prepared. During backend
exit, temporary-relation cleanup pushed a regular transaction snapshot,
which honored the session's default settings and could wait for a safe
serializable snapshot. So, if the conflicting transaction was prepared,
this wait could last indefinitely.
The wait occurred while the backend was already exiting and interrupts
were held off, so even pg_terminate_backend() could not cancel it. The
backend therefore remained until the prepared transaction was resolved.
Temporary-relation cleanup only needs an active MVCC snapshot for fetching
TOAST data from catalog tuples while dropping temporary objects. It does
not need a transaction snapshot affected by the user's default isolation
settings. So, use a catalog snapshot instead, which provides the needed
protection without entering the deferrable safe-snapshot wait.
Backpatch to all supported versions.
Bug: #19441
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/19441-ec29f3b1363b4a68@postgresql.org
Backpatch-through: 14
The buildfarm member akepa reported a failure in the
031_recovery_conflict.pl test.
The test checked pg_stat_database_conflicts immediately after detecting
a recovery conflict in the standby log. However, the conflict counter is
flushed by the canceled backend during backend exit, so WAL replay
completion and the log message did not guarantee that the updated
statistics are visible yet. So, previously, the test could see a conflict
counter of 0 even though the conflict had already occurred, triggering
the test failure.
Fix this by polling for the expected conflict counter instead of reading
it only once, handling the asynchronous pgstats update.
Per buildfarm member akepa.
Backpatch to v17, where this test is enabled and has the same race.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwHmiLNRfvJDAR=PmxQgf7DbzPSO1M-1RoqO8oy=t2G5KA@mail.gmail.com
Backpatch-through: 17
transformGraphElementPattern() and transformGraphPattern() ran their
WHERE clauses through transformExpr() without ever applying
coerce_to_boolean(), so a WHERE clause of any type was accepted and
its bare datum was used as the qual, which resulted in wrong results
without a diagnostic. Route both sites through
transformWhereClause(), like every other WHERE clause, and add
regression tests for the element-level and pattern-level cases.
Author: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAON2xHNZUS8ZwzVEJFeRirsC_-7EJvdbwABSzdGaJ_DF9DvHoQ%40mail.gmail.com
Not many buildfarm members run this test, since it's gated behind
PG_TEST_EXTRA=xid_wraparound, but of those that do, the slower ones
not infrequently fail. The reason seems to be that the test expects
an INSERT command to either succeed or fail, but there's a window
where it can succeed while issuing a warning about impending
wraparound. poll_query_until treats nonempty stderr as a failure,
so it loops an extra time until the INSERT succeeds with no warning.
There seems no reason to treat this behavior as wrong, so adjust
the test to accept it.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Diagnosed-by: Alexander Lakhin <exclusion@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/7a68bee2-91bd-481d-be44-160e6beebc83@gmail.com
Backpatch-through: 17
Commit f63ae72bbc converted postgresql.conf.sample to spaces so
that it lines up at any tab width, but replace_guc_value() still
pads with tabs when it re-aligns the trailing comment of a
rewritten setting. Fix that.
Oversight in commit f63ae72bbc.
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/apRezMywHRuR5h-6%40nathan
Backpatch-through: 19
Commit 44fb59fc6 taught the grouping-conflict walker to treat the arg
of a simple CASE as a direct operand of each WHEN comparison, but it
only checked the collation, on the assumption that the WHEN operator
is always the type-default "=" and thus matches the grouping eqop.
That assumption fails once the arg is relabeled to another type: the
WHEN then compares under that type's "=", which need not agree with
the grouping equality. For instance, with a DISTINCT over a citext
column, a qual such as "CASE t::text WHEN 'A' THEN ..." was pushed
below the Unique, although the equivalent "t::text = 'A'" is correctly
kept above it.
Instead of special-casing the arg, have the walker bind a Var arg
while walking the WHEN conditions and resolve each CaseTestExpr to it,
so that the arg is checked exactly as each WHEN uses it: with the
opfamily and collation checks of a direct operand when the WHEN is a
comparison, and as a non-operand reference otherwise. A non-Var arg
is walked once as a non-operand, as before. The CaseTestExpr in an
ArrayCoerceExpr's elemexpr and a JsonConstructorExpr's coercion stand
for something else and are left alone.
Reported-by: Tender Wang <tndrwang@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/CAHewXNkvGTOgijRLjmudpg=wz0d-J8ChY2o7ksh3Be+Q_Bxwog@mail.gmail.com
Backpatch-through: 18
When recovery starts from a backup without a signal file, pg_subtrans
is not started at the beginning of recovery and remains unstarted
throughout recovery. However, previously, a restartpoint run by
the checkpointer during recovery nevertheless tried to truncate
pg_subtrans, triggering the assertion failure:
TRAP: failed Assert("TransactionIdIsValid(initial)")
This commit fixes this by tracking whether pg_subtrans has been
started during recovery, and have the checkpointer check this flag
before truncating pg_subtrans at restartpoints.
Backpatch to all supported versions.
Reported-by: Imran Zaheer <imran.zhir@gmail.com>
Author: Imran Zaheer <imran.zhir@gmail.com>
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA+UBfamzfEReT0VOGRUW_=_AecCYQ-CNKLR_T4Q+YEmJAH3fdg@mail.gmail.com
Backpatch-through: 14
Several queries in the foreign_key test sort pg_constraint rows with
"ORDER BY oid::regclass::text". These OIDs are constraint OIDs, not
relation OIDs, so the regclass cast renders them as numeric strings,
and text comparison of numeric strings depends on digit count. If the
OID counter crosses a power of ten while the test runs, the output
order flips, causing spurious test failures. This cannot happen in a
fresh cluster, but installcheck against a long-lived cluster can hit
it.
To fix, sort by constraint name, which is unique within each of these
queries, and use COLLATE "C" so that the order does not depend on the
database's default collation either.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49KaJFGs4rfBWk+YmHPTXZ+pJTMJ12iaz7ruZdSK+sC5w@mail.gmail.com
Backpatch-through: 18
When reduce_outer_joins reduces an outer join to an antijoin, any IS
NULL qual on a Var from the antijoin's nullable side is necessarily
true. Previously, such quals were discarded later in
distribute_qual_to_rels, mainly to avoid bogus selectivity estimates.
But that discard was incomplete: the qual remained in the jointree,
while its Vars were not counted in attr_needed. Since commit
2ebf25e7d, join removal edits the jointree and expects it to contain
no other references to a removed rel, so it could remove a rel that
such a discarded qual still references, and then trip an assertion on
the qual's stale Var.
To fix, move this processing to an earlier phase: such quals are now
removed from the jointree by reduce_outer_joins itself. This way
later phases see a consistent query tree, and
check_redundant_nullability_qual is no longer needed, so remove it.
Back-patch to v16, as with commit 2ebf25e7d.
Reported-by: Tender Wang <tndrwang@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAHewXNk8b0TsSy4dL=CO7FXL2W3WBm0BcdP-zwNJePa-Qj4HzA@mail.gmail.com
Backpatch-through: 16
Several invocations decoded and printed the full WAL range even though
the test only needs to confirm that a command form works or that
decoding reaches a specific error. Limit those checks to one record,
and start the fall-off-the-end checks near the end of the generated WAL.
This change reduces the IPC overhead overall, particularly on Windows,
without reducing coverage.
I am usually hesitant to backpatch such changes as this is only an
improvement, but the gains are too good in terms of IO and runtime, for
both the CI and the buildfarm. Based on the numbers provided, the CI
takes 30% less time to run the test with this change on Windows (worst
case shown on the lists). These tests have been introduced in
96063e2836.
Author: Sehrope Sarkuni <sehrope@jackdb.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAH7T-araSEdsNpxiKaMOW8kr_ZHhekCLHx5yzm2ksq4FdgULbA@mail.gmail.com
Backpatch-through: 17
While some <refentry> instances of supported PostgreSQL versions use the
sql-alter-table style, sql-altertable is the dominant style. Change
these before they're baked into the doc URLs of a supported version.
Backpatch-through: 19
After adding pg_stats.tableid, we taught pg_dump to use it in the
attribute statistics query for servers running v19 or later.
Notably, we did not give this new form of the query a "redundant
filter clause" like we did the pre-v19 form. Testing indicates
that produces a significant performance regression, and that adding
a similar filter clause to the v19-or-later form of the query fixes
it.
Oversight in commit 4b5ba0c4ca.
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/apQ9nvLW_zFdarYd%40nathan
Backpatch-through: 19
Sufficiently large settings of the *_freeze_score_weight parameters
can lower the scaling thresholds below the *_freeze_max_age
settings. This can lead the scoring code to raise a value < 1.0 to
a power > 1.0, thus lowering the score, which is the opposite of
what is intended. To fix, only exponentiate the scores when doing
so won't lower their values.
Oversight in commit d7965d65fc.
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/apWcbq91BAfhmsKU%40nathan
Backpatch-through: 19
The SIMD path for these commands reads ahead into the input buffer
via CopyLoadInputBuf(), which reports encoding and conversion
errors. The problem is that the errant data might lie beyond the
end-of-copy marker, which we aren't copying and thus needn't error
for. To fix, add a "speculative" parameter to CopyLoadInputBuf()
that defers these errors, and use it in the SIMD path. The errors
are instead reported only if we actually reach the errant data.
Testing indicates this does not meaningfully impact performance.
Oversight in commit e0a3a3fd53.
Reviewed-by: Greg Burd <greg@burd.me>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/apWOr1kgVSqweBnU%40nathan
Backpatch-through: 19
We must handle cases where an inserted temporal leftover gets routed to
a different partition than the original tuple's. This happens if the
partition key depends on the application-time column used in the FOR
PORTION OF clause. Everything works for UPDATE, but for DELETE we can
hit some Asserts, since that operation never needed to be covered
before. One Assert is for the RETURNING list; the other, for WITH CHECK
OPTION. This commit updates both to include CMD_DELETE.
Author: Paul A Jungwirth <pj@illuminatedcomputing.com>
Reported-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/03276924-6d55-470d-8192-a49c053c940f@eisentraut.org
The test expects a 128 kB request to exceed the shared memory available after
startup. Huge pages can round up the main shared memory segment enough for
that request to succeed, defeating the test.
Disable huge pages for the allocation-failure phase so that it exercises the
intended error path independently of shared memory segment rounding.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/rpkhzrrmo7qiz7rr3qlufksb2oci33wcg3ao75le7dqindgoif@6yozkjwb6bdf
Backpatch-through: 19
eval_const_expressions() didn't constant-fold XmlExpr. In some cases
it cannot, but often it can. Failure to do so can cause visible
regressions in SQL-language function behavior compared to pre-v18,
though only in a rather narrow set of contexts: basically, if you're
trying to use a CASE to prevent evaluation of a failure-prone XML
function.
While we're at it, make contain_mutable_functions() handle XmlExpr
more precisely, and adjust some comments that justify not treating
XmlExpr explicitly.
This seems worth sneaking into v19, but I'm hesitant to put it into
v18. It's not really a bug fix, because we disclaim the safety of
using CASE this way, and it carries some risk of de-optimizing queries
that worked satisfactorily before.
Bug: #19487
Reported-by: Ilya Portnov <i.portnov@compassplus.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19487-367258bc497b923a@postgresql.org
Backpatch-through: 19
The VALUES reference page said that an AS clause was required when
VALUES is used in a FROM clause. This was imprecise because AS is
optional when specifying an alias.
In v14 and v15, a table alias is still required in this case, so state
that explicitly instead. In v16 and later, commit bcedd8f5fc made table
aliases for subqueries in FROM clauses optional, so state that the
table alias is optional there. In all branches, continue to recommend
explicit column aliases as good practice.
Backpatch to all supported versions.
Author: Ian Barwick <barwick@gmail.com>
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAB8KJ=jrvZdNLLhYSiCgbjLTz2LKEjYw+-HVQkaF+kgk61KtHA@mail.gmail.com
Backpatch-through: 14
The test for disabling data checksums after a failure didn't wait
for the state transition to complete before testing, which could
lead to false negatives in testing. Fix by passing the wait param
when disabling. Identified on buildfarm member culicidae.
Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/b2f34fd3-8e98-450e-9e8c-ac08238e9116@gmail.com
Backpatch-through: 19
A negative n means "return all but the first |n| characters", so
text_right() negates n before clipping. Negating PG_INT32_MIN
overflows; with -fwrapv the result is PG_INT32_MIN again, still
negative, and pg_mbcharcliplen() then returns an offset of zero,
so the whole string is returned where the correct answer is an
empty string:
SELECT right('abcdef', (-2147483648)::int4); -- 'abcdef', want ''
SELECT right('abcdef', -2147483647); -- '', correct
Clamp to PG_INT32_MAX instead. Any n whose absolute value is at
least the string's length skips all of it, and a text value cannot
be longer than PG_INT32_MAX, so this gives the same answer for every
other input. Erroring out, as text_format_string_conversion() does
for a width of INT_MIN, would not be correct here: unlike a format
width, an out-of-range skip count has a well-defined result.
Using pg_neg_s32_overflow() would be a slightly more optimal fix but
as it's only available in PostgreSQL 18 and later the decision was
taken to apply the same fix to all backbranches.
Backpatch to all supported versions.
Author: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CAON2xHNnBz-AcPJgDmd5_39+8qR5AUKEZk4X3ZM-0zdsATn8kQ@mail.gmail.com
Backpatch-through: 14
When to_char() formatted an integer value with a V pattern, it could
return an incorrect result instead of reporting an overflow. V shifts
the decimal point by multiplying the input value by a power of ten before
formatting it, so, for example,
to_char(3, '9V999999999')
requires computing 3 * 10^9. This result does not fit in int4, but
the integer variant of to_char() performed the multiplication using a
plain int32 expression. The intermediate result could therefore
overflow, causing the function to output incorrect digits instead of
raising "integer out of range".
Use dtoi4() and int4mul() for this calculation so that both an
out-of-range multiplier and an out-of-range product are detected, as
with ordinary integer arithmetic. This also matches the existing int8
implementation, which uses dtoi8() and int8mul() for the same
operation.
After this change, to_char() with V format either returns the
correctly formatted result when the scaled value fits in int4, or
raises "integer out of range" when it does not.
Backpatch to all supported versions.
Reported-by: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Miłosz Bieniek <bieniek.milosz@proton.me>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAB8bMivEfqZxOVdzc3kZDN++XshmkEz2t7dfGBU8+oUm864EZg@mail.gmail.com
Backpatch-through: 14
The compound flags collected from COMPOUNDFLAG and friends are stored in
either the string or the integer member of a union, according to the
flag mode that the affix file's FLAG line declares. NIImportOOAffixes()
converted each flag as soon as it read it, using the mode in effect at
that point, and recorded that mode in the entry. Since FLAG may appear
anywhere in the file, including after the compound flags, entries written
before and after it could disagree about which member of the union holds
the flag.
In assert-enabled builds, this would result in an assertion failure.
Otherwise, cmpcmdflag() takes the mode from its first argument and
applies it to both, so it can read an integer as a char pointer and pass
that to strcmp(). Depending on which way the mismatch goes, the result
is a segfault while sorting the array, a segfault in the bsearch() that
later looks flags up (the lookup key is built with the final mode, so
this happens even when the array itself is consistent), or, when both
members happen to be readable, no crash at all and a compound flag that
is never found, which silently disables compound word splitting. This
isn't a security bug because we consider dictionary files to be trusted
data, but it's still worth fixing. (In practice, dictionary files
usually put the FLAG line first, which is why this went unreported for
so long.)
Fix by keeping the flags as strings while the file is read and converting
them once it has been read in full, when the mode is final. This also
makes the position of the FLAG line irrelevant, which is how the flags on
AF, SFX and PFX lines are already treated: those are parsed in a second
pass and so always use the final mode. That precedent is reason for
behaving this way rather than throwing an error.
The old ispell file format reaches addCompoundAffixFlagValue() too,
from NIImportAffixes(), and returns without entering NIImportOOAffixes(),
so it needs the conversion step as well.
While we're here, also fix some integer width mismatches: store the
result of strtol() into a "long", and cast to int only after we've done
range checks. Typically a value too wide for int would fail the range
checks anyway, but in some cases it would be silently accepted after
truncation to int.
Author: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAON2xHN3QmsaySM6DGWa1gttcbJoFh0wjAE-_ZpSPo=LKN1hYw@mail.gmail.com
Backpatch-through: 14
The REPACK grammar accepts ONLY before the table name and * after
the table name, but that's neither documented nor handled in the
code. Perhaps REPACK should support that syntax, but for now let's
just bring it in line with its documentation.
Oversight in commit ac58465e06.
Reviewed-by: Antonin Houska <ah@cybertec.at>
Discussion: https://postgr.es/m/apBTsWGwkLXVh8Ow%40nathan
Backpatch-through: 19
AutoVacuumUpdateCostLimit() runs after each nap in vacuum_delay_point()
and follows av_nworkersForBalance, but the new limit never reached the
shared cost params in the vacuum DSM: propagation only required config
reload. Parallel workers computed their delays from the stale limit,
so a parallel autovacuum could run at up to twice the configured budget
(or half of it) until the next SIGHUP, contradicting the propagation
promise in maintenance.sgml.
Call parallel_vacuum_propagate_shared_delay_params() after rebalancing.
Gated on the leader: parallel workers take the same nap path and must
not overwrite the shared parameters.
This also adds a test to validate the behaviour: pause the leader at the
existing injection point, start a second autovacuum worker and hold it at
a new injection point placed after it joined the balance, then check the
first parameter load of the parallel workers reports the balanced limit.
The hold is needed because a second worker left running can finish its own
vacuum before the leader resumes, which puts the balance back where it
started. Autovacuum is disabled for everything but the two test tables
via thresholds, as a worker spawned by catalog churn would get trapped at
the hold point and starve the test of its second worker slot.
Backpatch to v19 where autovacuum gained the ability to use parallel
vacuum workers.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAN4CZFOZtEPwGQ6oa9LvHvN522zEp8h_dW9hHExSh7pVXofoKQ@mail.gmail.com
Backpatch-through: 19
In v19, pg_stat_autovacuum_scores computes a TOAST table's scores
from its own storage parameters alone. On the other hand,
autovacuum falls back to the main table's parameters when the TOAST
table has none. This means that the view may report scores that
don't match what autovacuum would calculate. This contradicts the
documented promise that the view generates its results the same way
autovacuum workers do. To fix, teach the view to do the same
fallback. As in do_autovacuum(), we must make a preliminary pass
over pg_class to collect the main tables' parameters, since the
pg_class scan may see TOAST tables before their main tables.
Commit fad70a09ff for v20 improved autovacuum's handling of TOAST
storage parameters and adjusted the view to match, but it was
deemed too intrusive to back-patch. This fix is for v19 only.
Oversight in commit 87f61f0c82.
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoB1CJRVfCDh8qYuD3eueiygXxk7F3nybgjN0RZXSD-QUw%40mail.gmail.com
Backpatch-through: 19 only
analyzejoins.c decided which joins could be dropped by consulting the
planner's derived data structures, but then implemented the removal
by updating those structures in-place. That is a lot of fiddly work,
and nothing keeps it in step with the rest of the planner:
remove_leftjoinrel_from_query only bothered to update "parts of the
planner's data structures that will actually be consulted later", with
no good way to know what those are. Bug #19560 is one consequence.
In that report, removing a join leaves an EquivalenceClass that now
gives rise to a base restriction clause, but base restriction clauses
have already been generated and nothing reconsiders them, so the WHERE
condition disappears from the plan and we return wrong answers.
The self-join elimination code has the same design and the same type
of hazard. We have seen many related bugs over the years too, so it's
time to do something drastic.
To fix, do the removals by editing root->parse->jointree (which is a
far simpler and more stable representation than the derived data),
and then have query_planner() discard everything it computed from the
jointree and derive it over again. This requires quite a bit less
code, and doesn't require touching analyzejoins.c every time we change
the data derived by query_planner(). For typical cases it can actually
save a bit of planning time, though in cases where we have to iterate
the derivation loop many times it does add some time.
reduce_unique_semijoins() gets the same treatment: rather than deleting
the semijoin's SpecialJoinInfo and relying on the jointree not being
consulted again, it now changes the JoinExpr's jointype to JOIN_INNER
and recalculates everything.
Some plans change in the join regression test. Qual evaluation order
shifts in a few cases, because the conditions now reach later planning
in jointree order rather than in whatever order the removal code
re-distributed them. A few plans improve, since the rebuilt relation
targetlists no longer carry columns that only a removed join needed.
We also detect a constant-false filter condition whose test used to
carry a FIXME label. One plan gets marginally worse, because the old
code recomputed attr_needed from equivalence classes after a join
removal; that is more accurate than what deconstruct_jointree()
derives from the original clauses, but we no longer do that. Making
that recomputation happen anyway could be worth doing, but it should
be considered independently and perhaps implemented differently.
Back-patch to v16, on the grounds that the introduction of
varnullingrels in v16 made the old approach significantly more complex
and bug-prone; notably, bug #19560 does not manifest before v16.
In released branches, do not remove externally-visible fixup
functions such as remove_join_clause_from_rels, in case any
extensions are relying on them; but they're no longer used by core
code. But we must nonetheless break API/ABI for remove_useless_joins,
reduce_unique_semijoins, and remove_useless_self_joins, as those now
have different outputs and very different behavior than before.
It seems unlikely that any extensions are calling those; but just in
case, make the breakage more obvious by renaming remove_useless_joins
to remove_useless_outer_joins, which is a more sensible name for it
anyway since the addition of remove_useless_self_joins.
Full disclosure: initial drafts of this patch were made with
Claude Opus 4.8.
Bug: #19560
Reported-by: Orestis Markou <orestis@orestis.gr>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Thom Brown <thom@linux.com>
Reviewed-by: Jacob Brazeal <jacob.brazeal@gmail.com>
Discussion: https://postgr.es/m/1186816.1784573544@sss.pgh.pa.us
Backpatch-through: 16
If SHMEM_CALLBACKS_ALLOW_AFTER_STARTUP is used to allocate shared
memory after startup, but the initialization fails half-way through,
the shmem area is left in an indeterminate state. Furthermore, if
multiple shmem areas are registered in one RegisterShmemCallbacks()
call, some might be allocated while others are not.
This commit adds an explicit 'initialized' flag to each shmem area.
We still leave behind an uninitialized area on error, but at least
they are now clearly marked, and you get a slightly nicer error
message if you try to re-register them. It'd be nice to clean up more
thoroughly and support actually retrying the allocations, but in
practice, the most likely reason for a shmem allocation or
initialization to fail is that you are out of shared memory and
retrying wouldn't help with that.
This isn't exactly a new problem, the old ShmemInitStruct() interface
had similar issues if the initialization code failed, or if you
allocated multiple structs and some allocations failed. It was just
left to the calling code to deal with it.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAJTYsWVRRWH48=PcuAo_2Y4Ap6M0QRmzxgUfFkNRtdWK74LjBQ@mail.gmail.com
Backpatch-through: 19
RegisterShmemCallbacks() left the backend in a bad state, if an error
occurred in the callbacks or if an allocation failed. Firstly,
'shmem_request_state' was left in wrong state, causing a subsequent
call to RegisterShmemCallbacks() to wrongly take the postmaster
startup codepath or assertion failures in some other functions.
Secondly, the 'pending_shmem_requests' list was not properly cleaned
up, causing a subsequent RegisterShmemCallbacks() to try to process
the stale, already-freed requests.
To fix, add a PG_TRY() block to clean those things up on error.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAJTYsWVRRWH48=PcuAo_2Y4Ap6M0QRmzxgUfFkNRtdWK74LjBQ@mail.gmail.com
Backpatch-through: 19
collate.linux.utf8 skips itself unless version() matches "linux-gnu",
and infinite_recurse skips itself when version() matches
"powerpc64[^,]*-linux-gnu". configure substitutes the GNU host triplet
into that string, but the meson build composes it from
host_machine.cpu_family() and host_system, which never carries the ABI
suffix. So ever since meson support arrived in 16, collate.linux.utf8
has not run at all on a meson build, and infinite_recurse has been
running on ppc64 Linux the very case it means to stay away from.
Meson documentation says it reports 'ppc64' instead of 'powerpc64'.
Fix by matching "-linux[-,]" and "p(ower)?pc64[^,]*-linux", which match
all spellings. Keeping the punctuation on either side confines the
match to the platform field. Neither pattern excludes musl, but
collate.linux.utf8's other conditions already require a set of glibc
locales to be present.
Backpatch to 16, where the meson build was introduced.
Discussion: https://postgr.es/m/a40b19da-9a02-47b4-8afd-2bbbde8db1e8@dunslane.net
Reviewed-By: Jonathan Gonzalez V. <jonathan@abdiel.eu>
Reviewed-By: Nazir Bilal Yavuz <byavuz81@gmail.com>
create_append_path() skips cost_append() when an Append has exactly
one child whose parallel awareness matches its own, since setrefs.c
strips such an Append out entirely. In that case it copies the
child's rowcount and costs directly, but it failed to copy
disabled_nodes. An Append over a disabled child therefore claimed to
contain no disabled nodes, letting a disabled path win over one that
is not disabled.
This is a regression in v18; before e22253467, disable_cost was folded
into a path's startup and total costs, so it rode along in the fields
this shortcut already copies.
Back-patch to v18. This can change plans in stable branches, but only
for installations that have explicitly disabled a node type, and only
to stop using the node they asked us to avoid.
Reported-by: Man Zeng <zengman@halodbtech.com>
Author: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAHewXNm_Zx5EDoaD7wo7bq6cfRroNznS+RBCzT_p2-CWQXpgSw@mail.gmail.com
Backpatch-through: 18
When performing partition pruning with a RANGE partitioned table where
the pruning quals are only present for a leading prefix of the partition
key, it was possible that partition pruning would accidentally prune away
some partitions which shouldn't be pruned and include some partitions that
were not needed.
This happened due to an incorrectly coded loop bound which was
terminating the loop when the bound reached the first or last element in
the partition bound array. This resulted in those end elements not being
checked in cases where they should be checked. It appears that it might
have been coded this way to avoid stepping off the array, but that was
done incorrectly as it failed to take into account the direction of travel
through the array (the loop can go forwards or backwards). I.e., it's
valid to loop when 'off' is the last element if we're going backwards
through the array, and valid to loop if 'off' is 0 and we're looping
forward through the array, but the code as it was didn't allow that.
Here we fix this by moving the loop condition check to after we've
calculated the array element to process, and break from the loop if that
element is beyond either end of the array.
Example of accidentally pruned partition:
p: partition by range (a, b);
p1: for values from (1, 4) to (1, 7);
p2: for values from (1, 7) to (3, 8);
p3: for values from (4, 8) to (6, 9);
def: default;
select * from p where a <= 1;
Here p2 was pruned by mistake.
Example of accidentally not pruning a partition:
p: partition by range (a, b);
p1: for values from (7, 2) to (7, 7);
def: default;
select * from p where a > 7;
No partitions would be pruned in this case, despite it being impossible
for matching rows to exist in p1.
Author: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/CAApHDvp5ne9AWaH-tG1Lke-USLz3NwWLWTUdP5NT7ypKtcFqcg@mail.gmail.com
Backpatch-through: 14
