Re: Freeing sortgroupatts in use_physical_tlist

On Fri, Jul 15, 2022 at 8:33 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Zhihong Yu <zyu(at)yugabyte(dot)com> writes:
> > I was looking at the code in use_physical_tlist().
> > In the code block checking CP_LABEL_TLIST, I noticed that
> > the Bitmapset sortgroupatts is not freed before returning from the
> method.
> > Looking at create_foreignscan_plan() (in the same file):
> > bms_free(attrs_used);
> > It seems the intermediate Bitmapset is freed before returning.
>
> TBH, I'd say that it's probably the former code not the latter
> that is good practice. Retail pfree's in code that's not in a
> loop very possibly expend more cycles than they are worth, because
> the space will get cleaned up anyway when the active memory
> context is reset, and pfree is not as cheap as one might wish.
> It might be possible to make a case for one method over the other
> with some study of the particular situation, but you can't say
> a priori which way is better.
>
> On the whole, I would not bother changing either of these bits
> of code without some clear evidence that it matters. It likely
> doesn't. It's even more likely that it doesn't matter enough
> to be worth investigating.
>
> regards, tom lane
>
Hi, Tom:
Thanks for responding over the weekend.

I will try to remember what you said.

Cheers