Hi, On 2019-09-22 20:16:15 -0400, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > 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. > > I think you're considering a much smaller slice of the system than > what seems to me to be at risk here.
Yea, I was only referencing the expression eval logic itself, as I understood your question to aim mainly at that... > As an example, an ExprState tree would also include any > fn_extra-linked state that individual functions might've set up. > We've got very little control over what the validity requirements are > for those or how robust the code that creates them is. I *think* that > most of the core code that makes such things is written in a way that > it doesn't leave partially-valid cache state if setup fails partway > through ... but I wouldn't swear that it all is, and I'd certainly bet > money on there being third-party code that isn't careful about that. Hm. I'd be kinda willing to just declare such code broken. But given that the memory leak situation, as you say, still exists, I don't think it matters for now. > > 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. > > Again, that'd do nothing to clean up subsidiary fn_extra state. > If we want no leaks, we need a separate context for the tree > to live in. Well, it'd presumably leak a lot less :/. Greetings, Andres Freund