Обсуждение: 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

"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