Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning

On Thu, Sep 21, 2023 at 6:37 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> On Wed, Sep 20, 2023 at 10:24 PM Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > On Wed, Sep 20, 2023 at 5:24 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > > Just one comment on 0003:
> > >
> > > + /*
> > > + * Dummy SpecialJoinInfos do not have any translated fields and hence have
> > > + * nothing to free.
> > > + */
> > > + if (child_sjinfo->jointype == JOIN_INNER)
> > > + return;
> > >
> > > Should this instead be Assert(child_sjinfo->jointype != JOIN_INNER)?
> >
> > try_partitionwise_join() calls free_child_sjinfo_members()
> > unconditionally i.e. it will be called even SpecialJoinInfos of INNER
> > join. Above condition filters those out early. In fact if we convert
> > into an Assert, in a production build the rest of the code will free
> > Relids which are referenced somewhere else causing dangling pointers.
>
> You're right. I hadn't realized that the parent SpecialJoinInfo
> passed to try_partitionwise_join() might itself be a dummy. I guess I
> should've read the whole thing before commenting.

Maybe there's something to improve in terms of readability, I don't know.

--
Best Wishes,
Ashutosh Bapat