Hi,
On 2019-01-14 11:57:55 -0500, Tom Lane wrote:
> I wrote:
> > Andres Freund <andres@anarazel.de> writes:
> >> Think I know where the problem is - let me have a coffee and check? I think I might have a good lying around...
>
> > I had just determined that the extra context was added by bf6c614a2,
> > which at this point has accumulated a pretty serious collection of
> > bugs, judging by subsequent mentions in the commit log.
Hm, there was one bug related to this before? Fixing up test failures in
sepgsql (which I can't run locally) just after commit doesn't really
count.
> > I had just determined that the extra context was added by bf6c614a2,
>
> The short answer here is that this addition to BuildTupleHashTable:
>
> hashtable->exprcontext = CreateExprContext(parent->state);
>
> allocates memory that is not freed by freeing the hashtable's tablecxt,
> breaking the API for grouping hashtables.
>
> Why does a hashtable need its own exprcontext now when it didn't before?
Because the comparison is now done via ExprState machinery than manual
fmgr invocations. I'd discussed a patch solving this a few weeks ago
with Andrew Gierth, but got stuck with the fact that essentially all
fixes entail either an API or an ABI break - but I think we gotta do
that anyway.
For performance reasons the API really should be changed so we don't
allocate/deallocate the entire hashtable on each round - I've prototyped
a fix that only resets it and there's noticable performance gains. But
that's not something we can easily do in the back branches.
Let me rebase and cleanup my patches, it's probably easier to discuss
with them in front of us.
Greetings,
Andres Freund
