Horton

Re: memory leak in trigger handling (since PG12)

Tomas Vondra <tomas.vondra@enterprisedb.com>

View thread View on postgr.es
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-06-07T17:05:39Z
Lists: pgsql-hackers 
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

Commits

  1. Fix oversight in handling of modifiedCols since f24523672d

  2. Use per-tuple context in ExecGetAllUpdatedCols

  3. Generated columns