Re: BUG #18830: ExecInitMerge Segfault on MERGE

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>, David Rowley <dgrowleyml(at)gmail(dot)com>, pgsql-bugs(at)lists(dot)postgresql(dot)org, tharakan(at)gmail(dot)com
Subject: Re: BUG #18830: ExecInitMerge Segfault on MERGE
Date: 2025-03-18 07:52:38
Message-ID: CA+HiwqFWS5+i5eO4yx-weEKZ-xtewgQXLjNqCpoKZKnav7+D1A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Tue, Mar 18, 2025 at 8:36 AM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> 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.

Ok, thanks -- appreciate your thoughts. I agree it would’ve been nice
to avoid adding a new field to PlannedStmt, but I couldn’t see a
simple way around it either, given the lock timing restrictions.

> > 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 :-)

Yes, of course. :-)

> > 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.

Ok, done in the attached.

I'll push this tomorrow.

--
Thanks, Amit Langote

Attachment Content-Type Size
v6-0001-Ensure-first-ModifyTable-rel-initialized-if-all-a.patch application/octet-stream 20.5 KB

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2025-03-18 08:02:46 BUG #18853: integer may overflow in array_user_functions
Previous Message Dean Rasheed 2025-03-17 23:35:59 Re: BUG #18830: ExecInitMerge Segfault on MERGE