Hi,
On 2019-11-13 12:21:47 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-11-13 15:50:04 +0100, Tomas Vondra wrote:
> >> The attached trivial patch fixes that by adding a pfree() at the end of
> >> the function.
>
> > Hm. That's clearly an improvement. But I'm not quite sure it's really
> > the right direction. It seems like a bad idea to rely on
> > ExecMakeTableFunctionResult() otherwise never leaking any memory.
>
> Considering that ExecMakeTableFunctionResult went from zero pallocs
> to one, I don't see a strong reason why it should have bothered with
> a private memory context before, nor do I think that's a good
> response now.
Well, that relies on type_is_rowtype(), exprType(),
TupleDescInitEntry(), lookup_rowtype_tupdesc_copy() not to leak memory
(besides lookup_rowtype_tupdesc_copy()'s return type, which is
explicitly freed). Which is true, but still seems not super reliable.
Thinking about it for a second longer, I don't think we'd need a new
context - afaict argcontext has exactly the lifetime requirements
needed.
Greetings,
Andres Freund