David Rowley <dgrowle...@gmail.com> writes:
> On Tue, 6 Sept 2022 at 11:09, Andres Freund <and...@anarazel.de> wrote:
>> I was looking at
>> MemoryContextContains(). Unless I am missing something, the patch omitted
>> adjusting that? We'll probably always return false right now.
> Oops. Yes. I'll push a fix a bit later.
The existing uses in nodeAgg and nodeWindowAgg failed to expose this
because an incorrect false result just causes them to do extra work
(ie, a useless datumCopy). I think there might be a memory leak
too, but the regression tests wouldn't run an aggregation long
enough to make that obvious either.
+1 for adding something to regress.c that verifies that this
works properly for all three allocators. I suggest making
three contexts and cross-checking the correct results for
all combinations of chunk A vs context B.
regards, tom lane
