On Wed, Sep 20, 2023 at 10:24 PM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > On Wed, Sep 20, 2023 at 5:24 PM Amit Langote <amitlangot...@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
