Re: An oversight in ExecInitAgg for grouping sets

On Mon, Jan 9, 2023 at 5:21 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> On Thu, 5 Jan 2023 at 20:06, Richard Guo <guofenglinux(at)gmail(dot)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