From 8d4362529ee769c497952a4939909cfbbe8453d0 Mon Sep 17 00:00:00 2001 From: jian he Date: Tue, 14 Apr 2026 12:10:09 +0800 Subject: [PATCH v2] Fix INSTEAD OF triggers with DELETE/UPDATE FOR PORTION OF We should not try to insert temporal leftovers following an INSTEAD OF trigger. For one thing, the resultRel is the view, not the base relation, so we can't look up the pre-update/delete row. But also, the INSTEAD OF trigger is responsible for doing the work, and we don't know what it really did. If it wants leftovers, it should insert them or use FOR PORTION OF itself. Discussion: https://postgr.es/m/CAHg%2BQDd74fnd4obCRMqVS0AVWf%3DcSFH%3DCv7trTJWgm%2B_bhTK6w%40mail.gmail.com --- src/backend/executor/nodeModifyTable.c | 20 +++++++++++-- src/test/regress/expected/for_portion_of.out | 31 ++++++++++++++++++++ src/test/regress/sql/for_portion_of.sql | 25 ++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index ef2a6bc6e9d..c7784cc896e 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1810,7 +1810,15 @@ ExecDeleteEpilogue(ModifyTableContext *context, ResultRelInfo *resultRelInfo, /* Compute temporal leftovers in FOR PORTION OF */ if (((ModifyTable *) context->mtstate->ps.plan)->forPortionOf) - ExecForPortionOfLeftovers(context, estate, resultRelInfo, tupleid); + { + /* + * Skip leftovers if there were INSTEAD OF triggers. + * We would have no way of accessing the old row. + */ + if (!resultRelInfo->ri_TrigDesc || + !resultRelInfo->ri_TrigDesc->trig_delete_instead_row) + ExecForPortionOfLeftovers(context, estate, resultRelInfo, tupleid); + } /* AFTER ROW DELETE Triggers */ ExecARDeleteTriggers(estate, resultRelInfo, tupleid, oldtuple, @@ -2615,7 +2623,15 @@ ExecUpdateEpilogue(ModifyTableContext *context, UpdateContext *updateCxt, /* Compute temporal leftovers in FOR PORTION OF */ if (((ModifyTable *) context->mtstate->ps.plan)->forPortionOf) - ExecForPortionOfLeftovers(context, context->estate, resultRelInfo, tupleid); + { + /* + * Skip leftovers if there were INSTEAD OF triggers. + * We would have no way of accessing the old row. + */ + if (!resultRelInfo->ri_TrigDesc || + !resultRelInfo->ri_TrigDesc->trig_delete_instead_row) + ExecForPortionOfLeftovers(context, context->estate, resultRelInfo, tupleid); + } /* AFTER ROW UPDATE Triggers */ ExecARUpdateTriggers(context->estate, resultRelInfo, diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out index 31f772c723d..1afa26c86bc 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -2097,4 +2097,35 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at; (4 rows) DROP TABLE temporal_partitioned; +-- Test: FOR PORTION OF should be rejected on views with INSTEAD OF triggers +CREATE TABLE fpo_instead_base (id int, valid_at daterange, val int); +INSERT INTO fpo_instead_base VALUES (1, '[2024-01-01,2024-12-31)', 100); +CREATE VIEW fpo_instead_view AS SELECT * FROM fpo_instead_base; +CREATE FUNCTION fpo_instead_trig_fn() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + RETURN NEW; +END; +$$; +CREATE TRIGGER fpo_instead_trig INSTEAD OF UPDATE ON fpo_instead_view + FOR EACH ROW EXECUTE FUNCTION fpo_instead_trig_fn(); +CREATE TRIGGER fpo_instead_del_trig INSTEAD OF DELETE ON fpo_instead_view + FOR EACH ROW EXECUTE FUNCTION fpo_instead_trig_fn(); +UPDATE fpo_instead_view FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-08-01' + SET val = 999 WHERE id = 1; +SELECT * FROM fpo_instead_view; + id | valid_at | val +----+-------------------------+----- + 1 | [2024-01-01,2024-12-31) | 100 +(1 row) + +DELETE FROM fpo_instead_view FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-08-01' + WHERE id = 1; +SELECT * FROM fpo_instead_view; + id | valid_at | val +----+-------------------------+----- + 1 | [2024-01-01,2024-12-31) | 100 +(1 row) + +DROP VIEW fpo_instead_view; +DROP TABLE fpo_instead_base; RESET datestyle; diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql index d4062acf1d1..0b5a86408b9 100644 --- a/src/test/regress/sql/for_portion_of.sql +++ b/src/test/regress/sql/for_portion_of.sql @@ -1365,4 +1365,29 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at; DROP TABLE temporal_partitioned; +-- Test: FOR PORTION OF should be rejected on views with INSTEAD OF triggers +CREATE TABLE fpo_instead_base (id int, valid_at daterange, val int); +INSERT INTO fpo_instead_base VALUES (1, '[2024-01-01,2024-12-31)', 100); +CREATE VIEW fpo_instead_view AS SELECT * FROM fpo_instead_base; +CREATE FUNCTION fpo_instead_trig_fn() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + RETURN NEW; +END; +$$; +CREATE TRIGGER fpo_instead_trig INSTEAD OF UPDATE ON fpo_instead_view + FOR EACH ROW EXECUTE FUNCTION fpo_instead_trig_fn(); +CREATE TRIGGER fpo_instead_del_trig INSTEAD OF DELETE ON fpo_instead_view + FOR EACH ROW EXECUTE FUNCTION fpo_instead_trig_fn(); + +UPDATE fpo_instead_view FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-08-01' + SET val = 999 WHERE id = 1; +SELECT * FROM fpo_instead_view; + +DELETE FROM fpo_instead_view FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-08-01' + WHERE id = 1; +SELECT * FROM fpo_instead_view; + +DROP VIEW fpo_instead_view; +DROP TABLE fpo_instead_base; + RESET datestyle; -- 2.45.0