public inbox for [email protected]
help / color / mirror / Atom feedFrom: Paul A Jungwirth <[email protected]>
To: jian he <[email protected]>
Cc: SATYANARAYANA NARLAPURAM <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Chao Li <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: SQL:2011 Application Time Update & Delete
Date: Wed, 15 Apr 2026 16:25:58 -0700
Message-ID: <CA+renyW9o=-DBK6YO7a31xbr6xw7RFN_tSxTiUi2tSOQ2w5_zQ@mail.gmail.com> (raw)
In-Reply-To: <CACJufxEaD_DtByjv4CZg3yKg0n6hMPThfOmTr-D6JKY+v1BJDQ@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<CA+renyUazgR-hB_6RY60n23L0y-n_h9G1AappZmPENO0k5pL1g@mail.gmail.com>
<[email protected]>
<CA+renyVXg5pV84wQnGQuK8-=qoKw3BiBgQzesxM_LkcxxWmYjA@mail.gmail.com>
<[email protected]>
<CA+renyWKOj5=rMmQmJcbybu-Vdomxdp=eJ93kp76AgmQKYdfiQ@mail.gmail.com>
<[email protected]>
<CA+renyUhuXB2nTVCMREXew9E4DZOnFxQNjME5bcw91+k72Bosg@mail.gmail.com>
<CA+renyWUCSyTMn3s03kviEN-oaVrJP-QkDQCLNfaY=MHV5QEiQ@mail.gmail.com>
<CA+renyV4tWU2d=n9_v=XNPHbZfNqqLokzd-Xt78M-zLd+46ubA@mail.gmail.com>
<[email protected]>
<CA+renyUSgqXpjj+vV7w+wirPB49VQFrmPjVT_s04JmZSOPNNsQ@mail.gmail.com>
<[email protected]>
<CA+renyX-eV+2hFUaZg3BSREqLE7dh+LoWm7ZqhFAiGsirjjtRQ@mail.gmail.com>
<[email protected]>
<CAHg+QDckLFqthQyox2NDetYRs9sRrjmAiSA-gYRowyg8w_4vgw@mail.gmail.com>
<CAHg+QDd74fnd4obCRMqVS0AVWf=cSFH=Cv7trTJWgm+_bhTK6w@mail.gmail.com>
<CAHg+QDcVL2d4ih5zs2Mzh63ts41N+jtnMQTdZ2_0be6aF4aqYQ@mail.gmail.com>
<CACJufxEaD_DtByjv4CZg3yKg0n6hMPThfOmTr-D6JKY+v1BJDQ@mail.gmail.com>
On Mon, Apr 13, 2026 at 9:33 PM jian he <[email protected]> wrote:
>
> On Fri, Apr 10, 2026 at 3:42 AM SATYANARAYANA NARLAPURAM
> <[email protected]> wrote:
> >
> >> Repro:
> >>
> >> CREATE TABLE t (id INT, valid_at daterange, val INT);
> >> INSERT INTO t VALUES (1, '[2026-01-01,2026-12-31)', 100);
> >> CREATE VIEW v AS SELECT * FROM t;
> >>
> >> CREATE FUNCTION v_trig() RETURNS trigger LANGUAGE plpgsql AS $$
> >> BEGIN
> >> UPDATE t SET val = NEW.val WHERE id = OLD.id;
> >> RETURN NEW;
> >> END;
> >> $$;
> >> CREATE TRIGGER trg INSTEAD OF UPDATE ON v
> >> FOR EACH ROW EXECUTE FUNCTION v_trig();
> >>
> >> -- This crashes the server:
> >> UPDATE v FOR PORTION OF valid_at FROM '2026-04-01' TO '2026-08-01'
> >> SET val = 999 WHERE id = 1;
> >>
> >> I am thinking we should just reject this case. Attached a draft patch to fix the issue.
> >
> Yech, we should reject it.
I think using INSTEAD OF triggers to replace an UPDATE/DELETE FOR
PORTION OF is a valid use-case, but it doesn't make sense to insert
temporal leftovers. As you say, we can't access the underlying
storage. But also we don't know what changes the trigger actually
made. The trigger should be responsible for leftovers, and we
shouldn't try to add more. So I think the fix is just to skip
inserting leftovers. I've attached a patch to do that.
This is a good use-case for a pending followup patch (which will have
to wait for v20 I think), which makes the FOR PORTION OF parameters
accessible to triggers. We need that ourselves for PERIOD foreign keys
with CASCADE/SET NULL/SET DEFAULT, but it's nice to have another
example of why you might want it.
Yours,
--
Paul ~{:-)
[email protected]
Attachments:
[application/octet-stream] v2-0001-Fix-INSTEAD-OF-triggers-with-DELETE-UPDATE-FOR-PO.patch (5.5K, 2-v2-0001-Fix-INSTEAD-OF-triggers-with-DELETE-UPDATE-FOR-PO.patch)
download | inline diff:
From 8d4362529ee769c497952a4939909cfbbe8453d0 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
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
view thread (54+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: SQL:2011 Application Time Update & Delete
In-Reply-To: <CA+renyW9o=-DBK6YO7a31xbr6xw7RFN_tSxTiUi2tSOQ2w5_zQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox