public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Dean Rasheed <[email protected]>
Cc: Tender Wang <[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, 7 Jan 2026 18:37:12 +0900
Message-ID: <CA+HiwqGCfYgT1DnnK836qRX63okKNeNR=CQmshC6=aATCw6VVA@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>
Hi Dean,
Apologies for the delay in getting back to this thread (vacation got
unexpectedly long for family reasons).
On Thu, Dec 25, 2025 at 8:12 AM Dean Rasheed <[email protected]> wrote:
>
> 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.
>
> 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'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.
Yes, I think the approach in your patch seems better for the reason
you mentioned, at least for back-patching sanity.
I intended all of these relid sets to account for prunable RELATION RTEs only.
Thanks Tender and Bernice for the additional analysis. I prefer Dean's
fix-the-executor approach for back-patching. Bernice, are there other
related issues you're aware of beyond this rowmark bug? Want to make
sure Dean's patch covers them too.
> 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.
Yeah, I agree it should have been named relationRelids. Perhaps worth
renaming in a separate cleanup, though not urgent.
Thanks for the patch! Do you intend to commit and back-patch this
yourself, or would you like me to handle it?
--
Thanks, Amit Langote
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: <CA+HiwqGCfYgT1DnnK836qRX63okKNeNR=CQmshC6=aATCw6VVA@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