Andres Freund <andres@anarazel.de> writes:
> On 2019-02-09 09:34:41 -0500, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> ... Given that, I think it's ok
>>> to not explicitly shutdown the expr context.
>> Uh, what?
> Well, you explicitly removed the surrounding reasoning. What's the issue
> you see here? The generated expression cannot contain anything that uses
> shutdown callbacks
Even if that happens to be true today, it certainly looks like a booby
trap primed and ready to bite somebody in the future. An ExprContext
that fails to provide one of the basic services of an ExprContext ---
indeed, the *only* basic service of an ExprContext, since whatever else
it does is just a memory context feature --- doesn't sound like a great
plan to me.
What is it you're actually hoping to do by removing this guarantee?
(I apologize for not having been paying attention to this thread,
but I do have finite bandwidth.)
>> This doesn't really seem like the kind of patch to push on a release
>> weekend. At this point you can't even be confident of getting a readout
>> from every active buildfarm member.
> Yea, I'd hoped to push this earlier, but unfortune family issues +
> related travel + work travel prevented me from getting to this
> earlier. The memory leak is significant, the patch hasn't materially
> changed in the two weeks since it has been posted, and there's nothing
> operating system / architecture related, which I think makes this
> acceptable.
I'm thinking more of the valgrind and clobber_cache_always members,
which are pretty slow by definition.
regards, tom lane