| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
| Cc: | Tender Wang <tndrwang(at)gmail(dot)com>, Bh W <wangbihua(dot)cn(at)gmail(dot)com>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: BUG #19355: Attempt to insert data unexpectedly during concurrent update |
| Date: | 2026-01-15 11:39:01 |
| Message-ID: | CA+HiwqGSogk5__9rFp9F6TGDpyM11CTVQHy5GRptGZC3A4DwwA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
On Thu, Jan 8, 2026 at 10:16 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Wed, Jan 7, 2026 at 9:52 PM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> > On Wed, 7 Jan 2026 at 09:37, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > >
> > > 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.
> >
> > Yes, I think that makes sense.
> >
> > > 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.
> >
> > It looks to me as though either approach would work, so I'm happy for
> > you to decide which approach fits best with your design.
>
> Ok, I thought about this some more.
>
> I admit I'd be lying if I said I didn't have doubts about my original
> design. It might be better for PlannedStmt.unprunableRelids and
> es_unpruned_relids to cover *all* RTEs except those that are prunable
> (decided by the planner) or pruned (decided during executor startup).
> That way, code checking these sets wouldn't need to also verify the
> RTE kind, as your patch currently does.
>
> If we were to change the design, we could remove
> PlannerGlobal.allRelids entirely on master, since it would no longer
> need to be selectively populated -- unprunableRelids could just be
> computed directly from the full rtable. But that would mean we'd be
> leaving v18 with an unused field in PlannerGlobal. That's not great,
> but having different designs in the two branches isn't ideal either.
>
> So I'll go with your minimal fix for the back-patch. We can revisit
> the design cleanup on master later if desired.
>
> > > Thanks for the patch! Do you intend to commit and back-patch this
> > > yourself, or would you like me to handle it?
> >
> > It's your code, and you're more familiar with it than me, so I'm happy
> > to leave it to you :-)
>
> Surely.
Here's Dean patch with a commit message added. I plan to commit it
barring objections.
--
Thanks, Amit Langote
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Fix-rowmark-handling-for-non-relation-RTEs-during.patch | application/octet-stream | 7.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | PG Bug reporting form | 2026-01-15 11:57:14 | BUG #19379: Role pg_read_all_data don't allowed read large objects |
| Previous Message | ma lz | 2026-01-15 08:06:00 | 回复: uninitialized var in encnames.c |