From 88ca7956d4b24f6940587d90ac54dcb4a025e725 Mon Sep 17 00:00:00 2001 From: jian he Date: Tue, 14 Apr 2026 12:10:09 +0800 Subject: [PATCH v6 1/2] Skip FOR PORTION OF leftovers after INSTEAD OF trigger We should not try to insert temporal leftovers following an INSTEAD OF trigger. It will crash because the resultRel is the view, not the base relation, so we can't look up the pre-update/delete row. More essentially, the leftovers are part of original UPDATE/DELETE command, which the trigger replaced, so we shouldn't be executing that. Even if we wanted to, we don't know what the INSTEAD OF trigger did, so we couldn't compute what leftovers are correct. If the user wants leftovers here, the trigger should insert them or use FOR PORTION OF itself. Discussion: https://postgr.es/m/CAHg%2BQDd74fnd4obCRMqVS0AVWf%3DcSFH%3DCv7trTJWgm%2B_bhTK6w%40mail.gmail.com Discussion: https://postgr.es/m/CAJ7c6TME%2Bix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g%40mail.gmail.com --- doc/src/sgml/dml.sgml | 6 +++ src/backend/executor/nodeModifyTable.c | 28 ++++++++++++- src/test/regress/expected/for_portion_of.out | 41 ++++++++++++++++++++ src/test/regress/sql/for_portion_of.sql | 33 ++++++++++++++++ 4 files changed, 106 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/dml.sgml b/doc/src/sgml/dml.sgml index 429aae9bd7b..e6887eb28cb 100644 --- a/doc/src/sgml/dml.sgml +++ b/doc/src/sgml/dml.sgml @@ -389,6 +389,12 @@ DELETE FROM products are not. + + If the updated table has an INSTEAD OF trigger, then + PostgreSQL skips inserting temporal leftovers. + It is the responsibility of the trigger to make whatever changes are desired. + + When temporal leftovers are inserted, all INSERT triggers are fired, but permission checks for inserting rows are diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index c333d7139fa..7f12400e9df 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1437,6 +1437,14 @@ ExecForPortionOfLeftovers(ModifyTableContext *context, oldtupleSlot = fpoState->fp_Existing; leftoverSlot = fpoState->fp_Leftover; + /* + * We only ever insert leftovers into a real table: foreign tables are + * rejected by CheckValidResultRel, and views with INSTEAD OF triggers are + * skipped by our callers (we'd have no base-table tuple to fetch here + * anyway). + */ + Assert(resultRelInfo->ri_RelationDesc->rd_rel->relkind == RELKIND_RELATION); + /* * Get the old pre-UPDATE/DELETE tuple. We will use its range to compute * untouched parts of history, and if necessary we will insert copies with @@ -1814,7 +1822,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, @@ -2619,7 +2635,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_update_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 43408972117..0355f6da9d9 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -2446,4 +2446,45 @@ NOTICE: fpo_before_row1: BEFORE UPDATE ROW: NOTICE: old: [10,100) NOTICE: new: [30,70) DROP TABLE fpo_update_of_trigger; +-- Inserting leftovers should be skipped 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,2025-01-01)', 100); +CREATE VIEW fpo_instead_view AS SELECT * FROM fpo_instead_base; +CREATE FUNCTION fpo_instead_trig_fn() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + IF TG_OP = 'UPDATE' THEN + RAISE NOTICE 'UPDATE OLD: %, NEW: %', OLD, NEW; + RETURN NEW; + ELSIF TG_OP = 'DELETE' THEN + RAISE NOTICE 'DELETE: OLD: %', OLD; + RETURN OLD; + END IF; + RETURN NEW; +END; +$$; +CREATE TRIGGER fpo_instead_upd INSTEAD OF UPDATE ON fpo_instead_view + FOR EACH ROW EXECUTE FUNCTION fpo_instead_trig_fn(); +CREATE TRIGGER fpo_instead_del 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; +NOTICE: UPDATE OLD: (1,"[2024-01-01,2025-01-01)",100), NEW: (1,"[2024-01-01,2025-01-01)",999) +SELECT * FROM fpo_instead_view; + id | valid_at | val +----+-------------------------+----- + 1 | [2024-01-01,2025-01-01) | 100 +(1 row) + +DELETE FROM fpo_instead_view FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-08-01' + WHERE id = 1; +NOTICE: DELETE: OLD: (1,"[2024-01-01,2025-01-01)",100) +SELECT * FROM fpo_instead_view; + id | valid_at | val +----+-------------------------+----- + 1 | [2024-01-01,2025-01-01) | 100 +(1 row) + +DROP VIEW fpo_instead_view; +DROP TABLE fpo_instead_base; +DROP FUNCTION fpo_instead_trig_fn(); RESET datestyle; diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql index 7b08f8cf45e..89205f01198 100644 --- a/src/test/regress/sql/for_portion_of.sql +++ b/src/test/regress/sql/for_portion_of.sql @@ -1591,4 +1591,37 @@ UPDATE fpo_update_of_trigger SET id = 2; DROP TABLE fpo_update_of_trigger; +-- Inserting leftovers should be skipped 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,2025-01-01)', 100); +CREATE VIEW fpo_instead_view AS SELECT * FROM fpo_instead_base; +CREATE FUNCTION fpo_instead_trig_fn() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + IF TG_OP = 'UPDATE' THEN + RAISE NOTICE 'UPDATE OLD: %, NEW: %', OLD, NEW; + RETURN NEW; + ELSIF TG_OP = 'DELETE' THEN + RAISE NOTICE 'DELETE: OLD: %', OLD; + RETURN OLD; + END IF; + RETURN NEW; +END; +$$; +CREATE TRIGGER fpo_instead_upd INSTEAD OF UPDATE ON fpo_instead_view + FOR EACH ROW EXECUTE FUNCTION fpo_instead_trig_fn(); +CREATE TRIGGER fpo_instead_del 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; +DROP FUNCTION fpo_instead_trig_fn(); + RESET datestyle; -- 2.47.3