tsqueryrecv() calls findoprnd() even when the input has no query
items, so it fails on an empty tsquery, which is legal data.
This breaks, for example, binary COPY of a table containing an
empty tsquery.
Skip the operand walk for an empty query, as parse_tsquery() does.
Bug: #19778
Reported-by: Shay Rojansky <roji@roji.org>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19778-1befc1fba593ab1b@postgresql.org
Backpatch-through: 14
The function is declared in pg_proc.dat a bigint as its second input
argument, but the function's code used PG_GETARG_INT32().
This is an issue in 32-bit architectures in stable branches (up to v18)
where bigint is still passed by reference, reporting bogus numbers.
Since 2a600a93c7 (v19 and HEAD), this is less of a problem as Datums
became 8-byte wide everywhere.
Oversight in ab0dfc961b.
Author: Markos Fountoulakis <markos@planetscale.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/e791bbb7-af3a-4a91-a5bb-c744e5c3ef54@planetscale.com
Backpatch-through: 14
Commit 1e707d3256 made COPY TO report a proper error for unlogged
tables during recovery. currtid2() has the same problem: it reads the
table directly without going through the planner, so on a standby it
failed with a low-level error for an unlogged table:
ERROR: could not open file "base/5/16384": No such file or directory
Check for this case and report the same error as SELECT. No test is
added, as currtid2() is an undocumented internal function.
Backpatch to all supported versions, like commit 1e707d3256.
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
Author: Jim Jones <jim.jones@uni-muenster.de>
Discussion: https://postgr.es/m/CAD21AoDNa7pL760_23AN_wYPMkWXCt+Kw0_vJAcci4R5DoCQGg@mail.gmail.com
Backpatch-through: 14
Unlogged tables have no data on a standby, so reading one there must
fail. SELECT and COPY (SELECT ...) TO already report a clear error,
but COPY with a table name read the table directly and failed with a
low-level error instead:
ERROR: could not open file "base/5/16384": No such file or directory
Check for this case in COPY TO and report the same error as SELECT. On
v19 and later, also check each partition when copying from a
partitioned table, so that an unlogged partition is reported too.
Backpatch to all supported versions, since the low-level error looks
like data corruption.
Author: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/b6d71815-d362-4579-bf07-73eca01aee6d@uni-muenster.de
Backpatch-through: 14
The main portion of findtimezone.c's win32_tzmap[] is now directly
generated from the CLDR project's current windowsZones.xml file (using
a script available in the discussion thread, but not yet merged into
our tree). Windows zone names that are no longer present in CLDR are
kept, but are moved to a separate list at the end of the array to
simplify future management.
The important changes that this brings are inclusion of new zone names
for Canada's upcoming timekeeping changes. In addition, this tracks a
few minor changes that CLDR has made in other entries:
"Central Asia Standard Time" now maps to Asia/Bishkek, and
"Mountain Standard Time (Mexico)" now maps to America/Mazatlan.
Reported-by: CyberNative AI <ceo@cybernative.ai>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/179219370621.3670768.16110484606338109870@cybernative.ai
Backpatch-through: 14
When prepare_sort_from_pathkeys() cannot find a pathkey's expression
in the input plan's targetlist, it adds a resjunk entry computing an
EquivalenceClass member's expression. It copied that expression
as-is, without replacing lateral references with nestloop Params the
way build_path_tlist() does for the rest of the targetlist.
This matters when the sort key contains a PlaceHolderVar whose
expression has a lateral reference, and the sort is placed directly
atop the plan node that evaluates the PlaceHolderVar. The targetlist
already contains the PlaceHolderVar, so the sort key is considered
computable, but the entry added for it still has the outer relation's
Var inside the PlaceHolderVar. If the node is a join, setrefs.c fails
with "variable not found in subplan target lists". If it is a scan,
setrefs.c leaves the Var in place, and the executor fetches it from
the scan tuple, leading to wrong results.
To fix, pass root to prepare_sort_from_pathkeys() and apply
replace_nestloop_params() to the expression before adding it to the
targetlist.
Back-patch to all supported branches.
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49LWNm7_OxuW1V71wLi+Ev3KbfY1bMY36uydNKwSPnbtQ@mail.gmail.com
Backpatch-through: 14
When building sort keys for a child relation, such as the per-child
sort columns of a MergeAppend over partitionwise child joins,
prepare_sort_from_pathkeys() may need to compute an EquivalenceClass
member from the child's targetlist. find_computable_ec_member()
accepts a member if all the Vars and quasi-Vars it contains appear in
that targetlist. It ignores child members that don't belong to the
given relids, but it can still choose a parent member. Usually that
is harmless, since a parent member's Vars never appear in a child's
targetlist. PlaceHolderVars are different: equal() ignores their
phexpr and phrels, so a parent PlaceHolderVar matches its translated
copy in the child's targetlist. As a result, an untranslated parent
expression could be added to the child plan's targetlist, leading to
"variable not found in subplan target lists" errors from setrefs.c.
To fix, when relids is given, also ignore parent members that don't
belong to it. This changes nothing for a non-child relation, because
any member computable from its targetlist belongs to it anyway. When
relids is NULL, only child members are filtered, as before.
Also update the comments for prepare_sort_from_pathkeys() to reflect
that 'relids' can identify a non-child relation being sorted, which
has long been the case.
Back-patch to all supported branches.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: wenhui qiu <qiuwenhuifx@gmail.com>
Discussion: https://postgr.es/m/CAMbWs48bLLEGy6YC+n01w6=YR=28cj_kpoKcAgOHQR7jbtLb6w@mail.gmail.com
Backpatch-through: 14
Dumps taken from unpatched versions (pre-v14) will not have the AS
keyword that ruleutils.c now prints, causing a harmless diff in dump
output. Adjust the cross-version upgrade's old dump to make it match.
While I'm here, move the stanza that I hastily put in a non-optimal
place in commit f8ab6c622. It's worth fixing because it messed up
the logical structure of adjust_database_contents, making it
un-obvious where to put future additions.
Per buildfarm.
isSimpleNode() and related code treated every XmlExpr as
self-delimiting. Most of them are, because they look like function
calls with some extra decoration. But "x IS DOCUMENT" is not: it is
a postfix test, like IS NULL. Printed as the operand of another
operator, or as the operand of a postfix test, it was therefore not
parenthesized and could not be parsed back, or was parsed as something
unintended. Under-parenthesization occurred even in non-pretty-print
cases, so that this could break dump/restore workflows.
To fix, treat IS DOCUMENT as neither simple nor function-like, and
give it the same treatment as other postfix tests.
Also pass the single argument of IS DOCUMENT, not the argument list,
to get_rule_expr_paren(). This avoids unnecessary parenthesization
of the argument.
Author: Henson Choi <assam258@gmail.com>
Reviewed-by: Prashant Chinnam <crprashant@hotmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAAAe_zDuPSkc8ihCctV67Lxbqock+S4wqCHHo8xxZ1t4teXo0g@mail.gmail.com
Backpatch-through: 14
If the target relation of UPDATE, DELETE, or MERGE has an alias,
we printed that "bare" (without AS or forced quoting), the same
as we do in FROM. However, if that alias is "set", there's an
ambiguity with the SET keyword that must follow the relation
specification. The parser would choose the wrong interpretation,
so that the dumped query would fail to reload. The least surprising
and most future-proof fix seems to be to always print AS in these
contexts, as we already did in INSERT.
Also, we were insufficiently careful about parenthesizing the
left-hand argument of a SubLink (IN, op ANY/ALL, or row comparison),
so that the printed expression might not parse back with the intended
meaning depending on operator precedence. This is only a problem in
"pretty-print" mode, since non-pretty mode fully parenthesizes every
operator. So it isn't a hazard for pg_dump, but it still seems like
something to fix. To do so, switch to using get_rule_expr_paren.
Presently that will add parens in some cases where it wouldn't really
be necessary, but that's better than the reverse, and it arguably
is helpful for readability anyway.
Author: Henson Choi <assam258@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAAAe_zDqXin5NeB5atRCbZqUdcMOeyXRqhan+tVp0F30OCURnw@mail.gmail.com
Discussion: https://postgr.es/m/CAAAe_zCy+D45Qd_br484F+V34y0BVA=hp_7cb=u6HysyMkE3UQ@mail.gmail.com
Backpatch-through: 14
The pattern seems to be to EXPLAIN the query to ensure we get the
expected partition-wise aggregate plan, then run the same query to check
the results are what we expect. A couple of places didn't use the same
query between the EXPLAIN and the checking of the results, which
effectively voided the plan check.
Additionally, a comment claimed array_agg() didn't support partial mode,
but that was made not true in 16fd03e95.
Author: Matthias van de Meent <boekewurm@gmail.com>
Reviewed-by: Pierre Forstmann <pierre.forstmann@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAEze2WgjJUiqV-VZVrWqYz+ETLtd4oKk2gbvBWxf1Fo0is5XVQ@mail.gmail.com
Backpatch-through: 14
AllocSet contexts with different maxBlockSize values can have the same
freeListIndex. When reusing a context from one of the context_freelists,
we updated maxBlockSize, but we failed to adjust allocChunkLimit, even
though that ordinarily depends on maxBlockSize. This could result in
some allocations being put on an external block when they shouldn't have
been, resulting in excessive malloc() calls, or if the allocChunkLimit was
left set higher than it should be, allocations could have resulted in
memory not being free()'d after pfree() due to the chunk having been put
on a multi-chunk block (these would still later be cleaned up when the
context is reset, so no memory leaks).
The repercussions of this are only inefficient use of memory and perhaps
some performance instability depending on which context happened to be
top of the freelist when a new AllocSet context was created.
To fix, we move the allocChunkLimit calculation code into a helper
function and call that to reset the allocChunkLimit when reusing a
context.
Author: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Donghang Lin <donghanglin@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/1D54961E-6BEF-411A-BFAD-610C3B18D7E7@gmail.com
Backpatch-through: 14
Commit a83586b554 decided to scribble on the caller's fcinfo but failed
to take into account that const arguments are written once at expression
init and assumed never to change.
The use_json_null branch overwrote fcinfo->args[2] with a jsonb null
built in the per-tuple expression context. In later rows,
args[2].isnull is still false and args[2].value still holds that
pointer, but the per-tuple context has been reset. Later rows find
args[2].isnull = false, short-circuit past the NULL handling at the top
of the function, and jump straight to jsonb_set with the stale pointer
regardless of their own null_value_treatment argument, which is never
read.
To fix, call jsonb_set through its own call frame instead. Arguments 0,
1 and 3 were already checked non-NULL above, so we can use
DirectFunctionCall4.
Add regression tests covering use_json_null followed by delete_key and
return_target, and a multi-row default-treatment query.
Reported-by: Evan Si <evsi@amazon.com>
Discussion: https://postgr.es/m/CANWCAZY-PS=ev8TiiV1j=NoR2xLopEL8fDq0fyQZahHiP5b+Pw@mail.gmail.com
Backpatch-through: 14
When remove_useless_result_rtes() removes an RTE_RESULT, any
PlaceHolderVars that were to be evaluated there must be evaluated
somewhere else. If the RTE_RESULT is the nullable side of an outer
join, there is no other place that would preserve the PHVs' nulling
behavior, so find_dependent_phvs() refuses the removal if any such PHV
exists anywhere in the query. If the RTE_RESULT is a child of an
inner join, its PHVs can instead be evaluated at the join of all its
sibling rels, unless a sibling needs them before that join is formed.
find_dependent_phvs_in_jointree() checks for that by looking for such
PHVs in the join quals and LATERAL RTEs of the siblings.
That misses a PHV that is evaluated at some but not all of the sibling
rels and contains one of the RTE_RESULT's PHVs. Such a PHV can appear
anywhere in the query, such as in the targetlist. This can happen
when pulled-up expressions are wrapped in PHVs because of outer joins
or grouping sets. Once the RTE_RESULT is removed, the outer PHV needs
a value that is only available at a join above its own evaluation
level. The outer PHV's rel thus becomes laterally dependent on the
other sibling rels. If one of those rels laterally references it in
turn, we end up with a rel that is laterally dependent on itself,
causing an assertion failure in create_lateral_join_info().
To fix, also search the whole query for PHVs that are evaluated at
some but not all of the sibling rels, and refuse to remove the
RTE_RESULT if any of them contains one of its PHVs. An outer PHV that
is evaluated at all of the sibling rels is not a problem, since the
moved PHV is available there, so we don't block removal in that common
case.
Back-patch to all supported branches.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAMbWs4_hLOsW8_SePp_3EmYTLX8Oc4zEZEvypyPRszpoMhDdGQ@mail.gmail.com
Backpatch-through: 14
The documentation stated that existing connections would continue to
function normally after the OOM killer terminated the postmaster.
In fact, those connections eventually terminate when server processes
detect the postmaster's death.
Clarify that the OOM killer can terminate either the postmaster or a
child process. If the postmaster is killed, the server must be restarted.
If a child process is killed, PostgreSQL typically terminates the
remaining connections and either recovers automatically or shuts down,
depending on restart_after_crash.
Backpatch to all supported versions.
Author: Takeshi Ideriha <iderihatakeshi@gmail.com>
Reviewed-by: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/ce269321-a7f4-48f4-a9ca-a4a4983b9fd6@gmail.com
Backpatch-through: 14
find_var_for_subquery_tle() searches a subquery rel's targetlist for a
Var referencing a given subquery output column, and asserted that any
Var it finds there belongs to that rel. That does not hold for an
appendrel child. set_append_rel_size() builds the child's targetlist
by translating the parent's, and as noted there the result can contain
arbitrary expressions. In particular, when a member of a LATERAL UNION
ALL subquery is pulled up and its output is a lateral reference, the
translated expression is a Var of the laterally referenced rel. If the
child is itself a subquery with sorted paths, convert_subquery_pathkeys()
would trip the Assert.
In non-assert builds, the function went on to compare such a Var's
varattno with the subquery column's resno, and so could match a Var of
an unrelated rel.
To fix, ignore Vars that do not belong to the subquery rel, the same
way set_rel_width() already does. Also correct the adjacent comment,
which claimed that the only non-Var entries are placeholders.
Back-patch to all supported branches.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: shihao zhong <zhong950419@gmail.com>
Discussion: https://postgr.es/m/CAMbWs48fFiPi7Q5yJCoM_nAs47zd5XgvPHd9nZyM_ewq+k_jpQ@mail.gmail.com
Backpatch-through: 14
As mentioned in the comments in TransactionIdDidAbort() and in
heapam_visibility.c, the usual procedure for checking whether a
transaction aborted is to first call TransactionIdIsInProgress(), and
then !TransactionIdDidCommit(). A couple places didn't get the memo,
and used TransactionIdDidAbort() incorrectly.
In DoesMultiXactIdConflict(), the consequence was that if a
conflicting transaction was just aborting, and had already marked the
transaction as aborted in pg_xact, but had not yet removed itself from
ProcArray, the function would return false meaning that it does not
conflict, even though it was still considered as running by
TransactionIdIsInProgress(). That could cause trouble later. One
known consequence is that if you then went ahead with updating the row
again and called MultiXactIdExpand(), MultiXactIdExpand() would see
both the old aborting transaction and the new updater as still
running, and throw an ERROR: new multixact has more than one updating
member. The multixact-aborted-updater.spec included in this commit
exercises that scenario.
Another place that got this wrong was in heap_update(), where we check
if the updating transaction that was part of a multixid committed or
aborted. Because it used TransactionIdDidAbort(), if the updating
transaction crashed, it was considered as committed and heap_update()
incorrectly returned TM_Updated indicating a conflict, even though the
row could be updated. This would only happen if the multixid as whole
was still considered running, i.e. if the tuple was still locked by
other transactions while the updating transaction crashed. That can
happen if the locker is a prepared transaction. The included
060_multixact_crashed_updater.pl test tests that scenario.
Author: Chee Wooson <wuqi@vastdata.com.cn>
Tested-by: Lionel Hamayon <lionel.hamayon@evolution-digitale.fr>
Discussion: https://www.postgresql.org/message-id/5778D6297D3CCF21+202609151836512678486@vastdata.com.cn
Backpatch-through: 14
The process by which a hash index inserter finishes an incomplete bucket
split involves a walk of the new bucket (the bucket "being populated")
to collect the TIDs that the interrupted split already moved. This
process keeps a pin on the new bucket's primary page, and then takes
conditional cleanup locks on both buckets. When either lock could not
be acquired, we gave up, but neglected to release the new bucket's pin.
To fix, release the buffer pin on failure.
Oversight in commit 6d46f4783, which improved hash index bucket split
behavior by switching it from using a heavyweight lock to using these
cleanup locks.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=b9sSBv15HsgZAuazjJu8-YRGHOpLjvp+dJgEDaBx0Ng@mail.gmail.com
Backpatch-through: 14
When constructing a historic snapshot for logical decoding,
reorderbuffer needs to keep track of movement of tuples in catalog
tables. For the most part this works correctly, except that when a
subtransaction that wrote a catalog tuple aborts, we failed to "forget"
about that change. There's no known reports of anything actually
breaking except assertion failures, but it is likely possible to
construct cases where historical snapshots produce bogus visibility
answers, potentially causing incorrect decoding output.
To fix, have the decode of a subtransaction abort go over the TUPLECID
list that has been accumulated on the ReorderBufferTXN for its topmost
parent transaction, and remove all the entries that were marked as
originating from the aborted subtransaction or its children. There are
some little details in which various devils lie, which the added code
comments explain (we hope).
Add a couple of isolation test cases that trip the assertions with the
original coding. They're not ideal test design, because they pass in
non-assert builds without the fix. But we'd rather have the fix be
verified in that limited scenario than nowhere at all, and they are
cheap enough.
This bug is sibling to the one fixed in commit 8c67d29fd5 and, like
that one, it goes all the way back to the introduction of
reorderbuffers, so backpatch to all live branches.
While reviewing this, I (Álvaro) was bothered by the snapmgr.c
tuplecid_data file-scoped variable, as it's easily confused with a
function parameter used widely in reorderbuffer.c. In branches 19 and
master, rename that to avoid confusion.
Reported-by: Arseny Sher <a.sher@postgrespro.ru>
Reported-by: Mark Dilger <mark.dilger@enterprisedb.com>
Reported-by: Alexander Kozhemyakin <a.kozhemyakin@postgrespro.ru>
Author: Bingshuai Li <lucian1412@outlook.com>
Author: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Author: Zhijie Hou <houzhijie22@gmail.com>
Backpatch-through: 14
Bug: #19555
Discussion: https://postgr.es/m/87r2c8xocq.fsf@ars-thinkpad
Discussion: https://postgr.es/m/CAHgHdKu5e3XY5e90Tuaxq_R4WrKxSV734Q%2BLwo5y39Omp2A-Gg@mail.gmail.com
Discussion: https://postgr.es/m/19555-3c700f40a13045cd@postgresql.org
getQuadrant() places a point on each axis with the fuzzy comparisons
FPlt(), FPgt() and FPeq(), which do not cover every case. FPeq()
compares the rounded difference against EPSILON, while FPlt() and FPgt()
round the sum of one operand and EPSILON. When the centroid plus
EPSILON rounds up, a point equal to that rounded sum is more than
EPSILON above the centroid, so FPeq() is false. FPgt() compares it with
the same rounded sum, so it is false too, and FPlt() is false because
the point is above the centroid. The same gap exists below the
centroid, when the centroid minus EPSILON rounds down. No quadrant
matches, and index builds, inserts and searches fail with
"getQuadrant: impossible case".
To fix, fall back to deciding each axis separately: Use fuzzy tests
first and then tiebreak with exact comparisons. That gives every finite
value a quadrant (NaN coordinates are still an error). Points that
matched a fuzzy test before are routed as before, so existing indexes
remain valid.
Bug: #19597
Reported-by: Michael Malis <malis@pgrust.com>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Pierre Forstmann <pierre.forstmann@gmail.com>
Discussion: https://postgr.es/m/19597-39c532e61d78dff6@postgresql.org
Backpatch-through: 14
When a Gather or Gather Merge node is rescanned, it starts a new set of
workers, which write their Memoize stats into the same shared memory
slots as the previous worker. ExecEndMemoize() used memcpy() for this,
so EXPLAIN ANALYZE showed only the stats for the final scan.
Here we adjust ExecEndMemoize() to accumulate the counters so that we
gather up the totals over all rescans.
Author: Tatsuya Kawata <kawatatatsuya0913@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAHza6qeA-4vB355co-2yAxDMRTpEneY0rg9CD2zMLYscYvv2UA@mail.gmail.com
Backpatch-through: 14
If a proposed join has lateral references to rels outside it,
join_is_legal() checks that those rels can still be joined to from
outside. To do that it collects the rels that must end up on the
inner side of an outer join with the proposed join, directly or
indirectly, and rejects the join if any of its lateral references
point to one of them.
That search followed only outer joins, from min_lefthand to
min_righthand. It failed to notice that a rel that laterally
references the proposed join, or any rel found by the search, cannot
supply a parameter to the proposed join either, and that the same
holds for rels on the inner side of an outer join with such a rel. As
a result, we could approve a join whose lateral references could never
be satisfied.
Such a join can never appear in a complete plan. Worse, as the
existing comment warns, its apparent legality could mislead the
clauseless-join heuristics into thinking that some other join rel need
not be formed. In particular, have_join_order_restriction() would not
force a clauseless outer join if one of its input rels appeared to
have a legal clause join elsewhere, and if that outer join was
required, we would fail with "failed to build any N-way joins".
To fix, also include in the search any input rel of the current join
search that laterally references a rel already found. The input rels
are taken from root->initial_rels rather than from the query's base
rels, because an input can be the joinrel of a sub-joinlist, as
happens for a full join or when from_collapse_limit or
join_collapse_limit prevents flattening. Such a joinrel is joined as
a unit, so it has to be included as a whole.
Back-patch to all supported branches.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: wenhui qiu <qiuwenhuifx@gmail.com>
Discussion: https://postgr.es/m/CAMbWs4_p6iWbsjNcYpd6-6mWFnog3UJ2fZKrxWAQTsyNoyeQTw@mail.gmail.com
Backpatch-through: 14
A WAL sender tracks its WAL reader and logical decoding context as two
static variables, which were left set after being freed for a
START_REPLICATION command. The WAL reader is used for in a physical
replication command, both the WAL reader and the logical decoding
context are used for a logical replication command.
Both variables are now cleared once their contents are released at the
end of each replication command, to keep the release of the resources
and the maintenance of the pointers close to each other. Some more
cleanup is done by WalSndErrorCleanup(), as well, once the WAL reader is
clear.
The assertion added by 879ad9f90e is moved before the execution of a
replication command, checking now that both the WAL reader and the
logical decoding context are never set before the execution of a new
command.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAJTYsWW+hdmmVt_A4hvGv2_5RWK8-kaOf5ncT65=5i_hyD=BHg@mail.gmail.com
Backpatch-through: 14
CheckTablespaceDirectory() did not close the directory opened with
AllocateDir(), leaving its resources held during subsequent recovery,
potentially indefinitely on a standby. Add the missing FreeDir() call.
Backpatch to all supported versions.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/66A784B7-55C1-4613-8C24-B4C2A8D75414@gmail.com
Backpatch-through: 14
Building extended statistics of type "ndistinct" calls
ndistinct_for_combination() once for every combination of columns. Each
call allocates arrays for the whole sample and never frees them, with
memory accumulating until the whole statistics object is built. The
sample has 300 multiplied by target rows. With large statistics targets
(default_statistics_target up to 10k), this can add up to gigabytes
worth of memory, useless once a combination is computed.
ndistinct_for_combination() is now called in a dedicated memory context,
reset after each combination is computed. We only need the estimated
number of distinct values, so there is no need to copy anything out of
the memory context.
Similar fixes have been done in 83772cc78e and ad8a166ca8 for other
areas of the extended stats computations.
Author: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Discussion: https://postgr.es/m/20d6689e-f490-4ca7-a92f-e92f691ba07e@tantorlabs.com
Backpatch-through: 14
extract_jsp_bool_expr applied De Morgan's laws incorrectly when
considering an AND operator underneath a NOT. If one AND input
was implementable as a GIN indexqual while the other was not,
we derived an incomplete indexqual, leading to some rows not
being returned that should be.
extract_jsp_bool_expr also failed to check for methods being
applied to literals, so that for example '$.x == "42".double()'
was treated as '$.x == "42"' not '$.x == 42'. The indexqual
thus selected the wrong rows, all of which would fail qual
recheck, leading to no rows being returned.
While we're here, improve the comments a trifle.
Reported-by: Chinmay Kanchi <cgkanchi@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAJqPDh9Cm=x8eYZ1Jf=_avd2WEqc=0t_f7_SO4igo-CKPi25_Q@mail.gmail.com
Discussion: https://postgr.es/m/CAJqPDh-tu5KY66KU8cvAEg3DQrZLzDZwjkLLa42zD+_Xm1WJAg@mail.gmail.com
Backpatch-through: 14
A hash index scan whose target bucket is still being populated by a
split must read both buckets of the split: forward scans read the bucket
being populated (skipping the tuples that the split copied there) and
then the bucket being split, and backward scans read them in the
opposite order. _hash_readnext and _hash_readprev make the transition
from the first bucket to the second, but _hash_next did not: on reaching
the end of the first bucket, it ended the scan, so every matching tuple
in the second bucket was overlooked.
To fix, teach _hash_next to make the same transition, using helper
functions shared with _hash_readnext and _hash_readprev.
_hash_next only missed the transition when the last page that it read in
the first bucket had a match (otherwise _hash_readpage's loop made the
required transition). Getting wrong answers also needed a matching
tuple to be inserted into the bucket being populated _after_ the split
began. Forward scans then lost the older tuples in the bucket being
split. An incomplete split would keep the buckets in this state until
an inserter finished the split at some indeterminate later time. Note
that wrong answers didn't require an incomplete split.
Although this fix treats the bug as the omission of a required step in
_hash_next, the real problem is the way that page stepping works in
general. The logic for stepping to the next page appears both in
high-level routines like _hash_next and in low-level routines like
_hash_readnext and _hash_readprev. If anything, _hash_next already knew
too much about page stepping (cf. nbtree's _bt_next function). All of
the logic around stepping really belongs in one routine, modeled on
nbtree's _bt_steppage function, but that approach is much too invasive
for a backpatched fix.
Claude Code found this problem. The committed test case is a simplified
version of the one that it wrote to demonstrate this bug.
Oversight in commit 7c75ef57, which introduced page-at-a-time processing
of hash index scans.
Author: Peter Geoghegan <pg@bowt.ie>
Author: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Vignesh C <vignesh21@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=4CJK9ysZzbzxBGGTkgAg9ib8cJyCx=9q_d1r8tf0ang@mail.gmail.com
Backpatch-through: 14
_bt_pagedel only builds a search stack when it needs one to locate the
downlink of the page that it's about to mark half-dead (and when it
doesn't already have one from an earlier sibling deletion that took
place during the same _bt_pagedel call). _bt_pagedel won't build a
search stack for the page its caller passes whenever it happens to be
one that an earlier interrupted VACUUM left half-dead (since there is no
downlink to remove). We nevertheless asserted that page deletion could
only build a search stack for that initial caller-passed page.
To fix, remove the assertion, which spuriously failed when a half-dead
leaf page was passed by the _bt_pagedel caller and its right sibling
page was empty/had to be considered for deletion during the same call
(necessitating building a search stack to locate the right sibling's
downlink).
Oversight in commit cd8c73a38, which refactored nbtree page deletion's
INCOMPLETE_SPLIT check.
Author: Peter Geoghegan <pg@bowt.ie>
Reported-by: Iliia Khaprov <i.khaprov@gmail.com>
Discussion: https://postgr.es/m/CAFO+O_p5So4MXwQ1VEcL9+9vGgODu11TTWcYq9RDbxnira8o2w@mail.gmail.com
Backpatch-through: 14
As of a32b8dc98 and friends, amcheck will complain about bpchar
indexes that claim to allow de-duplication. This causes our
cross-version-upgrade tests to fail when upgrading from a version
that lacks those commits, since there are some such indexes in the
core regression tests. Drop such indexes from the upgrade source
database. (oidvector indexes would be problematic too, but
there aren't any user-defined indexes on oidvector in our tests.)
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAH2-Wzk-oO3HjLaByo8Vpse6Tpha1mqAW8DFShh-kS-Wo59ugA@mail.gmail.com
Backpatch-through: 14
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
seg.sgml claims that a seg can store 7 significant digits, but this
is nonsense. The underlying float4 data type is not (quite) that
precise, and seg_out() intentionally rounds off to at most FLT_DIG
decimal places. With IEEE float arithmetic, FLT_DIG will be defined
as 6, or at least that's the case on all platforms I know of.
AFAICT this thinko goes clear back to the introduction of seg
(commit a3694b420), but it wasn't questioned until now.
Bug: #19744
Reported-by: Ke <kehan5800@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19744-8c700582bbb1d075@postgresql.org
Backpatch-through: 14
COPY FROM runs its WHERE clause through eval_const_expressions(),
which among other things replaces NOT above an operator with the
operator's negator, if it has one (e.g., NOT (a > 0) becomes
a <= 0). The node it builds for the negator has no opfuncid, and
since COPY FROM never calls fix_opfuncids() on the clause, nothing
fills it in before the executor needs it. So
COPY tab FROM stdin WHERE NOT (a > 0)
fails with "cache lookup failed for function 0".
To fix, call fix_opfuncids() on the clause, as the relcache does
for index predicates.
Oversight in commit 31f3817402.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/asPZRTci4oBvxR9A%40nathan
Backpatch-through: 14
pull_up_simple_subquery() rechecks is_simple_subquery() after pulling
up the subquery's own subqueries, but it flattened join alias Vars in
the subquery's targetlist only after that, and never in its quals. A
join alias Var within the subquery, such as a whole-row Var of a join,
can expand to an expression containing lateral references to the outer
query once the join's inputs have been pulled up, and until it is
flattened is_simple_subquery() cannot see them. We could thus pull up
a LATERAL subquery whose targetlist or quals reference rels outside
the lowest outer join above it. This leads to assertion failures in
distribute_qual_to_rels() or "wrong phnullingrels" errors, and in some
cases to wrong query results, where rows that should have been
preserved by an outer join above the subquery are discarded.
To fix, flatten join alias Vars in the targetlist, and in the jointree
quals if the subquery is LATERAL, before the recheck. If the recheck
fails, the flattened copy is discarded along with the rest of the
modified subquery, so no harm is done.
Back-patch to all supported branches, as this can lead to wrong query
results.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tatsuya Kawata <kawatatatsuya0913@gmail.com>
Discussion: https://postgr.es/m/CAMbWs48GFZ=3Bjc1ug9JRsB0Qupg+CGcb9js5vmTM6kY2OGWOg@mail.gmail.com
Backpatch-through: 14
pg_blocking_pids() assumes that members of the same lock group
never block each other, but that hasn't been true for relation
extension locks since v13. This commit teaches the function to
report lock group members as blockers for relation extension locks.
Since parallel workers are reported with their leader's PID, the
result may now include the blocked session's own PID.
Oversight in commit 85f6b49c2c.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Shihao Zhong <zhong950419@gmail.com>
Discussion: https://postgr.es/m/apTpV8h%2BCYHf%2B1PJ%40bdtpg
Backpatch-through: 14
It's a bad idea to sleep while holding locks, because you might block
another process that wants to acquire the same lock. Furthermore,
it's not safe to call ProcessConfigFile() in a critical section or
while holding locks. So avoid calling vacuum_delay_point() while
holding locks, and also make vacuum_delay_point() return quickly
without sleeping if it's called while interrupts cannot be processed.
To find the vacuum_delay_point() calls that were made while holding
locks, I added "Assert(INTERRUPTS_CAN_BE_PROCESSED())" in
vacuum_delay_point() and ran the regression tests. I didn't include
that Assert in this commit, because there might be more places that do
that that are not covered by the regression tests, including
extensions.
In ginInsertCleanup(), there was already another vacuum_delay_point()
call later in the loop, while not holding any locks, so just remove
the other call that was made while holding the lock.
In hashbucketcleanup(), move the vacuum_delay_point() call up the
stack to its caller. The call in hashbucketcleanup() was not able to
handle interrupts because it held a lock, and because there were no
vacuum_delay_point() or CHECK_FOR_INTERRUPTS() calls in the caller's
loop, the whole hash index vacuuming phase was uninterruptible by
pending shutdown or query cancel. Now it can be interrupted between
buckets. Unfortunately, hashbucketcleanup() has to process all the
bucket's pages in one go without pausing; fixing that would require
changing how hashbucketcleanup()'s lock chaining works.
In acquire_sample_rows(), move the vacuum_delay_point() call in the
loop to between pages, to a time where we're not holding the buffer
lock.
In master, also remove a misleading CHECK_FOR_INTERRUPTS() call from
pgstat_write_statsfile(). It was a no-op because we're always holding
a lock on the current dshash partition at that point. A call that
does nothing is harmless, but it makes you think that the loop is
interruptible when in reality it's not.
Author: Kevin Rocker <me@kevinrocker.com>
Author: Andrey Borodin <amborodin@acm.org>
Author: Mostafa <mostafa.nabil.nafie@gmail.com>
Reported-by: Greg Burd <greg@burd.me>
Reported-by: Sergei Kornilov <sk@zsrv.org>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Neil Chen <carpenter.nail.cz@gmail.com>
Tested-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/19628-c2b17d358181a1ea@postgresql.org
Discussion: https://postgr.es/m/492c6247-43d3-477b-8981-fb0c56767b38%40app.fastmail.com
Backpatch-through: 14
preprocess_expression skips flatten_join_alias_vars for TABLESAMPLE
clauses, on the assumption that they cannot contain Vars of the
current query level. That's not true once a LATERAL subquery
containing a sampled relation has been pulled up: the relation's
TABLESAMPLE clause then becomes a lateral reference of this level, and
if it referred to a join alias Var, that Var is left unexpanded.
extract_lateral_references then pulls the join alias Var out as a
lateral reference and fails with "no relation entry for relid N".
To fix, treat such clauses like lateral function and VALUES RTEs:
introduce EXPRKIND_TABLESAMPLE_LATERAL, used for the TABLESAMPLE
clause of a lateral relation RTE, and keep skipping the alias
expansion only for the non-lateral kind.
Back-patch to all supported branches, as this dates back to the
introduction of TABLESAMPLE.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAMbWs4834TwOtyyycyvja3MuOBkfpXy3dFAL0zt_vcM1KtqV-Q@mail.gmail.com
Backpatch-through: 14
If a subquery has been flattened into its parent, the joinaliasvars
entries of a join above it can be arbitrary expressions rather than
plain Vars, so expanding a reference to such a join alias may insert a
SubLink into a lower-level subquery. flatten_join_alias_vars_mutator
detects that and sets the subquery's hasSubLinks flag, but only in the
single-column code path; the whole-row path just asserted in a comment
that its recursive call would handle this, which is true only when the
alias entry is itself a Var referencing another join.
Hence a whole-row reference to such a join appearing in a sub-select
left that sub-select's hasSubLinks false, so preprocess_expression
skipped SS_process_sublinks for it, and the unprocessed SubLink
reached code that is not prepared for one: this produced "cannot
handle unplanned sub-select" from cost_qual_eval, an assertion failure
in preprocess_aggrefs, or "unrecognized node type" at execution,
depending on where in the sub-select the SubLink ended up.
To fix, make the same check in the whole-row expansion path.
Back-patch to all supported branches.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49PgeNFhQTZ2gSzwatF0_LXyegMPGt++jSfxUN7NZuQ5Q@mail.gmail.com
Backpatch-through: 14
In an in-tree build, "make coverage-html" invoked lcov with two
"-d ." options, because $(srcdir) is just ".". With lcov 2.0
(and perhaps earlier versions?) that results in scanning every
foo.gcda file twice and producing counts twice what they should
be. With later lcov versions (at least 2.3), it results in
"duplicate file" errors.
In VPATH builds we do need two -d options to cover both regular
and generated .c files. But in in-tree builds it suffices to
specify "-d .", so suppress the second switch.
Author: Narek Galstyan <narek.galstyan@enterprisedb.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAHsn6_xCDQWe8_vVFhtFk27_xTdyVV+Dr0yWzaooBZ6+-VH-5w@mail.gmail.com
Backpatch-through: 14
dumpIndex() emits ALTER INDEX .. SET STATISTICS for per-column
statistics targets, but dumpConstraint() never did. Targets set on the
expression columns of an exclusion constraint's index were silently
lost on dump/restore and pg_upgrade.
The same code can be used in the index and constraint dump code to
generate these ALTER INDEX commands. The logic has been refactored so
that both share a new subroutine in charge of generating all these
custom SET STATISTICS, if any.
Oversight in e4fca461ab.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Andrew Krylosov <krylosov.andrew@gmail.com>
Discussion: https://postgr.es/m/CAN4CZFMd9riOuV5LkM+uV=uF-HYp49C8Fh22xWyMxptGLBCZ3A@mail.gmail.com
Backpatch-through: 14
FetchStatementTargetList() dereferenced the portal named by a
FetchStmt without checking that it still exists. With the extended
query protocol, a client can sent a Parse "FETCH n FROM c" while cursor
"c" is open (which caches a result's TupleDesc), issue a CLOSE on cursor
"c", and then Describe the prepared statement or a portal bound to it.
Describe would re-resolve the target list through the named cursor, and
crash. In a non-assert builds, this causes a NULL pointer dereference.
An assertion was triggered in assert builds slightly before the pointer
dereference.
This commit changes the describe of a FetchStmt to match with the
ExecuteStmt case: if the named cursor's portal is invalid, fail rather
than assume that the portal should always exist.
A test case is added to libpq_pipeline, backpatchable all the way down.
Author: Dirkjan Bussink <d.bussink@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/7DE248EB-7509-4F8A-9F37-4694A40BBAB7@gmail.com
Backpatch-through: 14
There have been several recent complaints that << and >> don't behave
the same way on all supported platforms. There is a risk that <<
overflows the signed integer type, which is undefined behavior, and there
are platform-dependent behaviors from shifting by numbers of bits >= the
bitwidth of the type, and also from shifting by negative values.
Document this to warn unsuspecting users of these facts.
Author: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAApHDvr0O6ZF8NO=Rq9Q+Sz=FHT=NZ1kkNTXZDcNph-OWuk7TQ@mail.gmail.com
Backpatch-through: 14
Canada's Northwest Territories will move to permanent UTC-06 on
2026-11-01. For now, assume their TZ abbreviation will be CST from
that time forward. This affects only America/Inuvik as the rest of
the territory is covered by America/Edmonton, for which the
equivalent change was released in 2026c.
Manitoba (America/Winnipeg) will move to permanent UTC-05 on
2026-10-31. For now, assume their TZ abbreviation will be EST
from that time forward.
(Upstream notes that more changes may be needed soon if smaller
jurisdictions choose to break away from what these have done,
but for now the plan is wait-and-see.)
Historical corrections for Colombia, Iran, and Ireland.
The backward-compatibility zone names EST5EDT, CST6CDT, MST7MDT, and
PST8PDT now conform better to POSIX. For example, EST5EDT now always
uses the abbreviation "EST" for standard time (now always 5 hours
behind UTC) and "EDT" for daylight saving time, whereas it formerly
had different UT offsets before standard time was introduced and
sometimes used abbreviations like "LMT", "EWT" and "EPT", all contrary
to POSIX. Also, though not specified by POSIX these names now use US
federal rules rather than rules of specific places like New York,
reverting to 2024a behavior. This change affects only timestamps
before 1966-10-30 at 01:00 standard time.
Backpatch-through: 14
ReplicationSlotRelease() copied MyProc->statusFlags unconditionally
when resetting PROC_IN_LOGICAL_DECODING, using its pgxactoff to decide
which area to update.
Slot invalidations can be done by the checkpointer or the startup
process, and ReplicationSlotRelease() ignored the fact that pgxactoff is
only meaningful for processes in the proc array, that auxiliary
processes never join. pgxactoff would remain at 0 for these, meaning
that an incorrect set of status flags would be updated.
The consequences of this issue could be various, depending on the
statsFlags lost in the proc slot incorrectly overwritten. Losing
PROC_AFFECTS_ALL_HORIZONS has been mentioned as the worse case, causing
VACUUM to remove rows a standby may needs even if standby feedback is
enabled. Assertions failures are another possibility.
Let's skip the update of statusFlags unless PROC_IN_LOGICAL_DECODING is
set. Nothing but StartupDecodingContext() sets it, and no auxiliary
processes call it.
Author: Vlad Lesin <vladlesin@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/889a06dd-45fc-423b-9dd2-87d5b5dc60c6@gmail.com
Backpatch-through: 14
