Hi,
On 2019-09-22 18:43:23 -0400, Tom Lane wrote:
> I'm not convinced that it'd be safe to re-use an ExprState after a
> previous execution failed (though perhaps Andres has a different
> opinion?)
I don't immediately see why it'd be problematic to reuse at a later
time, as long as it's guaranteed that a) there's only one execution
happening at a time b) the failure wasn't in the middle of writing a
value. a) would be problematic regardless of reuse-after-failure, and
b) should be satisfied by only failing at ereport etc.
Most memory writes during ExprState evaluation are redone from scratch
every execution, and the remaining things should only be things like
tupledesc's being cached at first execution. And that even uses an
ExprContext callback to reset the cache on context shutdown.
The other piece is that on the first execution of a expression we use
ExecInterpExprStillValid, and we don't on later executions. Not sure if
that's relevant here?
> so I think the only way to avoid the intratransaction memory leak would
> be to set up each new cast ExprState in its own memory context that we
> could free. That seems like adding quite a lot of overhead to get rid
> of a leak that we've been living with for ages.
Hm. I interestingly am working on a patch that merges all the memory
allocations done for an ExprState into one or two allocations (by
basically doing the traversal twice). Then it'd be feasible to just
pfree() the memory, if that helps.
Greetings,
Andres Freund