Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning

On Thu, Sep 21, 2023 at 6:37 AM Amit Langote <amitlangote09@gmail.com> wrote:
>
> On Wed, Sep 20, 2023 at 10:24 PM Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> > On Wed, Sep 20, 2023 at 5:24 PM Amit Langote <amitlangote09@gmail.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