On Wed, Jan 7, 2026 at 9:52 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> On Wed, 7 Jan 2026 at 09:37, Amit Langote <amitlangote09@gmail.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.
--
Thanks, Amit Langote
