On Wed, Sep 20, 2023 at 5:24 PM Amit Langote <amitlangote09@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