public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tender Wang <[email protected]>
To: Amit Langote <[email protected]>
Cc: Dean Rasheed <[email protected]>
Cc: Bh W <[email protected]>
Cc: Laurenz Albe <[email protected]>
Cc: [email protected]
Subject: Re: BUG #19355: Attempt to insert data unexpectedly during concurrent update
Date: Wed, 24 Dec 2025 20:07:00 +0800
Message-ID: <CAHewXNn8ApcYfZKjZ1R8Z7yHRoi+C+mu5r9LbZUbb6691d6Rrw@mail.gmail.com> (raw)
In-Reply-To: <CA+HiwqE8gNK7Utqo2EAKh24KExWfypt06m1EFNXBd5TFDO2f6w@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<CANjBVm3AMm=9-mB7=uqJjaWN-+RrA8+w-gXhZn6L7VHu7K_Urw@mail.gmail.com>
<CAEZATCVcV4cfqGnmPGVxwjsPh7rxsTD510=KOGFyi2_FMgqWaQ@mail.gmail.com>
<CA+HiwqE8gNK7Utqo2EAKh24KExWfypt06m1EFNXBd5TFDO2f6w@mail.gmail.com>
Amit Langote <[email protected]> 于2025年12月24日周三 16:08写道：
> Hi,
>
> On Tue, Dec 23, 2025 at 4:07 Dean Rasheed <[email protected]>
> wrote:
>
>> On Mon, 22 Dec 2025 at 14:51, Bh W <[email protected]> wrote:
>> >
>> > The issue is that the MERGE INTO match condition is not updated.
>> > In the MATCHED path of MERGE INTO, when the target row satisfies the
>> match condition and the condition itself has not changed, the system should
>> still be able to handle concurrent updates to the same target row by
>> relying on EvalPlanQual (EPQ) to refetch the latest version of the tuple,
>> and then proceed with the intended update.
>> > However, in the current implementation, even though the concurrent
>> update does not modify any columns relevant to the ON condition, the EPQ
>> recheck unexpectedly results in a match condition failure, causing the
>> update path that should remain MATCHED to be treated as NOT MATCHED.
>>
>> I spent a little time looking at this, and managed to reduce the
>> reproducer test case down to this:
>>
>> -- Setup
>> drop table if exists t1,t2;
>> create table t1(a int primary key, b int);
>> create table t2(a int, b int);
>>
>> insert into t1 values(1,0),(2,0);
>> insert into t2 values(1,1),(2,2);
>>
>> -- Session 1
>> begin;
>> update t1 set b = b+1;
>>
>> -- Session 2
>> merge into t1 using (values(1,1),(2,2)) as t3(a,b) on (t1.a = t3.a)
>> when matched then
>> update set b = t1.b + 1
>> when not matched then
>> insert (a,b) values (1,1);
>>
>> -- Session 1
>> commit;
>>
>> This works fine in PG17, but fails with a PK violation in PG18.
>> Git-bisecting points to this commit:
>>
>> cbc127917e04a978a788b8bc9d35a70244396d5b is the first bad commit
>> commit cbc127917e04a978a788b8bc9d35a70244396d5b
>> Author: Amit Langote <[email protected]>
>> Date: Fri Feb 7 17:15:09 2025 +0900
>>
>> Track unpruned relids to avoid processing pruned relations
>>
>> Doing a little more debugging, it looks like the problem might be this
>> change in InitPlan():
>>
>> - /* ignore "parent" rowmarks; they are irrelevant at runtime */
>> - if (rc->isParent)
>> + /*
>> + * Ignore "parent" rowmarks, because they are irrelevant at
>> + * runtime. Also ignore the rowmarks belonging to child
>> tables
>> + * that have been pruned in ExecDoInitialPruning().
>> + */
>> + if (rc->isParent ||
>> + !bms_is_member(rc->rti, estate->es_unpruned_relids))
>> continue;
>>
>> which seems to cause it to incorrectly skip a rowmark, which I suspect
>> is what is causing EvalPlanQual() to return the wrong result.
>
>
> Thanks for the detailed analysis and adding me to the thread, Dean.
>
> I would think that a case that involves no partitioning at all would be
> untouchable by this code, but it looks like the logic I added is
> incorrectly affecting cases where pruning isn’t even relevant. I’ll need to
> look more carefully at why such a rowmark would exist in the rowmarks list
> if its relation isn’t in es_unpruned_relids. Maybe the set population is
> incorrect at some point, or perhaps it matters that the set is a copy in
> the EPQ estate.
>
I did some debugging, and I found that:
In add_rte_to_flat_rtable(), the RTE of value was not added into
glob->AllRelids, because below codes:
.....
if (newrte->rtekind == RTE_RELATION ||
(newrte->rtekind == RTE_SUBQUERY && OidIsValid(newrte->relid)))
{
glob->relationOids = lappend_oid(glob->relationOids, newrte->relid);
glob->allRelids = bms_add_member(glob->allRelids,
list_length(glob->finalrtable));
}
....
The VALUE rte was not satisfied above if, so it was not added into the
glob->allRelids.
Then in standard_planner(), we have:
....
result->unprunableRelids = bms_difference(glob->allRelids,
glob->prunableRelids);
....
So the result->unprunableRelids contains only 1.
In InitPlan(), we have:
.....
/*
* Ignore "parent" rowmarks, because they are irrelevant at
* runtime. Also ignore the rowmarks belonging to child tables
* that have been pruned in ExecDoInitialPruning().
*/
if (rc->isParent ||
!bms_is_member(rc->rti, estate->es_unpruned_relids))
continue;
.....
the estate->es_unpruned_relids equals with result->unprunableRelids
contains. So the rowMark was skipped incorrectly.
I did a quick fix as the attached patch.
Any thoughts?
--
Thanks,
Tender Wang
Attachments:
[application/octet-stream] 0001-fix-concurrent-update.patch (978B, 3-0001-fix-concurrent-update.patch)
download | inline diff:
From ed9194a5735db35c6a21c3d7e9f3a6b283cbfbf2 Mon Sep 17 00:00:00 2001
From: Tender Wang <[email protected]>
Date: Wed, 24 Dec 2025 19:57:56 +0800
Subject: [PATCH] fix concurrent update.
---
src/backend/optimizer/plan/setrefs.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index cd7ea1e6b58..83662134d2e 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -584,6 +584,10 @@ add_rte_to_flat_rtable(PlannerGlobal *glob, List *rteperminfos,
(newrte->rtekind == RTE_SUBQUERY && OidIsValid(newrte->relid)))
{
glob->relationOids = lappend_oid(glob->relationOids, newrte->relid);
+ }
+ if (newrte->rtekind == RTE_RELATION || newrte->rtekind == RTE_VALUES ||
+ (newrte->rtekind == RTE_SUBQUERY && OidIsValid(newrte->relid)))
+ {
glob->allRelids = bms_add_member(glob->allRelids,
list_length(glob->finalrtable));
}
--
2.34.1
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19355: Attempt to insert data unexpectedly during concurrent update
In-Reply-To: <CAHewXNn8ApcYfZKjZ1R8Z7yHRoi+C+mu5r9LbZUbb6691d6Rrw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox