public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tender Wang <[email protected]>
To: Dean Rasheed <[email protected]>
Cc: Amit Langote <[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: Thu, 25 Dec 2025 12:48:08 +0800
Message-ID: <CAHewXN=D2SUh1CvE0gnk1Ev_k4RAEO9aoEgQZG_7eAuSWxZA0Q@mail.gmail.com> (raw)
In-Reply-To: <CAEZATCVr+=F=iTbi3XyABSEr5Cqw=ptm2PyGc75SaQ1JYrPnsg@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>
<CAHewXNn8ApcYfZKjZ1R8Z7yHRoi+C+mu5r9LbZUbb6691d6Rrw@mail.gmail.com>
<CAEZATCVr+=F=iTbi3XyABSEr5Cqw=ptm2PyGc75SaQ1JYrPnsg@mail.gmail.com>
Dean Rasheed <[email protected]> 于2025年12月25日周四 07:12写道：
> On Wed, 24 Dec 2025 at 12:07, Tender Wang <[email protected]> wrote:
> >
> > 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:
> > .....
> > 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?
>
> Yes. However, it's not sufficient to only add RTE_VALUES RTEs to what
> gets included in PlannerGlobal.allRelids. Rowmarks can be attached to
> other kinds of RTEs. An obvious example is an RTE_SUBQUERY RTE that is
> an actual subquery that did not come from a view. So, for this
> approach to work robustly, it really should include *all* RTEs in
> PlannerGlobal.allRelids.
>
Yes, it is. I can reproduce the same with a subquery as below:
merge into t1 using (select a from (values(1,1),(2,2) as t4(a,b)) as t3(a)
on (t1.a = t3.a)
when matched then
update set b = t1.b + 1
when not matched then
insert (a,b) values (1,1);
or
merge into t1 using (select generate_series(1,2) as a) as t3 on (t1.a =
t3.a)
when matched then
update set b = t1.b + 1
when not matched then
insert (a,b) values (1,1);
All reported the same error. My approach obviously can not cover all cases.
>
> I took a slightly different approach, which was to change the test in
> InitPlan() (and also in ExecInitLockRows() and ExecInitModifyTable())
> to only ignore rowmarks for pruned relations that are plain
> RTE_RELATION relations, since those are the only relations that are
> ever actually pruned. So rowmarks attached to any other kind of RTE
> are not ignored. I also added an isolation test case.
>
I failed to apply your patch, some errors, like "error: git apply: bad
git-diff - expected /dev/null on line 2"
or "Patch format detection failed."
I looked through the patch. It worked for me.
> I'm somewhat conflicted as to which approach is better. I think maybe
> there is less chance of other unintended side-effects if the set of
> RTEs included in PlannerGlobal.allRelids, unprunableRelids, and
> es_unpruned_relids is not changed. However, as it stands,
> PlannerGlobal.allRelids is misnamed (it should probably have been
> called "relationRelids", in line with the "relationOids" field).
> Making it actually include all RTEs would solve that.
>
.....
/*
* RT indexes of all relation RTEs in finalrtable (RTE_RELATION and
* RTE_SUBQUERY RTEs of views)
*/
Bitmapset *allRelids;
......
Per the comment, I guess Amit didn't plan to include all RTEs in his design.
--
Thanks,
Tender Wang
view thread (12+ messages) latest in thread
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: <CAHewXN=D2SUh1CvE0gnk1Ev_k4RAEO9aoEgQZG_7eAuSWxZA0Q@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