Hi Paul, > > > I definitely don't like checking volatility at parse time, though. > > > > So.... the last comment from Tom was that he is not happy :) but I'm > > not sure about the actionable items. Tom, could you please elaborate a > > bit? > > Here is the thread for moving checks out of analysis: > https://www.postgresql.org/message-id/CA%2BrenyUte0_UJsJiDJQi82oaBsMJn%3Dcct0Wn%3DvOqXtuDn%3DYYJA%40mail.gmail.com
Thanks for the clarification! So if my understanding is correct this is out of scope of the fix under question. > I don't think forbidding INSTEAD OF triggers with FOR PORTION OF is > the right solution. It seems too heavy-handed. But we *should* skip > trying to insert temporal leftovers. After all if you did something > *instead of* the update/delete, we can't know what the leftovers > should be. Or another way of putting it: the trigger function runs > instead of the original command, but inserting leftovers is part of > that original command. Skipping temporal leftovers is implemented in > my v5 patch on this thread from April 22: > https://www.postgresql.org/message-id/CA%2BrenyUoTRnn0o4Pnfy2AOdtqMH3%2Bn29_AfD4Aih3ifwMX9vyA%40mail.gmail.com This patch rotted and needed a rebase (attached as .txt). Also it's incorrect, it only masks the crash: ``` CREATE TABLE t (id int, valid_at daterange, val int); INSERT INTO t VALUES (1, '[2024-01-01,2025-01-01)', 100); CREATE VIEW v AS SELECT * FROM t; CREATE FUNCTION trig_fn() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RAISE NOTICE 'UPDATE OLD: %, NEW: %', OLD, NEW; RETURN NEW; END; $$; CREATE TRIGGER trig INSTEAD OF UPDATE ON v FOR EACH ROW EXECUTE FUNCTION trig_fn(); UPDATE v FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-08-01' SET val = 999 WHERE id = 1; server closed the connection unexpectedly ``` v1-0001 passes the tests successfully: ``` =# UPDATE v FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-08-01' SET val = 999 WHERE id = 1; ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF LINE 1: UPDATE v FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-... ``` IMO it's a little bit late in the PG19 cycle for making this work. This is an implementation of a new feature which is not present at the moment which to my knowledge we don't do after the feature freeze. Our goal is to fix the crash and leave the rest for the PG20 cycle. Clearly the feature needs more discussion and thorough testing. -- Best regards, Aleksander Alekseev
From be515d091b4923c29cd8477c1c7a5eb7273a2f1f Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Tue, 14 Apr 2026 12:10:09 +0800 Subject: [PATCH v6] Skip FOR PORTION OF leftovers after INSTEAD OF trigger 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 Discussion: https://postgr.es/m/CAJ7c6TME%2Bix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g%40mail.gmail.com --- doc/src/sgml/dml.sgml | 7 ++++ src/backend/executor/nodeModifyTable.c | 20 +++++++++- src/test/regress/expected/for_portion_of.out | 41 ++++++++++++++++++++ src/test/regress/sql/for_portion_of.sql | 33 ++++++++++++++++ 4 files changed, 99 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/dml.sgml b/doc/src/sgml/dml.sgml index 429aae9bd7b..4b29e675f2e 100644 --- a/doc/src/sgml/dml.sgml +++ b/doc/src/sgml/dml.sgml @@ -389,6 +389,13 @@ DELETE FROM products are not. </para> + <para> + If the updated table has an <literal>INSTEAD OF</literal> trigger, then + <productname>PostgreSQL</productname> skips updating the start/end times + or inserting temporal leftovers. It is the responsibility of the trigger + to make whatever changes are desired. + </para> + <para> When temporal leftovers are inserted, all <literal>INSERT</literal> 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 846dc516b43..055856ba3d4 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1814,7 +1814,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 +2627,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 43408972117..61f588ea387 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 = 'INSERT' then + raise NOTICE 'INSERT NEW: %', NEW; + RETURN NEW; + elsif TG_OP = 'DELETE' then + raise NOTICE 'DELETE: OLD: %', OLD; + RETURN OLD; + end if; + RETURN NEW; +END; +$$; +CREATE TRIGGER fpo_instead_trig INSTEAD OF UPDATE OR 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; RESET datestyle; diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql index 7b08f8cf45e..a397838a3c6 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 = 'INSERT' then + raise NOTICE 'INSERT NEW: %', NEW; + RETURN NEW; + elsif TG_OP = 'DELETE' then + raise NOTICE 'DELETE: OLD: %', OLD; + RETURN OLD; + end if; + RETURN NEW; +END; +$$; +CREATE TRIGGER fpo_instead_trig INSTEAD OF UPDATE OR 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.43.0
