On Mon, 2008-02-25 at 21:00 -0500, Tom Lane wrote: > I find this part of the patch to be a seriously bad idea. > nodeFunctionscan has no right to assume that the function has returned > an expendable tupdesc; indeed, I would think that the other case is > more nearly what's expected by the API for SRFs.
AFAICS this is not true of any of the SRFs in the backend, which always return expendable tupdescs. > A safer fix would be to try to make the tupdesc be in the new > multi_call_ctx when it's being created by the funcapi.c code. funcapi.c doesn't create the tupdesc, though: it is created by user code, and merely assigned to a field of the RSI (note that the TupleDesc in question is the one in the ReturnSetInfo, not in FuncCallContext.) A minor detail is that the lifetime of the tupdesc also needs to exceed the lifetime of the multi_call_ctx, at least as currently implemented. >From looking at the existing SRFs in the backend, the TupleDesc is always *explicitly* allocated in the estate's per-query context, so it will leak unless freed. Perhaps it would be sufficient to FreeTupleDesc iff tupdesc->refcount == -1? BTW, I'm thinking of changing the various SRFs that make allocations in the EState's per-query context to instead use the SRF's multi_call_ctx. This means the memory will be reclaimed sooner and avoids possible memory leaks. -Neil ---------------------------(end of broadcast)--------------------------- TIP 2: Don't 'kill -9' the postmaster