2015-01-20 14:22 GMT+07:00 Jeff Davis <pg...@j-davis.com>: > The current patch, which I am evaluating for commit, does away with > per-group memory contexts (it uses a common context for all groups), and > reduces the initial array allocation from 64 to 8 (but preserves > doubling behavior).
Jeff & Tomas, spotted this comment in v8 patch: @@ -4713,6 +4733,11 @@ accumArrayResult(ArrayBuildState *astate, /* * makeArrayResult - produce 1-D final result of accumArrayResult * + * If the array build state was initialized with a separate memory context, + * this also frees all the memory (by deleting the subcontext). If a parent + * context was used directly, the memory may be freed by an explicit pfree() + * call (unless it's meant to be freed by destroying the parent context). + * * astate is working state (must not be NULL) * rcontext is where to construct result */ Simple pfree(astate) call is not enough to free the memory. If it's scalar accumulation (initArrayResult), the user must pfree(astate->dvalues) and pfree(astate->dnulls) before astate. If it's array accumulation, pfree(astate->data) and pfree(astate->nullbitmap), with both can be null if no array accumulated and some other cases. If its any (scalar or array) accumulation, it's more complicated. I suggest it's simpler to just force the API user to destroy the parent context instead. So the comment become like this: @@ -4713,6 +4733,11 @@ accumArrayResult(ArrayBuildState *astate, /* * makeArrayResult - produce 1-D final result of accumArrayResult * + * If the array build state was initialized with a separate memory context, + * this also frees all the memory (by deleting the subcontext). If a parent + * context was used directly, the memory is meant to be freed by destroying + * the parent context. + * * astate is working state (must not be NULL) * rcontext is where to construct result */ Regards, -- Ali Akbar
