public inbox for [email protected]  
help / color / mirror / Atom feed
From: 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: Thu, 8 Jan 2026 22:16:12 +0900
Message-ID: <CA+HiwqFC8OssEd-sAixUiZetiMGz8BGbbn6GJC8krUQx2J=huQ@mail.gmail.com> (raw)
In-Reply-To: <CAEZATCVwnjNYNSLLTQCXgwhgGEascKHu+hEtDVn7PF4ZCZvm8g@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>
	<CA+HiwqGCfYgT1DnnK836qRX63okKNeNR=CQmshC6=aATCw6VVA@mail.gmail.com>
	<CAEZATCVwnjNYNSLLTQCXgwhgGEascKHu+hEtDVn7PF4ZCZvm8g@mail.gmail.com>

On Wed, Jan 7, 2026 at 9:52 PM Dean Rasheed <[email protected]> wrote:
> On Wed, 7 Jan 2026 at 09:37, Amit Langote <[email protected]> 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.

-- 
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+HiwqFC8OssEd-sAixUiZetiMGz8BGbbn6GJC8krUQx2J=huQ@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