Re: BUG #18806: When enable_rartitionwise_join is set to ON, the database shuts down abnormally

On Tue, Feb 18, 2025 at 8:23 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> On Thu, Feb 13, 2025 at 1:55 PM Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > On Thu, Feb 13, 2025 at 8:44 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> > > I'm thinking that a better approach might be to check if each member
> > > of the child_sjinfo is a translated copy and only free it if that's
> > > the case, something like attached.
> >
> > This approach doesn't allocate memory when translation is not required
> > but it requires parent SJinfo to be present when
> > free_child_join_sjinfo() is called. It's true right now but future
> > changes to the code may not ensure that. So there is a potential
> > maintenance overhead here.
>
> Given the function's name, I don't see a problem with adding a new
> parameter to pass parent_sjinfo. In fact, I'd be especially concerned
> if any caller doesn’t have parent_sjinfo handy -- that could signal an
> unexpected usage or future maintenance risk. Making it explicit helps
> keep things correct and avoids silent errors.

Normal SJInfos are created somewhere and put in a list while they are
passed around. In future, we may do something like that for child
SJInfos as well. That was my thinking behind this suggestion. But we
will see what happens when it comes to that.

>
> > We have to be always cautious of freeing or modifying the Relids
> > returned by adjust_child_relids() [1]. For example, we could free
> > intermediate Relids in adjust_child_relids_multilevel() but we need to
> > be cautious of them being same as the parent Relids. Amit's approach
> > of bms_copy() in adjust_child_relids() frees us from that caution. May
> > be we could pass a flag to adjust_child_relids() asking it to create a
> > new bitmapset always. So even if it's a larger bandaid, it may be
> > useful at multiple places.
>
> We could do this separately, but for a bug fix, I'd prefer to keep the
> changes focused on the issue at hand and avoid modifications unrelated
> to the bug.

That's a fair point.

--
Best Wishes,
Ashutosh Bapat