On Wed, Sep 20, 2023 at 5:24 PM Amit Langote <amitlangot...@gmail.com> wrote: > > I read this thread and have been reading the latest patch. At first > glance, it seems quite straightforward to me. I agree with Richard > that pfree()'ing 4 bitmapsets may not be a lot of added overhead. I > will study the patch a bit more.
Thanks. > > 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. -- Best Wishes, Ashutosh Bapat