Tom Lane wrote:
> I don't much care for the thought of trawling every expression tree
> looking for functions-returning-set during plan shutdown, so the thought
> that comes to mind is to expect functions that want a shutdown callback
> to register themselves somehow. Adding a list of callbacks to
> ExprContext seems pretty reasonable, but you'd also need some link in
> ReturnSetInfo to let the function find the ExprContext to register
> itself with. Then FreeExprContext would call the callbacks.
I've made changes which fix this and will send them in with a revised
SRF patch later today. Summary of design:
1.) moved the execution_state struct and ExecStatus enum to executor.h
2.) added "void *es" member to ExprContext
3.) added econtext member to ReturnSetInfo
4.) set rsi->econtext on the way in at ExecMakeFunctionResult()
5.) set rsi->econtext->es on the way in at fmgr_sql()
6.) used econtext->es on the way out at ExecFreeExprContext() to call
ExecutorEnd() if needed (because postquel_execute() never got the chance).
One note: I changed ExecFreeExprContext() because that's where all the
action was for SQL function calls. FreeExprContext() was not involved
for the test case, but it looked like it probably should have the same
changes, so I made them there also.
>
> Hmm ... another advantage of doing this is that the function would be
> able to find the ecxt_per_query_memory associated with the ExprContext.
> That would be a Good Thing.
What does this allow done that can't be done today?
>
> We should also think about the fcache (FunctionCache) struct and whether
> that needs to tie into this. See the FIXME in utils/fcache.h.
While I was at it, I added an fcache member to ExprContext, and
populated it in ExecMakeFunctionResult() for SRF cases. I wasn't sure
what else to do with it at the moment, but at least it is a step in the
right direction.
Joe
