On Thu, Jun 25, 2026 at 3:32 AM Aleksander Alekseev
<[email protected]> wrote:
>
> This patch rotted and needed a rebase (attached as .txt).
Thank you for taking care of the rebase!
> 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
> ```
You're right, this was due to a copy/paste bug (looking for a delete
trigger in the update code). By splitting the test into two triggers,
we catch the problem and correctly skip the leftovers. I've attached a
patch with that correction.
> 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.
I don't really think this is a new feature. It is a fix to make FOR
PORTION OF not execute when it shouldn't. The change here is quite
simple.
But I don't mind holding it back if that's what people want to do.
Looking more closely at INSTEAD OF triggers, I found another bug: the
FOR PORTION OF qual (and TLE) were not added, so the trigger would
fire on more rows than it should, and NEW.valid_at was not
pre-computed. The second patch here fixes that. I'll defer to others
whether we should fix the INSTEAD OF interaction now or wait 'til v20.
Yours,
--
Paul ~{:-)
[email protected]
From 88ca7956d4b24f6940587d90ac54dcb4a025e725 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
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.
</para>
+ <para>
+ If the updated table has an <literal>INSTEAD OF</literal> trigger, then
+ <productname>PostgreSQL</productname> skips 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 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
From d555595875cbc4e382c8ecf2d9f43dfefc8971b4 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
Date: Sun, 28 Jun 2026 23:34:39 -0700
Subject: [PATCH v6 2/2] Fix INSTEAD OF targeting too many rows with FOR
PORTION OF
In the rewriter, FOR PORTION OF against a view only adds its qual and
TLE after replacing an updatable view with its base table. But with an
INSTEAD OF trigger, we never get to the base table, so the qual and TLE
are never added. This commit adds them to the view itself, if it has an
INSTEAD OF trigger. That way the trigger won't fire on unmatched rows,
and NEW.valid_at will be set correctly.
Discussion: https://postgr.es/m/CAJ7c6TME%2Bix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g%40mail.gmail.com
---
src/backend/rewrite/rewriteHandler.c | 36 ++++++++++++--------
src/test/regress/expected/for_portion_of.out | 25 ++++++++------
src/test/regress/sql/for_portion_of.sql | 17 +++++----
3 files changed, 46 insertions(+), 32 deletions(-)
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index e7ae9cce65f..4683fcaf5e7 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -4253,14 +4253,18 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length,
if (parsetree->forPortionOf)
{
/*
- * Don't add FOR PORTION OF details until we're done rewriting
- * a view update, so that we don't add the same qual and TLE
- * on the recursion.
+ * Add the FOR PORTION OF qual and the range-narrowing TLE.
+ *
+ * For a plain table we add them here. For an auto-updatable
+ * view we defer them: after rewriteTargetView we'll recurse
+ * back here and do it then. But views with INSTEAD OF triggers
+ * never recurse, so we must do those now too.
*
* Views don't need to do anything special here to remap Vars;
* that is handled by the tree walker.
*/
- if (rt_entry_relation->rd_rel->relkind != RELKIND_VIEW)
+ if (rt_entry_relation->rd_rel->relkind != RELKIND_VIEW ||
+ view_has_instead_trigger(rt_entry_relation, CMD_UPDATE, NIL))
{
ListCell *tl;
@@ -4270,7 +4274,10 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length,
*/
AddQual(parsetree, parsetree->forPortionOf->overlapsExpr);
- /* Update FOR PORTION OF column(s) automatically. */
+ /*
+ * Update the FOR PORTION OF column(s) automatically. For an
+ * INSTEAD OF trigger this makes NEW hold the affected portion.
+ */
foreach(tl, parsetree->forPortionOf->rangeTargetList)
{
TargetEntry *tle = (TargetEntry *) lfirst(tl);
@@ -4328,21 +4335,20 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length,
if (parsetree->forPortionOf)
{
/*
- * Don't add FOR PORTION OF details until we're done rewriting
- * a view delete, so that we don't add the same qual on the
- * recursion.
+ * Add qual: DELETE FOR PORTION OF should be limited to rows
+ * that overlap the target range.
+ *
+ * For a plain table we add the qual here. For an auto-updatable
+ * view we defer it: after rewriteTargetView we'll recurse
+ * back here and do it then. But views with INSTEAD OF triggers
+ * never recurse, so we must do those now too.
*
* Views don't need to do anything special here to remap Vars;
* that is handled by the tree walker.
*/
- if (rt_entry_relation->rd_rel->relkind != RELKIND_VIEW)
- {
- /*
- * Add qual: DELETE FOR PORTION OF should be limited to
- * rows that overlap the target range.
- */
+ if (rt_entry_relation->rd_rel->relkind != RELKIND_VIEW ||
+ view_has_instead_trigger(rt_entry_relation, CMD_DELETE, NIL))
AddQual(parsetree, parsetree->forPortionOf->overlapsExpr);
- }
}
}
else
diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out
index 0355f6da9d9..ea287735435 100644
--- a/src/test/regress/expected/for_portion_of.out
+++ b/src/test/regress/expected/for_portion_of.out
@@ -2446,9 +2446,13 @@ 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
+-- Inserting leftovers should be skipped on views with INSTEAD OF triggers.
+-- The FOR PORTION OF range must still restrict which rows are affected, and
+-- UPDATE triggers should see the correct NEW.valid_at.
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);
+INSERT INTO fpo_instead_base VALUES
+ (1, '[2024-01-01,2025-01-01)', 100),
+ (2, '[2020-01-01,2021-01-01)', 200);
CREATE VIEW fpo_instead_view AS SELECT * FROM fpo_instead_base;
CREATE FUNCTION fpo_instead_trig_fn() RETURNS trigger LANGUAGE plpgsql AS $$
BEGIN
@@ -2467,22 +2471,23 @@ CREATE TRIGGER fpo_instead_upd INSTEAD OF UPDATE ON fpo_instead_view
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;
+ SET val = 999;
+NOTICE: UPDATE OLD: (1,"[2024-01-01,2025-01-01)",100), NEW: (1,"[2024-04-01,2024-08-01)",999)
+SELECT * FROM fpo_instead_view ORDER BY id;
id | valid_at | val
----+-------------------------+-----
1 | [2024-01-01,2025-01-01) | 100
-(1 row)
+ 2 | [2020-01-01,2021-01-01) | 200
+(2 rows)
-DELETE FROM fpo_instead_view FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-08-01'
- WHERE id = 1;
+DELETE FROM fpo_instead_view FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-08-01';
NOTICE: DELETE: OLD: (1,"[2024-01-01,2025-01-01)",100)
-SELECT * FROM fpo_instead_view;
+SELECT * FROM fpo_instead_view ORDER BY id;
id | valid_at | val
----+-------------------------+-----
1 | [2024-01-01,2025-01-01) | 100
-(1 row)
+ 2 | [2020-01-01,2021-01-01) | 200
+(2 rows)
DROP VIEW fpo_instead_view;
DROP TABLE fpo_instead_base;
diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql
index 89205f01198..1c0540a2e55 100644
--- a/src/test/regress/sql/for_portion_of.sql
+++ b/src/test/regress/sql/for_portion_of.sql
@@ -1591,9 +1591,13 @@ 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
+-- Inserting leftovers should be skipped on views with INSTEAD OF triggers.
+-- The FOR PORTION OF range must still restrict which rows are affected, and
+-- UPDATE triggers should see the correct NEW.valid_at.
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);
+INSERT INTO fpo_instead_base VALUES
+ (1, '[2024-01-01,2025-01-01)', 100),
+ (2, '[2020-01-01,2021-01-01)', 200);
CREATE VIEW fpo_instead_view AS SELECT * FROM fpo_instead_base;
CREATE FUNCTION fpo_instead_trig_fn() RETURNS trigger LANGUAGE plpgsql AS $$
BEGIN
@@ -1613,12 +1617,11 @@ 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;
+ SET val = 999;
+SELECT * FROM fpo_instead_view ORDER BY id;
-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;
+DELETE FROM fpo_instead_view FOR PORTION OF valid_at FROM '2024-04-01' TO '2024-08-01';
+SELECT * FROM fpo_instead_view ORDER BY id;
DROP VIEW fpo_instead_view;
DROP TABLE fpo_instead_base;
--
2.47.3
