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.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com