Alexander Pyhalov <a.pyhalov@postgrespro.ru> writes:
> I've looked through it and made some tests, including ones which
> caused me to create separate context for planing. Was a bit worried
> that it has gone, but now, as fcache->fcontext is deleted in the end
> of function execution, I don't see leaks, which were the initial
> reason for introducing it.
Yeah. As it's set up in v10, we do parsing work in the caller's
context (which is expected to be short-lived) when creating or
recreating the long-lived cache entry. However, planning work
(if needed) is done in the fcontext, since that will happen within
init_execution_state which is called after fmgr_sql has switched into
the fcontext. I thought about switching back to the caller's context
but decided that it wouldn't really be worth the trouble. For a
non-SRF there's no meaningful difference anyway. For a SRF, it'd mean
that planning cruft survives till end of execution of the SRF rather
than possibly going away when the first row is returned. But we
aren't looping: any given query within the SRF is planned only once
during an execution of the function. So there's no possibility of
indefinite accumulation of leakage.
If we wanted to improve that, my inclination would be to try to
not switch into the fcontext for the whole of fmgr_sql, but only
use it explicitly for allocations that need to survive. But I
don't think that'll save much, so I think any such change would
be best left for later. The patch is big enough already.
regards, tom lane
