Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning

Hi Richard,

On Fri, Jul 28, 2023 at 2:03 PM Richard Guo <guofenglinux@gmail.com> wrote:
>
>
> It doesn't seem too expensive to translate SpecialJoinInfos, so I think
> it's OK to construct and free the SpecialJoinInfo for a child join on
> the fly.  So the approach in 0002 looks reasonable to me.  But there is
> something that needs to be fixed in 0002.

Thanks for the review. I am fine if going ahead with 0002 is enough.

>
> +   bms_free(child_sjinfo->commute_above_l);
> +   bms_free(child_sjinfo->commute_above_r);
> +   bms_free(child_sjinfo->commute_below_l);
> +   bms_free(child_sjinfo->commute_below_r);
>
> These four members in SpecialJoinInfo only contain outer join relids.
> They do not need to be translated.  So they would reference the same
> memory areas in child_sjinfo as in parent_sjinfo.  We should not free
> them, otherwise they would become dangling pointers in parent sjinfo.
>

Good catch. Saved my time. I would have caught this rather hard way
when running regression. Thanks a lot.

I think we should 1. add an assert to make sure that commute_above_*
do not require any transations i.e. they do not contain any parent
relids of the child rels. 2. Do not free those. 3. Add a comment about
keeping the build and free functions in sync. I will work on those
next.

--
Best Wishes,
Ashutosh Bapat