Re: BUG #18830: ExecInitMerge Segfault on MERGE

On Mon, 17 Mar 2025 at 12:21, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> > I wondered about replacing the new field
> > "firstResultRels" with something like "mtResultRels", which would be a
> > list of lists, to allow that, but I didn't try it.
>
> Or a List of Bitmapset, so that bms_intersect() can be used to check
> whether all result relations of a given ModifyTable have been pruned.
> But whether it’s that or a List of Lists, both would add a potentially
> large amount of memory to PlannedStmt, to avoid taking an extra lock.
> Probably not a great tradeoff, but we can revisit if it becomes
> worthwhile?

Yeah. It would have been nice to fix this without adding a new field
to PlannedStmt at all, but I couldn't see an easy way to do that.
Actually I don't think a list of lists or Btimapsets would be that
large. There are unlikely to be more than one or two ModifyTable nodes
in most queries, and the number of bottom-level elements would be less
than in the flat resultRelations list. But still, it doesn't really
seem worth the effort.

> I've attached v5 with my commit message but were you planning to
> commit it yourself?

I'll leave it to you, since it was your commit originally :-)

> If not, does the commit message look good to you?

It could perhaps be more specific about saying that
ExecInitModifyTable() includes the first result relation if all others
have been pruned, so it isn't always included.

Regards,
Dean