Re: memory leak in trigger handling (since PG12)

On 5/25/23 16:41, Tomas Vondra wrote:
> ...
>
> The attached patch does this - I realized we actually have estate in
> ExecGetAllUpdatedCols(), so we don't even need a variant with a
> different signature. That makes the patch much simpler.
>
> The question is whether we need the signature anyway. There might be a
> caller expecting the result to be in CurrentMemoryContext (be it
> ExecutorState or something else), and this change might break it. But
> I'm not aware of any callers, so maybe that's fine.
>

I've just pushed a fix along these lines, with a comment explaining that
the caller is expected to copy the bitmap into a different context if
per-tuple context is not sufficient.

IMHO this is the simplest backpatchable solution, and I haven't found
any callers that'd need to do that.

I agree with the idea not to calculate the bitmap over and over, but
that's clearly not backpatchable so it's a matter for separate patch.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company