Re: freeing bms explicitly - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: freeing bms explicitly
Date
Msg-id CALNJ-vR3dVBq3ARG0Hj1Q5vQB37OpabjeiW65yCxn7x5TwpeyQ@mail.gmail.com
Whole thread Raw
In response to Re: freeing bms explicitly  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: freeing bms explicitly  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers


On Mon, Mar 21, 2022 at 3:05 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Zhihong Yu <zyu@yugabyte.com> writes:
>> I was looking at calls to bms_free() in PG code.
>> e.g. src/backend/commands/publicationcmds.c line 362
>>      bms_free(bms);
>> The above is just an example, there're other calls to bms_free().
>> Since the bms is allocated from some execution context, I wonder why this
>> call is needed.
>>
>> When the underlying execution context wraps up, isn't the bms freed ?

Yeah, that's kind of pointless --- and the pfree(rfnode) after it is even
more pointless, since it'll free only the top node of that expression
tree.  Not to mention the string returned by TextDatumGetCString, and
whatever might be leaked during the underlying catalog accesses.

If we were actually worried about transient space consumption of this
function, it'd be necessary to do a lot more than this.  It doesn't
look to me like it's worth worrying about though -- it doesn't seem
like it could be hit more than once per query in normal cases.

                        regards, tom lane

Thanks Tom for replying.

What do you think of the following patch ?

Cheers 
Attachment

pgsql-hackers by date:

Previous
From: Nathan Bossart
Date:
Subject: Re: Estimating HugePages Requirements?
Next
From: Jimmy Yih
Date:
Subject: Re: Concurrent deadlock scenario with DROP INDEX on partitioned index