Re: An oversight in ExecInitAgg for grouping sets - Mailing list pgsql-hackers

From Richard Guo
Subject Re: An oversight in ExecInitAgg for grouping sets
Date
Msg-id CAMbWs4-oPinvc+50L+fwd9e+3Z8YcTs7D3GpL-558StyLY-WLA@mail.gmail.com
Whole thread Raw
In response to Re: An oversight in ExecInitAgg for grouping sets  (David Rowley <dgrowleyml@gmail.com>)
Responses Re: An oversight in ExecInitAgg for grouping sets  (David Rowley <dgrowleyml@gmail.com>)
List pgsql-hackers

On Mon, Jan 9, 2023 at 5:21 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Thu, 5 Jan 2023 at 20:06, Richard Guo <guofenglinux@gmail.com> wrote:
> I reviewed this patch and have some comments.

Thanks for looking at this. I think I've fixed all the issues you mentioned.

One extra thing I noticed was that I had to add a new
VALGRIND_MAKE_MEM_DEFINED in AllocSetAlloc when grabbing an item off
the freelist. I didn't quite manage to figure out why that's needed as
when we do AllocSetFree() we don't mark the pfree'd memory with
NOACCESS, and it also looks like AllocSetReset() sets the keeper
block's memory to NOACCESS, but that function also clears the
freelists too, so the freelist chunk is not coming from a recently
reset context.

I might need to spend a bit more time on this to see if I can figure
out why this is happening.  On the other hand, maybe we should just
mark pfree'd memory as NOACCESS as that might find another class of
issues.

It occurred to me that this hasn't been applied.  Should we add it to
the CF to not lose track of it?

Thanks
Richard

pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: Privileges on PUBLICATION
Next
From: Michael Paquier
Date:
Subject: Re: RADIUS tests and improvements