Re: Memory leak in nodeAgg

Neil Conway <neilc(at)samurai(dot)com> writes:
> Attached is a patch that fixes a gradual memory leak in ExecReScanAgg(),
> when the AGG_HASHED strategy is used:

Hmm. Good catch, but I can't help wondering if this is just the tip
of the iceberg. Should *every* MemoryContextReset be
MemoryContextResetAndDeleteChildren?

What this probably boils down to is whether there are cases where we
keep pointers to a sub-context in some place other than the parent
context. I remember thinking there would be cases like that when I
proposed the current memory context API, but now I'm less sure that
it's a good idea.

If we redefined MemoryContextReset to be the same as
MemoryContextResetAndDeleteChildren, it'd be possible to keep the
headers for child contexts in their parent context, thus easing
traffic in TopMemoryContext, and perhaps saving a few pfree cycles
when resetting the parent (since we'd not bother explicitly releasing
the child headers). But that would be a pain to undo if it turned out
wrong.

Anyone want to investigate what happens if we make MemoryContextReset
the same as MemoryContextResetAndDeleteChildren?

regards, tom lane