>>> * I removed the "buildercxt" memory context. It seemed mostly >>> pointless, and I was disturbed by the MemoryContextResetOnly(). >>> Per-value memory still uses the per-value memory context, but the >>> rest of the stuff is in the per-query context, which should be >>> pretty much the same.
Andrew> A quick reading suggests that the per-value context should have Andrew> been changed to not be a child of "buildercxt" (which would Andrew> avoid the MemoryContextResetOnly issue - that's a good sign Andrew> that you've put a child context under the wrong parent). But Andrew> the use of the per-query context instead is exactly what causes Andrew> this blowup; compare with what nodeFunctionscan does with its Andrew> "argcontext" (and the corresponding comments in Andrew> ExecMakeTableFunctionResult).
And here's a patch (against pg10, but I don't think the code's changed since) that I think does it the right way.
This works by changing the meaning of perValueCxt - it's now the per-INPUT-value context (equivalent to FunctionScan's argcontext, but I didn't change the name for simplicity), not per-output-value; for the latter, we use the result exprcontext's per-tuple memory like everything else does (cf. FunctionScan).
I've verified that this fixes the leak; it also passes all other tests I have thrown at it. Anyone see any issues with this? (CC'ing in Pavel as original author)
I'll apply it in due course unless anyone says otherwise.
