Fix trigger memory leak

On Nov 1, 2026, at 01:17, Paul A Jungwirth <pj@illuminatedcomputing.com> wrote:

Hi Hackers,

Here is a patch to fix some memory leaks in BEFORE/INSTEAD OF triggers.

Andres reported the problem in [0] while reviewing FOR PORTION OF, but
as he suspected, it is a problem on master too. It goes back to v14 at
least. You can reach it if (1) the rel is not a regular table (e.g. a
view) and (2) the trigger returns NEW without changing it (or NULL in
ExecIRInsertTriggers).

[0] /messages/by-id/yfers7d2kizq6ockz4nu7e7avq3gj5anpd7ilxkazyvicv4qgy@3cesow2bbhnx

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com
<v1-0001-Free-the-tuple-fetched-for-BEFORE-INSTEAD-OF-row-.patch>

V1 overall looks good to me. There seems a typo in the commit message:
```
heap slot) and (2) the trigger returned NEW unchanged (or NULL in
ExecIRInsertTriggers).
```

I think it should be “or NULL in ExecIRUpdateTriggers”.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Hi Paul,

I tested v1 against master using PL/pgSQL triggers that return NEW
without assigning to it, with a single statement over 50k rows and a 1kB
text column.

For INSTEAD OF INSERT and UPDATE on a view, ExecutorState grows to about
104MB on master, and the same happens when the INSTEAD OF UPDATE trigger
returns NULL. With v1 it stays flat in all three cases. If the trigger
assigns to NEW at all (even NEW.id := NEW.id), the leak goes away, since
PL/pgSQL then returns a new tuple.

I also found that you don't need a view to hit this. A plain BEFORE
INSERT trigger on a partitioned table leaks just as much when the row is
routed to a partition with the same column layout as the parent. In that
case there's no conversion map, so ExecPrepareTupleRouting() passes the
root's virtual slot straight through and ExecBRInsertTriggers() ends up
with a copy it never frees. Inserting directly into the partition, or
routing to one with a different column order, doesn't leak. Maybe we can
put that in the commit message but not really needed.

v1 looks good to me.

Here's the partitioned case, in case it's useful. It prints about 105MB
on master and about 544kB with v1:

CREATE TABLE p (id int, pad text) PARTITION BY RANGE (id);
CREATE TABLE p1 PARTITION OF p FOR VALUES FROM (0) TO (100000);

CREATE FUNCTION trg() RETURNS trigger LANGUAGE plpgsql AS $$
BEGIN
IF NEW.id = 50000 THEN
RAISE NOTICE 'ExecutorState: %', (SELECT
pg_size_pretty(max(total_bytes))
FROM pg_backend_memory_contexts WHERE name = 'ExecutorState');
END IF;
RETURN NEW;
END $$;
CREATE TRIGGER t BEFORE INSERT ON p FOR EACH ROW EXECUTE FUNCTION trg();

INSERT INTO p SELECT g, repeat('x', 1000) FROM generate_series(1, 50000)
g;

Regards,
Surya Poondla

On Mon, Nov 2, 2026 at 5:34 AM Chao Li <li.evan.chao@gmail.com> wrote:

V1 overall looks good to me. There seems a typo in the commit message:
```
heap slot) and (2) the trigger returned NEW unchanged (or NULL in
ExecIRInsertTriggers).
```

I think it should be "or NULL in ExecIRUpdateTriggers".

Thank you for the review! You're right; fixed.

On Mon, Nov 2, 2026 at 11:03 PM surya poondla <suryapoondla4@gmail.com> wrote:

I also found that you don't need a view to hit this. A plain BEFORE
INSERT trigger on a partitioned table leaks just as much when the row is
routed to a partition with the same column layout as the parent. In that
case there's no conversion map, so ExecPrepareTupleRouting() passes the
root's virtual slot straight through and ExecBRInsertTriggers() ends up
with a copy it never frees. Inserting directly into the partition, or
routing to one with a different column order, doesn't leak. Maybe we can
put that in the commit message but not really needed.

Thank you too! I updated the commit message to mention that case.

v2 attached.

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com

Hi Paul,

Thanks for the update. v2 looks good to me.

Regards,
Surya Poondla