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
