Thread: Memory leak on hashed agg rescan
I noticed a minor leak in the per-query context when ExecReScanAgg() is called for a hashed aggregate. During rescan, build_hash_table() is called to create a new empty hash table in the aggcontext. However, build_hash_table() also constructs the "hash_needed" column list in the per-query context, so repeated calls of build_hash_table() result in leaking this memory for the duration of the query. Attached is a patch that fixes this by only constructing "hash_needed" if it doesn't already exist. I also bms_free'd the temporary BMS that is created, although that's pretty harmless. Neil
Attachment
"Neil Conway" <neil.conway@gmail.com> writes: > I noticed a minor leak in the per-query context when ExecReScanAgg() > is called for a hashed aggregate. During rescan, build_hash_table() is > called to create a new empty hash table in the aggcontext. However, > build_hash_table() also constructs the "hash_needed" column list in > the per-query context, so repeated calls of build_hash_table() result > in leaking this memory for the duration of the query. > Attached is a patch that fixes this by only constructing "hash_needed" > if it doesn't already exist. I also bms_free'd the temporary BMS that > is created, although that's pretty harmless. It would probably be cleaner to take that logic out of build_hash_table altogether, and put it in a separate function to be called by ExecInitAgg. regards, tom lane
On Thu, Oct 16, 2008 at 5:26 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > It would probably be cleaner to take that logic out of build_hash_table > altogether, and put it in a separate function to be called by > ExecInitAgg. Yeah, I considered that -- makes sense. Attached is the patch I applied to HEAD, REL8_3_STABLE and REL8_2_STABLE. Neil