Joe Conway <[EMAIL PROTECTED]> writes: > Just now I was looking for a way to propagate the necessary information > to call ExecutorEnd() from ExecEndFunctionScan() in the case that fmgr > doesn't. It looks like I might be able to add a member to the > ExprContext struct for this purpose. Does this sound like the correct > (or at least a reasonable) approach?
Yeah, this is something that's bothered me in the past: with the existing API, a function-returning-set will not get a chance to shut down cleanly and release resources if its result is not read all the way to completion. You can demonstrate the problem without any use of the SRF patch. Using current CVS tip (no patch), and the regression database: regression=# create function foo(int) returns setof int as ' regression'# select unique1 from tenk1 where unique2 > $1' regression-# language sql; regression=# select foo(9990) limit 4; WARNING: Buffer Leak: [009] (freeNext=-3, freePrev=-3, rel=16570/135224, blockNum=29, flags=0x4, refcount=1 1) WARNING: Buffer Leak: [021] (freeNext=-3, freePrev=-3, rel=16570/18464, blockNum=232, flags=0x4, refcount=1 1) foo ------ 4093 6587 6093 429 (4 rows) 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. 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. We should also think about the fcache (FunctionCache) struct and whether that needs to tie into this. See the FIXME in utils/fcache.h. regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 4: Don't 'kill -9' the postmaster