Hi,
On 2019-09-22 20:16:15 -0400, Tom Lane wrote:
> I think you're considering a much smaller slice of the system than
> what seems to me to be at risk here. As an example, an ExprState
> tree would also include any fn_extra-linked state that individual
> functions might've set up.
> > Hm. I interestingly am working on a patch that merges all the memory
> > allocations done for an ExprState into one or two allocations (by
> > basically doing the traversal twice). Then it'd be feasible to just
> > pfree() the memory, if that helps.
>
> Again, that'd do nothing to clean up subsidiary fn_extra state.
> If we want no leaks, we need a separate context for the tree
> to live in.
As mentioned, as part of some expression evaluation improvements (both
w/ and wo/ jit) I now have a prototype patch that moves nearly all the
dynamically allocated memory, including the FunctionCallInfo, into one
chunk of memory. That's currently allocated together with the ExprState.
With the information collected for that it'd be fairly trivial to reset
things like fn_extra in a reasonably efficient manner, without needing
knowledge about each ExprEvalOp.
Obviously that'd not itself change e.g. anything about the lifetime of
the memory pointed to by fn_extra etc. But it'd not be particularly hard
to have FmgrInfo->fn_mcxt point somewhere else.
As part of the above rework ExecInitExprRec() etc all now pass down a
new ExprStateBuilder * object, containing state needed somewhere down in
the expression tree. It'd e.g. be quite possible to add an
ExecInitExpr() variant that allows to specify in more detail what memory
context ought to be used for what.
I'm however not at all sure it's worth investing time into doing so
specifically for this case. But it seems like it might generally be
something we'll need more infrastructure for in other cases too.
Greetings,
Andres Freund