On Tue, Apr 21, 2026 at 4:11 PM Paul A Jungwirth
<[email protected]> wrote:
>
> On Tue, Apr 21, 2026 at 6:41 AM Aleksander Alekseev
> <[email protected]> wrote:
> >
> > I discovered that it's possible to crash Postgres when using VIEWS,
> > FOR PORTION OF syntax and INSTEAD OF triggers together. See crash.sql.
> >
> > This happens because in ExecModifyTable() around line 4827 there is no
> > check for `relkind == RELKIND_VIEW`. If this is the case `tupleid`
> > ends up being NULL which causes null pointer dereference later when
> > ExecDeleteEpilogue() or ExecUpdateEpilogue() calls
> > ExecForPortionOfLeftovers() with tupleid = NULL. An example stacktrace
> > is attached.
>
> Thanks for testing FOR PORTION OF! This specific bug has been reported
> already and has a patch at [1].
>
> [1]
> https://www.postgresql.org/message-id/CAHg%2BQDd74fnd4obCRMqVS0AVWf%3DcSFH%3DCv7trTJWgm%2B_bhTK6w%40mail.gmail.com
>
> > I propose fixing this by explicitly forbidding using the named
> > features together. See the patch.
>
> I don't think disabling these features is necessary. You are right
> that we can't use the tupleid when we have a view, but I think an
> INSTEAD OF trigger should cause us to skip inserting temporal
> leftovers. (If we didn't do the update, we can't draw conclusions
> about what history was touched vs untouched.)
(Quoting my full message yesterday since I forgot to Reply-All.)
Here is v4 of the fix for this. I'd like to continue the conversation
here since that other thread is huge and attached to the original
commitfest entry for the feature. I'll make a new CF entry for just
this thread.
This patch squashes jian he's test enhancements from his v3 patch.
Also I changed '[2024-01-01,2024-12-31)' to '[2024-01-01,2025-01-01)',
which look less like a mistake (not that it matters to the test), and
I cleaned up the test comment a little.
Yours,
--
Paul ~{:-)
[email protected]
From bfffb6ef789a43befd3b28184339a2e06071bd97 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Tue, 14 Apr 2026 12:10:09 +0800
Subject: [PATCH v4] 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
---
src/backend/executor/nodeModifyTable.c | 20 +++++++++-
src/test/regress/expected/for_portion_of.out | 41 ++++++++++++++++++++
src/test/regress/sql/for_portion_of.sql | 33 ++++++++++++++++
3 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 4cb057ca4f9..8eba0ae3ecf 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1826,7 +1826,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,
@@ -2631,7 +2639,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..5f296f1324e 100644
--- a/src/test/regress/expected/for_portion_of.out
+++ b/src/test/regress/expected/for_portion_of.out
@@ -2097,4 +2097,45 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at;
(4 rows)
DROP TABLE temporal_partitioned;
+-- 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 d4062acf1d1..32ea456b6a2 100644
--- a/src/test/regress/sql/for_portion_of.sql
+++ b/src/test/regress/sql/for_portion_of.sql
@@ -1365,4 +1365,37 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at;
DROP TABLE temporal_partitioned;
+-- 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.47.3
