public inbox for [email protected]
help / color / mirror / Atom feedRe: [PATCH] Fix null pointer dereference in PG19
13+ messages / 4 participants
[nested] [flat]
* Re: [PATCH] Fix null pointer dereference in PG19
@ 2026-06-24 08:52 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Peter Eisentraut @ 2026-06-24 08:52 UTC (permalink / raw)
To: Paul A Jungwirth <[email protected]>; jian he <[email protected]>; +Cc: Tom Lane <[email protected]>; Aleksander Alekseev <[email protected]>; pgsql-hackers
On 15.05.26 23:21, Paul A Jungwirth wrote:
> On Thu, Apr 23, 2026 at 9:45 PM jian he <[email protected]> wrote:
>>
>> On Tue, Apr 21, 2026 at 11:24 PM Tom Lane <[email protected]> wrote:
>>>
>>> * I'd tend to move the anti-FDW check to execution too.
>>> It's not actively wrong, since nowadays we don't permit
>>> relations to change relkind, but it seems out of place.
>>> Also, it seems inadequate to deal with the case of a target
>>> that is a partitioned table having FDW partitions.
>>>
>> Hi.
>>
>> Instead of adding another subnode in CheckValidResultRel,
>> I am passing ModifyTable to it, this will be more future-proof.
>
> Thank you for working on this! I started a new thread at [1] so that I
> could give it a commitfest entry.
>
> Your patch looks great to me. I didn't notice that you had posted one,
> so I made my own, but yours is better. Doing the check in
> CheckValidResultRel makes a lot of sense.
>
> I thought there was one other case we should test for: when a
> partition has a child FDW that gets pruned, we should not raise an
> error. So I swapped in my own tests, which were otherwise similar to
> yours.
>
> That new thread also includes a patch to move the functionality check
> into plan-time.
>
> [1] https://www.postgresql.org/message-id/CA%2BrenyUte0_UJsJiDJQi82oaBsMJn%3Dcct0Wn%3DvOqXtuDn%3DYYJA%40...
I don't understand the status of this discussion. The original bug
report is related to views and INSTEAD OF triggers. But the last few
patches that have been posted here appear to be unrelated to that
specific issue.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
@ 2026-06-24 11:12 ` Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Aleksander Alekseev @ 2026-06-24 11:12 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Paul A Jungwirth <[email protected]>; jian he <[email protected]>
Hi Peter,
Thanks for your attention to this.
> I don't understand the status of this discussion. The original bug
> report is related to views and INSTEAD OF triggers. But the last few
> patches that have been posted here appear to be unrelated to that
> specific issue.
That makes two of us. My humble understanding is that there are
several related discussions but they are happening elsewhere now. Feel
free correcting me if this is not the case.
Here I propose to focus on a particular crash and the particular
proposed bugfix, v1-0001. Perhaps it should be rewritten or maybe we
should reject it in favor of another patch. Both are possible
outcomes. Right now I'm only interested in fixing a particular crash.
> 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?
--
Best regards,
Aleksander Alekseev
Attachments:
[text/x-patch] v1-0001-Forbid-FOR-PORTION-OF-on-views-with-INSTEAD-OF-tr.patch (4.4K, ../../CAJ7c6TO+kTwk5MBayLARxsPk-YCsDt2QH9P_PeTYoc3zPYCS3Q@mail.gmail.com/2-v1-0001-Forbid-FOR-PORTION-OF-on-views-with-INSTEAD-OF-tr.patch)
download | inline diff:
From 63b6699d5c03405f36396dd98c54f53a4bcf40d3 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Tue, 21 Apr 2026 15:46:04 +0300
Subject: [PATCH v1] Forbid FOR PORTION OF on views with INSTEAD OF triggers
Priviously an attempt to use these features together caused a crash.
Oversight of commit 8e72d914c528.
Author: Aleksander Alekseev <[email protected]>
Reviewed-by: TODO FIXME
Discussion: TODO FIXME
---
src/backend/parser/analyze.c | 11 +++++++++
src/test/regress/expected/updatable_views.out | 24 +++++++++++++++++++
src/test/regress/sql/updatable_views.sql | 24 +++++++++++++++++++
3 files changed, 59 insertions(+)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index cb4e5019c2f..4fc0ae7199e 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1333,6 +1333,17 @@ transformForPortionOfClause(ParseState *pstate,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("foreign tables don't support FOR PORTION OF")));
+ /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
+ if (targetrel->rd_rel->relkind == RELKIND_VIEW &&
+ targetrel->rd_rel->relhastriggers &&
+ targetrel->trigdesc != NULL &&
+ (isUpdate ? targetrel->trigdesc->trig_update_instead_row
+ : targetrel->trigdesc->trig_delete_instead_row))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF"),
+ parser_errposition(pstate, forPortionOf->location)));
+
result = makeNode(ForPortionOfExpr);
/* Look up the FOR PORTION OF name requested. */
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 8852160718f..4701938bfe2 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -4151,3 +4151,27 @@ select * from base_tab order by a;
drop view base_tab_view;
drop table base_tab;
+-- FOR PORTION OF is not supported on views with INSTEAD OF triggers
+create view uv_fpo_instead_view as select id, valid_at, b from uv_fpo_tab;
+create function uv_fpo_instead_trig() returns trigger language plpgsql as
+$$ begin return null; end $$;
+create trigger uv_fpo_instead_upd_trig
+ instead of update on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+create trigger uv_fpo_instead_del_trig
+ instead of delete on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+update uv_fpo_instead_view
+ for portion of valid_at from '2015-01-01' to '2020-01-01'
+ set b = 99 where id = '[1,1]'; -- error
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+LINE 2: for portion of valid_at from '2015-01-01' to '2020-01-01'
+ ^
+delete from uv_fpo_instead_view
+ for portion of valid_at from '2017-01-01' to '2022-01-01'
+ where id = '[1,1]'; -- error
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+LINE 2: for portion of valid_at from '2017-01-01' to '2022-01-01'
+ ^
+drop view uv_fpo_instead_view;
+drop function uv_fpo_instead_trig();
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index f7646999bd4..30c2db7ad7d 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -2137,3 +2137,27 @@ values (1, 2, default, 5, 4, default, 3), (10, 11, 'C value', 14, 13, 100, 12);
select * from base_tab order by a;
drop view base_tab_view;
drop table base_tab;
+
+-- FOR PORTION OF is not supported on views with INSTEAD OF triggers
+create view uv_fpo_instead_view as select id, valid_at, b from uv_fpo_tab;
+
+create function uv_fpo_instead_trig() returns trigger language plpgsql as
+$$ begin return null; end $$;
+
+create trigger uv_fpo_instead_upd_trig
+ instead of update on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+create trigger uv_fpo_instead_del_trig
+ instead of delete on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+
+update uv_fpo_instead_view
+ for portion of valid_at from '2015-01-01' to '2020-01-01'
+ set b = 99 where id = '[1,1]'; -- error
+
+delete from uv_fpo_instead_view
+ for portion of valid_at from '2017-01-01' to '2022-01-01'
+ where id = '[1,1]'; -- error
+
+drop view uv_fpo_instead_view;
+drop function uv_fpo_instead_trig();
--
2.43.0
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
@ 2026-06-24 16:02 ` Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Paul A Jungwirth @ 2026-06-24 16:02 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; jian he <[email protected]>
On Wed, Jun 24, 2026 at 4:13 AM Aleksander Alekseev
<[email protected]> wrote:
>
> Hi Peter,
>
> Thanks for your attention to this.
>
> > I don't understand the status of this discussion. The original bug
> > report is related to views and INSTEAD OF triggers. But the last few
> > patches that have been posted here appear to be unrelated to that
> > specific issue.
>
> That makes two of us. My humble understanding is that there are
> several related discussions but they are happening elsewhere now. Feel
> free correcting me if this is not the case.
I agree it is confusing. A lot of things were originally reported on
unrelated threads. I've been making separate threads for each
issue+patch and trying to steer discussion there. I'd like to keep
this thread limited to the original issue reported by Aleksander: that
we crash trying to insert temporal leftovers on a view with an INSTEAD
OF trigger.
One thing confusing about this bug is that it was reported twice. The
first time was here:
https://www.postgresql.org/message-id/[email protected]...
But since that was on the original development thread, I thought this
thread was a better place to continue discussion.
The Open Items page should point to the right thread for each issue.
But I see that for this issue, it points to the original bug report,
so you have to click through a lot of messages to wind up in the right
place. I'll update the link to come here.
> Here I propose to focus on a particular crash and the particular
> proposed bugfix, v1-0001. Perhaps it should be rewritten or maybe we
> should reject it in favor of another patch. Both are possible
> outcomes. Right now I'm only interested in fixing a particular crash.
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...
> > 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%40...
Yours,
--
Paul ~{:-)
[email protected]
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
@ 2026-06-25 10:32 ` Aleksander Alekseev <[email protected]>
2026-06-29 07:38 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Aleksander Alekseev @ 2026-06-25 10:32 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Paul A Jungwirth <[email protected]>; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; jian he <[email protected]>
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%40...
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...
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
Attachments:
[text/plain] rebased.txt (6.9K, ../../CAJ7c6TN7dekEHU8Vu4jV5+Zq7h2cVZQgwkYFCrU0jrjT26-ANw@mail.gmail.com/2-rebased.txt)
download | inline diff:
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
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
@ 2026-06-29 07:38 ` Paul A Jungwirth <[email protected]>
2026-07-02 07:25 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Paul A Jungwirth @ 2026-06-29 07:38 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; jian he <[email protected]>
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]
Attachments:
[text/x-patch] v6-0001-Skip-FOR-PORTION-OF-leftovers-after-INSTEAD-OF-tr.patch (7.7K, ../../CA+renyXs5mkPb-Vg=uF2Hj0jP2ufutc1_Eht=eb3+b_WpjiqLg@mail.gmail.com/2-v6-0001-Skip-FOR-PORTION-OF-leftovers-after-INSTEAD-OF-tr.patch)
download | inline diff:
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
[text/x-patch] v6-0002-Fix-INSTEAD-OF-targeting-too-many-rows-with-FOR-P.patch (7.9K, ../../CA+renyXs5mkPb-Vg=uF2Hj0jP2ufutc1_Eht=eb3+b_WpjiqLg@mail.gmail.com/3-v6-0002-Fix-INSTEAD-OF-targeting-too-many-rows-with-FOR-P.patch)
download | inline diff:
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
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-29 07:38 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
@ 2026-07-02 07:25 ` Peter Eisentraut <[email protected]>
2026-07-02 14:29 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Peter Eisentraut @ 2026-07-02 07:25 UTC (permalink / raw)
To: Paul A Jungwirth <[email protected]>; Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers
On 29.06.26 09:38, Paul A Jungwirth wrote:
>> 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.
It seems to me that both FOR PORTION OF and INSTEAD OF triggers are SQL
standard features, so this discussion should refer to what the standard
says, and possibly consider what other implementations do (in addition
to discussing what makes sense). Since that hasn't been done yet, maybe
prohibiting this combination for now, as proposed by Aleksander, would
be best.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-29 07:38 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-02 07:25 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
@ 2026-07-02 14:29 ` Paul A Jungwirth <[email protected]>
2026-07-06 06:34 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Paul A Jungwirth @ 2026-07-02 14:29 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers
On Thu, Jul 2, 2026 at 12:25 AM Peter Eisentraut <[email protected]> wrote:
>
> > 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.
>
> It seems to me that both FOR PORTION OF and INSTEAD OF triggers are SQL
> standard features, so this discussion should refer to what the standard
> says, and possibly consider what other implementations do (in addition
> to discussing what makes sense). Since that hasn't been done yet, maybe
> prohibiting this combination for now, as proposed by Aleksander, would
> be best.
Okay, let's do that. I'll do some research to see what the standard
says and whether any other RDBMS offers guidance.
Yours,
--
Paul ~{:-)
[email protected]
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-29 07:38 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-02 07:25 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-07-02 14:29 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
@ 2026-07-06 06:34 ` Peter Eisentraut <[email protected]>
2026-07-08 01:45 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Peter Eisentraut @ 2026-07-06 06:34 UTC (permalink / raw)
To: Paul A Jungwirth <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers
On 02.07.26 16:29, Paul A Jungwirth wrote:
> On Thu, Jul 2, 2026 at 12:25 AM Peter Eisentraut <[email protected]> wrote:
>>
>>> 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.
>>
>> It seems to me that both FOR PORTION OF and INSTEAD OF triggers are SQL
>> standard features, so this discussion should refer to what the standard
>> says, and possibly consider what other implementations do (in addition
>> to discussing what makes sense). Since that hasn't been done yet, maybe
>> prohibiting this combination for now, as proposed by Aleksander, would
>> be best.
>
> Okay, let's do that. I'll do some research to see what the standard
> says and whether any other RDBMS offers guidance.
Do we have a suitable patch for that?
The patch proposed at the top of this thread checks for the presence of
triggers at parse time, which, I think, again has the problem that the
presence of triggers could change between parse and execution time.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-29 07:38 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-02 07:25 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-07-02 14:29 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-06 06:34 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
@ 2026-07-08 01:45 ` Paul A Jungwirth <[email protected]>
2026-07-08 02:07 ` Re: [PATCH] Fix null pointer dereference in PG19 Tom Lane <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Paul A Jungwirth @ 2026-07-08 01:45 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>
On Sun, Jul 5, 2026 at 11:34 PM Peter Eisentraut <[email protected]> wrote:
>
> >> It seems to me that both FOR PORTION OF and INSTEAD OF triggers are SQL
> >> standard features, so this discussion should refer to what the standard
> >> says, and possibly consider what other implementations do (in addition
> >> to discussing what makes sense). Since that hasn't been done yet, maybe
> >> prohibiting this combination for now, as proposed by Aleksander, would
> >> be best.
> >
> > Okay, let's do that. I'll do some research to see what the standard
> > says and whether any other RDBMS offers guidance.
>
> Do we have a suitable patch for that?
>
> The patch proposed at the top of this thread checks for the presence of
> triggers at parse time, which, I think, again has the problem that the
> presence of triggers could change between parse and execution time.
Here are patches for that.
Actually, even though Tom originally objected to checking at
parse-time, I think it is safe. He said:
On Tue, Apr 21, 2026 at 8:24 AM Tom Lane <[email protected]> wrote:
>
> Checking this at parse time is completely the wrong thing.
> The view could have gained (or lost) triggers by the time
> it's executed.
But INSTEAD OF triggers are selected in the rewriter, which uses the
same relcache snapshot as parse analysis. And a concurrent change
can't sneak in different triggers, because that causes a relcache
invalidation, so we redo the parse & rewrite phases. I can't find any
way to get a crash. I tried running UPDATE FOR PORTION OF in one
session, adding a trigger in another, and then re-running the UPDATE
in the first session. But the plan is re-analyzed and the check fires.
Here is a copy of Aleksander's patch rebased, with a v2 moving the
check to rewrite-time and a v3 moving the check to execution-time. The
v1 and v2 patches have nearly identical behavior, as far as I can
tell. Perhaps v2 is better since it checks at the same time we find
the INSTEAD OF triggers. (Maybe this could avoid a future TOCTOU
problem?) OTOH v1 gives a slightly nicer error message, since it has
the position info.
There is one difference I could find with the execution-time check: a
zero-row update fails a check at parse-time or rewrite-time, but not
at execution-time. But I think failing is better, isn't it? I've added
tests to capture that difference.
So my preference is to apply v2 (squashed), but not v3. Squashing all
three is okay though if you think not failing on zero rows is better.
--
Paul ~{:-)
[email protected]
Attachments:
[text/x-patch] v7-0003-Check-FOR-PORTION-OF-against-INSTEAD-OF-triggers-.patch (5.2K, ../../CA+renyXPEJYOVov3XtzTp3NusGkObc9UeW-Nm78K8_nsWSnnfg@mail.gmail.com/2-v7-0003-Check-FOR-PORTION-OF-against-INSTEAD-OF-triggers-.patch)
download | inline diff:
From 1eb176eeb4c8519c260f4d1430d1673e374da8bf Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
Date: Tue, 7 Jul 2026 17:23:01 -0700
Subject: [PATCH v7 3/3] Check FOR PORTION OF against INSTEAD OF triggers at
execution time
Doing the test at execution time means that zero-row updates don't fail.
Discussion: https://www.postgresql.org/message-id/CAJ7c6TME%2Bix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g%40mail.gmail.com
---
src/backend/executor/nodeModifyTable.c | 13 +++++++++++++
src/backend/rewrite/rewriteHandler.c | 8 --------
src/test/regress/expected/updatable_views.out | 10 ++++------
src/test/regress/sql/updatable_views.sql | 8 ++++----
4 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index c333d7139fa..440f535dd04 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1879,6 +1879,13 @@ ExecDelete(ModifyTableContext *context,
bool dodelete;
Assert(oldtuple != NULL);
+
+ /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
+ if (((ModifyTable *) context->mtstate->ps.plan)->forPortionOf)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF")));
+
dodelete = ExecIRDeleteTriggers(estate, resultRelInfo, oldtuple);
if (!dodelete) /* "do nothing" */
@@ -2772,6 +2779,12 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->trig_update_instead_row)
{
+ /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
+ if (((ModifyTable *) context->mtstate->ps.plan)->forPortionOf)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF")));
+
if (!ExecIRUpdateTriggers(estate, resultRelInfo,
oldtuple, slot))
return NULL; /* "do nothing" */
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 38f54b57eec..e7ae9cce65f 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -4171,14 +4171,6 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length,
*/
rt_entry_relation = relation_open(rt_entry->relid, NoLock);
- /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
- if (parsetree->forPortionOf &&
- rt_entry_relation->rd_rel->relkind == RELKIND_VIEW &&
- view_has_instead_trigger(rt_entry_relation, event, NIL))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF")));
-
/*
* Rewrite the targetlist as needed for the command type.
*/
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 93af96b7ddd..d20676d1ef4 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -4183,15 +4183,13 @@ delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
where id = '[1,1]'; -- error
ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
--- The check does not depend on which rows match, so it errors even when
--- no rows do.
+-- Here the check runs per row in the executor, so a statement that matches
+-- no rows never reaches it and is allowed.
update uv_fpo_instead_view
for portion of valid_at from '2015-01-01' to '2020-01-01'
- set b = 99 where id = '[9,9]'; -- error, even with no matching rows
-ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+ set b = 99 where id = '[9,9]'; -- ok, no rows affected
delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
- where id = '[9,9]'; -- error, even with no matching rows
-ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+ where id = '[9,9]'; -- ok, no rows affected
drop view uv_fpo_instead_view;
drop function uv_fpo_instead_trig();
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index f4a7e0327b0..0a7b3d17ec2 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -2170,15 +2170,15 @@ delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
where id = '[1,1]'; -- error
--- The check does not depend on which rows match, so it errors even when
--- no rows do.
+-- Here the check runs per row in the executor, so a statement that matches
+-- no rows never reaches it and is allowed.
update uv_fpo_instead_view
for portion of valid_at from '2015-01-01' to '2020-01-01'
- set b = 99 where id = '[9,9]'; -- error, even with no matching rows
+ set b = 99 where id = '[9,9]'; -- ok, no rows affected
delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
- where id = '[9,9]'; -- error, even with no matching rows
+ where id = '[9,9]'; -- ok, no rows affected
drop view uv_fpo_instead_view;
drop function uv_fpo_instead_trig();
--
2.47.3
[text/x-patch] v7-0001-Forbid-FOR-PORTION-OF-on-views-with-INSTEAD-OF-tr.patch (5.6K, ../../CA+renyXPEJYOVov3XtzTp3NusGkObc9UeW-Nm78K8_nsWSnnfg@mail.gmail.com/3-v7-0001-Forbid-FOR-PORTION-OF-on-views-with-INSTEAD-OF-tr.patch)
download | inline diff:
From b8765f75ec8084680937c3f0b437e1e19a2cda96 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Tue, 7 Jul 2026 17:20:05 -0700
Subject: [PATCH v7 1/3] Forbid FOR PORTION OF on views with INSTEAD OF
triggers
Previously an attempt to use these features together caused a crash.
Oversight of commit 8e72d914c528.
Author: Aleksander Alekseev <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAJ7c6TME%2Bix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g%40mail.gmail.com
---
src/backend/parser/analyze.c | 11 ++++++
src/test/regress/expected/updatable_views.out | 38 +++++++++++++++++++
src/test/regress/sql/updatable_views.sql | 34 +++++++++++++++++
3 files changed, 83 insertions(+)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 2932d17a107..1e6fbc95964 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1344,6 +1344,17 @@ transformForPortionOfClause(ParseState *pstate,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("WHERE CURRENT OF with FOR PORTION OF is not implemented"));
+ /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
+ if (targetrel->rd_rel->relkind == RELKIND_VIEW &&
+ targetrel->rd_rel->relhastriggers &&
+ targetrel->trigdesc != NULL &&
+ (isUpdate ? targetrel->trigdesc->trig_update_instead_row
+ : targetrel->trigdesc->trig_delete_instead_row))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF"),
+ parser_errposition(pstate, forPortionOf->location)));
+
result = makeNode(ForPortionOfExpr);
/* Look up the FOR PORTION OF name requested. */
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 7b00c742776..acab165ae4c 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -4165,3 +4165,41 @@ select * from base_tab order by a;
drop view base_tab_view;
drop table base_tab;
+-- FOR PORTION OF is not supported on views with INSTEAD OF triggers
+create view uv_fpo_instead_view as select id, valid_at, b from uv_fpo_tab;
+create function uv_fpo_instead_trig() returns trigger language plpgsql as
+$$ begin return null; end $$;
+create trigger uv_fpo_instead_upd_trig
+ instead of update on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+create trigger uv_fpo_instead_del_trig
+ instead of delete on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+update uv_fpo_instead_view
+ for portion of valid_at from '2015-01-01' to '2020-01-01'
+ set b = 99 where id = '[1,1]'; -- error
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+LINE 2: for portion of valid_at from '2015-01-01' to '2020-01-01'
+ ^
+delete from uv_fpo_instead_view
+ for portion of valid_at from '2017-01-01' to '2022-01-01'
+ where id = '[1,1]'; -- error
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+LINE 2: for portion of valid_at from '2017-01-01' to '2022-01-01'
+ ^
+-- The check does not depend on which rows match, so it errors even when
+-- no rows do.
+update uv_fpo_instead_view
+ for portion of valid_at from '2015-01-01' to '2020-01-01'
+ set b = 99 where id = '[9,9]'; -- error, even with no matching rows
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+LINE 2: for portion of valid_at from '2015-01-01' to '2020-01-01'
+ ^
+delete from uv_fpo_instead_view
+ for portion of valid_at from '2017-01-01' to '2022-01-01'
+ where id = '[9,9]'; -- error, even with no matching rows
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+LINE 2: for portion of valid_at from '2017-01-01' to '2022-01-01'
+ ^
+drop view uv_fpo_instead_view;
+drop function uv_fpo_instead_trig();
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index 4a60126ec90..f4a7e0327b0 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -2148,3 +2148,37 @@ values (1, 2, default, 5, 4, default, 3), (10, 11, 'C value', 14, 13, 100, 12);
select * from base_tab order by a;
drop view base_tab_view;
drop table base_tab;
+
+-- FOR PORTION OF is not supported on views with INSTEAD OF triggers
+create view uv_fpo_instead_view as select id, valid_at, b from uv_fpo_tab;
+
+create function uv_fpo_instead_trig() returns trigger language plpgsql as
+$$ begin return null; end $$;
+
+create trigger uv_fpo_instead_upd_trig
+ instead of update on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+create trigger uv_fpo_instead_del_trig
+ instead of delete on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+
+update uv_fpo_instead_view
+ for portion of valid_at from '2015-01-01' to '2020-01-01'
+ set b = 99 where id = '[1,1]'; -- error
+
+delete from uv_fpo_instead_view
+ for portion of valid_at from '2017-01-01' to '2022-01-01'
+ where id = '[1,1]'; -- error
+
+-- The check does not depend on which rows match, so it errors even when
+-- no rows do.
+update uv_fpo_instead_view
+ for portion of valid_at from '2015-01-01' to '2020-01-01'
+ set b = 99 where id = '[9,9]'; -- error, even with no matching rows
+
+delete from uv_fpo_instead_view
+ for portion of valid_at from '2017-01-01' to '2022-01-01'
+ where id = '[9,9]'; -- error, even with no matching rows
+
+drop view uv_fpo_instead_view;
+drop function uv_fpo_instead_trig();
--
2.47.3
[text/x-patch] v7-0002-Check-FOR-PORTION-OF-against-INSTEAD-OF-triggers-.patch (4.2K, ../../CA+renyXPEJYOVov3XtzTp3NusGkObc9UeW-Nm78K8_nsWSnnfg@mail.gmail.com/4-v7-0002-Check-FOR-PORTION-OF-against-INSTEAD-OF-triggers-.patch)
download | inline diff:
From 8defe6de6f69c842ade84b9d6ac7596cf4a21283 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
Date: Tue, 7 Jul 2026 17:21:34 -0700
Subject: [PATCH v7 2/3] Check FOR PORTION OF against INSTEAD OF triggers
during rewriting
Move the INSTEAD OF trigger check to the rewriter. This loses the error
position but gives the check better locality with where we look up the
triggers.
Discussion: https://www.postgresql.org/message-id/CAJ7c6TME%2Bix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g%40mail.gmail.com
---
src/backend/parser/analyze.c | 11 -----------
src/backend/rewrite/rewriteHandler.c | 8 ++++++++
src/test/regress/expected/updatable_views.out | 8 --------
3 files changed, 8 insertions(+), 19 deletions(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 1e6fbc95964..2932d17a107 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1344,17 +1344,6 @@ transformForPortionOfClause(ParseState *pstate,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("WHERE CURRENT OF with FOR PORTION OF is not implemented"));
- /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
- if (targetrel->rd_rel->relkind == RELKIND_VIEW &&
- targetrel->rd_rel->relhastriggers &&
- targetrel->trigdesc != NULL &&
- (isUpdate ? targetrel->trigdesc->trig_update_instead_row
- : targetrel->trigdesc->trig_delete_instead_row))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF"),
- parser_errposition(pstate, forPortionOf->location)));
-
result = makeNode(ForPortionOfExpr);
/* Look up the FOR PORTION OF name requested. */
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index e7ae9cce65f..38f54b57eec 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -4171,6 +4171,14 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length,
*/
rt_entry_relation = relation_open(rt_entry->relid, NoLock);
+ /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
+ if (parsetree->forPortionOf &&
+ rt_entry_relation->rd_rel->relkind == RELKIND_VIEW &&
+ view_has_instead_trigger(rt_entry_relation, event, NIL))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF")));
+
/*
* Rewrite the targetlist as needed for the command type.
*/
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index acab165ae4c..93af96b7ddd 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -4179,27 +4179,19 @@ update uv_fpo_instead_view
for portion of valid_at from '2015-01-01' to '2020-01-01'
set b = 99 where id = '[1,1]'; -- error
ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
-LINE 2: for portion of valid_at from '2015-01-01' to '2020-01-01'
- ^
delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
where id = '[1,1]'; -- error
ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
-LINE 2: for portion of valid_at from '2017-01-01' to '2022-01-01'
- ^
-- The check does not depend on which rows match, so it errors even when
-- no rows do.
update uv_fpo_instead_view
for portion of valid_at from '2015-01-01' to '2020-01-01'
set b = 99 where id = '[9,9]'; -- error, even with no matching rows
ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
-LINE 2: for portion of valid_at from '2015-01-01' to '2020-01-01'
- ^
delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
where id = '[9,9]'; -- error, even with no matching rows
ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
-LINE 2: for portion of valid_at from '2017-01-01' to '2022-01-01'
- ^
drop view uv_fpo_instead_view;
drop function uv_fpo_instead_trig();
--
2.47.3
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-29 07:38 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-02 07:25 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-07-02 14:29 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-06 06:34 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-07-08 01:45 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
@ 2026-07-08 02:07 ` Tom Lane <[email protected]>
2026-07-08 16:48 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Tom Lane @ 2026-07-08 02:07 UTC (permalink / raw)
To: Paul A Jungwirth <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Aleksander Alekseev <[email protected]>; pgsql-hackers
Paul A Jungwirth <[email protected]> writes:
> On Tue, Apr 21, 2026 at 8:24 AM Tom Lane <[email protected]> wrote:
>> Checking this at parse time is completely the wrong thing.
>> The view could have gained (or lost) triggers by the time
>> it's executed.
> But INSTEAD OF triggers are selected in the rewriter, which uses the
> same relcache snapshot as parse analysis. And a concurrent change
> can't sneak in different triggers, because that causes a relcache
> invalidation, so we redo the parse & rewrite phases.
You have forgotten about views and rewrite rules. Those go to disk in
post-parser form, and will be rewritten only at execution sometime
later, *without* a re-parse.
regards, tom lane
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-29 07:38 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-02 07:25 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-07-02 14:29 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-06 06:34 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-07-08 01:45 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-08 02:07 ` Re: [PATCH] Fix null pointer dereference in PG19 Tom Lane <[email protected]>
@ 2026-07-08 16:48 ` Paul A Jungwirth <[email protected]>
2026-07-09 08:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Paul A Jungwirth @ 2026-07-08 16:48 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Aleksander Alekseev <[email protected]>; pgsql-hackers
On Tue, Jul 7, 2026 at 7:07 PM Tom Lane <[email protected]> wrote:
>
> Paul A Jungwirth <[email protected]> writes:
> > On Tue, Apr 21, 2026 at 8:24 AM Tom Lane <[email protected]> wrote:
> >> Checking this at parse time is completely the wrong thing.
> >> The view could have gained (or lost) triggers by the time
> >> it's executed.
>
> > But INSTEAD OF triggers are selected in the rewriter, which uses the
> > same relcache snapshot as parse analysis. And a concurrent change
> > can't sneak in different triggers, because that causes a relcache
> > invalidation, so we redo the parse & rewrite phases.
>
> You have forgotten about views and rewrite rules. Those go to disk in
> post-parser form, and will be rewritten only at execution sometime
> later, *without* a re-parse.
Ah yes, thank you! I should have been able to work that out.
Here is another patch series. No code changes, but I inserted a new
patch with tests showing that parse-time checking crashes, but
rewrite-time and exec-time checking catches the forbidden statement.
So the series is:
v1: parse-time check, tests pass
v2: add tests that crash the server
v3: rewrite-time check: tests pass
v4: exec-time check: tests pass
These are against REL_19_STABLE, not master (but I don't think it
makes a difference).
Yours,
--
Paul ~{:-)
[email protected]
Attachments:
[text/x-patch] v8-0003-Check-FOR-PORTION-OF-against-INSTEAD-OF-triggers-.patch (4.3K, ../../CA+renyV1WUtDSOHpNVv3fhD6JqhzZY_mV-0Nks23MczXLxpE8A@mail.gmail.com/2-v8-0003-Check-FOR-PORTION-OF-against-INSTEAD-OF-triggers-.patch)
download | inline diff:
From d0661fdf84e6d16af5cb59b18360cc08b51f8ab0 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
Date: Tue, 7 Jul 2026 17:21:34 -0700
Subject: [PATCH v8 3/4] Check FOR PORTION OF against INSTEAD OF triggers
during rewriting
Move the INSTEAD OF trigger check to the rewriter. This loses the error
position but gives the check better locality with where we look up the
triggers.
Discussion: https://www.postgresql.org/message-id/CAJ7c6TME%2Bix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g%40mail.gmail.com
---
src/backend/parser/analyze.c | 11 -----------
src/backend/rewrite/rewriteHandler.c | 8 ++++++++
src/test/regress/expected/updatable_views.out | 8 --------
3 files changed, 8 insertions(+), 19 deletions(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 1e6fbc95964..2932d17a107 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1344,17 +1344,6 @@ transformForPortionOfClause(ParseState *pstate,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("WHERE CURRENT OF with FOR PORTION OF is not implemented"));
- /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
- if (targetrel->rd_rel->relkind == RELKIND_VIEW &&
- targetrel->rd_rel->relhastriggers &&
- targetrel->trigdesc != NULL &&
- (isUpdate ? targetrel->trigdesc->trig_update_instead_row
- : targetrel->trigdesc->trig_delete_instead_row))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF"),
- parser_errposition(pstate, forPortionOf->location)));
-
result = makeNode(ForPortionOfExpr);
/* Look up the FOR PORTION OF name requested. */
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index e7ae9cce65f..38f54b57eec 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -4171,6 +4171,14 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length,
*/
rt_entry_relation = relation_open(rt_entry->relid, NoLock);
+ /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
+ if (parsetree->forPortionOf &&
+ rt_entry_relation->rd_rel->relkind == RELKIND_VIEW &&
+ view_has_instead_trigger(rt_entry_relation, event, NIL))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF")));
+
/*
* Rewrite the targetlist as needed for the command type.
*/
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index e6232a5f8c0..9c6bb2219f9 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -4179,28 +4179,20 @@ update uv_fpo_instead_view
for portion of valid_at from '2015-01-01' to '2020-01-01'
set b = 99 where id = '[1,1]'; -- error
ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
-LINE 2: for portion of valid_at from '2015-01-01' to '2020-01-01'
- ^
delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
where id = '[1,1]'; -- error
ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
-LINE 2: for portion of valid_at from '2017-01-01' to '2022-01-01'
- ^
-- The check does not depend on which rows match, so it errors even when
-- no rows do.
update uv_fpo_instead_view
for portion of valid_at from '2015-01-01' to '2020-01-01'
set b = 99 where id = '[9,9]'; -- error, even with no matching rows
ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
-LINE 2: for portion of valid_at from '2015-01-01' to '2020-01-01'
- ^
delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
where id = '[9,9]'; -- error, even with no matching rows
ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
-LINE 2: for portion of valid_at from '2017-01-01' to '2022-01-01'
- ^
drop view uv_fpo_instead_view;
drop function uv_fpo_instead_trig();
-- Forbid INSTEAD OF triggers with FOR PORTION OF even if the FOR PORTION OF
--
2.47.3
[text/x-patch] v8-0001-Forbid-FOR-PORTION-OF-on-views-with-INSTEAD-OF-tr.patch (5.6K, ../../CA+renyV1WUtDSOHpNVv3fhD6JqhzZY_mV-0Nks23MczXLxpE8A@mail.gmail.com/3-v8-0001-Forbid-FOR-PORTION-OF-on-views-with-INSTEAD-OF-tr.patch)
download | inline diff:
From b0f83ff70a1e0ee2c544c3cd31a2c40adbc9e058 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Tue, 7 Jul 2026 17:20:05 -0700
Subject: [PATCH v8 1/4] Forbid FOR PORTION OF on views with INSTEAD OF
triggers
Previously an attempt to use these features together caused a crash.
Oversight of commit 8e72d914c528.
Author: Aleksander Alekseev <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAJ7c6TME%2Bix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g%40mail.gmail.com
---
src/backend/parser/analyze.c | 11 ++++++
src/test/regress/expected/updatable_views.out | 38 +++++++++++++++++++
src/test/regress/sql/updatable_views.sql | 34 +++++++++++++++++
3 files changed, 83 insertions(+)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 2932d17a107..1e6fbc95964 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1344,6 +1344,17 @@ transformForPortionOfClause(ParseState *pstate,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("WHERE CURRENT OF with FOR PORTION OF is not implemented"));
+ /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
+ if (targetrel->rd_rel->relkind == RELKIND_VIEW &&
+ targetrel->rd_rel->relhastriggers &&
+ targetrel->trigdesc != NULL &&
+ (isUpdate ? targetrel->trigdesc->trig_update_instead_row
+ : targetrel->trigdesc->trig_delete_instead_row))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF"),
+ parser_errposition(pstate, forPortionOf->location)));
+
result = makeNode(ForPortionOfExpr);
/* Look up the FOR PORTION OF name requested. */
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 7b00c742776..acab165ae4c 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -4165,3 +4165,41 @@ select * from base_tab order by a;
drop view base_tab_view;
drop table base_tab;
+-- FOR PORTION OF is not supported on views with INSTEAD OF triggers
+create view uv_fpo_instead_view as select id, valid_at, b from uv_fpo_tab;
+create function uv_fpo_instead_trig() returns trigger language plpgsql as
+$$ begin return null; end $$;
+create trigger uv_fpo_instead_upd_trig
+ instead of update on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+create trigger uv_fpo_instead_del_trig
+ instead of delete on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+update uv_fpo_instead_view
+ for portion of valid_at from '2015-01-01' to '2020-01-01'
+ set b = 99 where id = '[1,1]'; -- error
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+LINE 2: for portion of valid_at from '2015-01-01' to '2020-01-01'
+ ^
+delete from uv_fpo_instead_view
+ for portion of valid_at from '2017-01-01' to '2022-01-01'
+ where id = '[1,1]'; -- error
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+LINE 2: for portion of valid_at from '2017-01-01' to '2022-01-01'
+ ^
+-- The check does not depend on which rows match, so it errors even when
+-- no rows do.
+update uv_fpo_instead_view
+ for portion of valid_at from '2015-01-01' to '2020-01-01'
+ set b = 99 where id = '[9,9]'; -- error, even with no matching rows
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+LINE 2: for portion of valid_at from '2015-01-01' to '2020-01-01'
+ ^
+delete from uv_fpo_instead_view
+ for portion of valid_at from '2017-01-01' to '2022-01-01'
+ where id = '[9,9]'; -- error, even with no matching rows
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+LINE 2: for portion of valid_at from '2017-01-01' to '2022-01-01'
+ ^
+drop view uv_fpo_instead_view;
+drop function uv_fpo_instead_trig();
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index 4a60126ec90..f4a7e0327b0 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -2148,3 +2148,37 @@ values (1, 2, default, 5, 4, default, 3), (10, 11, 'C value', 14, 13, 100, 12);
select * from base_tab order by a;
drop view base_tab_view;
drop table base_tab;
+
+-- FOR PORTION OF is not supported on views with INSTEAD OF triggers
+create view uv_fpo_instead_view as select id, valid_at, b from uv_fpo_tab;
+
+create function uv_fpo_instead_trig() returns trigger language plpgsql as
+$$ begin return null; end $$;
+
+create trigger uv_fpo_instead_upd_trig
+ instead of update on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+create trigger uv_fpo_instead_del_trig
+ instead of delete on uv_fpo_instead_view
+ for each row execute function uv_fpo_instead_trig();
+
+update uv_fpo_instead_view
+ for portion of valid_at from '2015-01-01' to '2020-01-01'
+ set b = 99 where id = '[1,1]'; -- error
+
+delete from uv_fpo_instead_view
+ for portion of valid_at from '2017-01-01' to '2022-01-01'
+ where id = '[1,1]'; -- error
+
+-- The check does not depend on which rows match, so it errors even when
+-- no rows do.
+update uv_fpo_instead_view
+ for portion of valid_at from '2015-01-01' to '2020-01-01'
+ set b = 99 where id = '[9,9]'; -- error, even with no matching rows
+
+delete from uv_fpo_instead_view
+ for portion of valid_at from '2017-01-01' to '2022-01-01'
+ where id = '[9,9]'; -- error, even with no matching rows
+
+drop view uv_fpo_instead_view;
+drop function uv_fpo_instead_trig();
--
2.47.3
[text/x-patch] v8-0004-Check-FOR-PORTION-OF-against-INSTEAD-OF-triggers-.patch (5.3K, ../../CA+renyV1WUtDSOHpNVv3fhD6JqhzZY_mV-0Nks23MczXLxpE8A@mail.gmail.com/4-v8-0004-Check-FOR-PORTION-OF-against-INSTEAD-OF-triggers-.patch)
download | inline diff:
From 52aa3063d498ac7380068c1537985e9cfcff0be3 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
Date: Tue, 7 Jul 2026 17:23:01 -0700
Subject: [PATCH v8 4/4] Check FOR PORTION OF against INSTEAD OF triggers at
execution time
Doing the test at execution time means that zero-row updates don't fail.
Discussion: https://www.postgresql.org/message-id/CAJ7c6TME%2Bix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g%40mail.gmail.com
---
src/backend/executor/nodeModifyTable.c | 13 +++++++++++++
src/backend/rewrite/rewriteHandler.c | 8 --------
src/test/regress/expected/updatable_views.out | 10 ++++------
src/test/regress/sql/updatable_views.sql | 8 ++++----
4 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index c333d7139fa..440f535dd04 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1879,6 +1879,13 @@ ExecDelete(ModifyTableContext *context,
bool dodelete;
Assert(oldtuple != NULL);
+
+ /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
+ if (((ModifyTable *) context->mtstate->ps.plan)->forPortionOf)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF")));
+
dodelete = ExecIRDeleteTriggers(estate, resultRelInfo, oldtuple);
if (!dodelete) /* "do nothing" */
@@ -2772,6 +2779,12 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->trig_update_instead_row)
{
+ /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
+ if (((ModifyTable *) context->mtstate->ps.plan)->forPortionOf)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF")));
+
if (!ExecIRUpdateTriggers(estate, resultRelInfo,
oldtuple, slot))
return NULL; /* "do nothing" */
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 38f54b57eec..e7ae9cce65f 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -4171,14 +4171,6 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length,
*/
rt_entry_relation = relation_open(rt_entry->relid, NoLock);
- /* We don't support FOR PORTION OF on views with INSTEAD OF triggers. */
- if (parsetree->forPortionOf &&
- rt_entry_relation->rd_rel->relkind == RELKIND_VIEW &&
- view_has_instead_trigger(rt_entry_relation, event, NIL))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("views with INSTEAD OF triggers do not support FOR PORTION OF")));
-
/*
* Rewrite the targetlist as needed for the command type.
*/
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 9c6bb2219f9..68927e78ce3 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -4183,16 +4183,14 @@ delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
where id = '[1,1]'; -- error
ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
--- The check does not depend on which rows match, so it errors even when
--- no rows do.
+-- Here the check runs per row in the executor, so a statement that matches
+-- no rows never reaches it and is allowed.
update uv_fpo_instead_view
for portion of valid_at from '2015-01-01' to '2020-01-01'
- set b = 99 where id = '[9,9]'; -- error, even with no matching rows
-ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+ set b = 99 where id = '[9,9]'; -- ok, no rows affected
delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
- where id = '[9,9]'; -- error, even with no matching rows
-ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+ where id = '[9,9]'; -- ok, no rows affected
drop view uv_fpo_instead_view;
drop function uv_fpo_instead_trig();
-- Forbid INSTEAD OF triggers with FOR PORTION OF even if the FOR PORTION OF
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index 2ef9aa32f36..b0b607ec335 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -2170,15 +2170,15 @@ delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
where id = '[1,1]'; -- error
--- The check does not depend on which rows match, so it errors even when
--- no rows do.
+-- Here the check runs per row in the executor, so a statement that matches
+-- no rows never reaches it and is allowed.
update uv_fpo_instead_view
for portion of valid_at from '2015-01-01' to '2020-01-01'
- set b = 99 where id = '[9,9]'; -- error, even with no matching rows
+ set b = 99 where id = '[9,9]'; -- ok, no rows affected
delete from uv_fpo_instead_view
for portion of valid_at from '2017-01-01' to '2022-01-01'
- where id = '[9,9]'; -- error, even with no matching rows
+ where id = '[9,9]'; -- ok, no rows affected
drop view uv_fpo_instead_view;
drop function uv_fpo_instead_trig();
--
2.47.3
[text/x-patch] v8-0002-Add-tests-for-FOR-PORTION-OF-reaching-INSTEAD-OF-.patch (5.5K, ../../CA+renyV1WUtDSOHpNVv3fhD6JqhzZY_mV-0Nks23MczXLxpE8A@mail.gmail.com/5-v8-0002-Add-tests-for-FOR-PORTION-OF-reaching-INSTEAD-OF-.patch)
download | inline diff:
From 5d41626a1c2fe5f7cd99e2b973552e9aae109549 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
Date: Wed, 8 Jul 2026 08:43:10 -0700
Subject: [PATCH v8 2/4] Add tests for FOR PORTION OF reaching INSTEAD OF
triggers via stored queries
These tests crash if we forbid FOR PORTION OF with INSTEAD OF triggers
at parse time. So they show why checking at rewrite or later is
necessary.
Anything that stores the parsed query risks a TOCTOU vulnerability. The
tests show two ways: a rewrite RULE and a BEGIN ATOMIC function.
Discussion: https://www.postgresql.org/message-id/1894792.1783476457%40sss.pgh.pa.us
---
src/test/regress/expected/updatable_views.out | 41 +++++++++++++++++++
src/test/regress/sql/updatable_views.sql | 39 ++++++++++++++++++
2 files changed, 80 insertions(+)
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index acab165ae4c..e6232a5f8c0 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -4203,3 +4203,44 @@ LINE 2: for portion of valid_at from '2017-01-01' to '2022-01-01'
^
drop view uv_fpo_instead_view;
drop function uv_fpo_instead_trig();
+-- Forbid INSTEAD OF triggers with FOR PORTION OF even if the FOR PORTION OF
+-- statement is parsed before the trigger exists.
+-- This can happen in at least a couple ways: a rewrite rule or a BEGIN ATOMIC function.
+create function uv_fpo_instead_trig() returns trigger language plpgsql as
+$$ begin return new; end $$;
+-- via a rewrite rule
+create table uv_fpo_rule_tab (id int4range, valid_at tsrange, b float);
+insert into uv_fpo_rule_tab values ('[1,1]', '[2000-01-01, 2030-01-01)', 0);
+create view uv_fpo_rule_view as select id, valid_at, b from uv_fpo_rule_tab;
+create rule uv_fpo_rule as on insert to uv_fpo_rule_tab do also
+ update uv_fpo_rule_view
+ for portion of valid_at from '2010-01-01' to '2020-01-01'
+ set b = 99 where id = '[1,1]';
+create trigger uv_fpo_rule_instead
+ instead of update on uv_fpo_rule_view
+ for each row execute function uv_fpo_instead_trig();
+-- Would crash if we checked at parse-time:
+insert into uv_fpo_rule_tab values ('[2,2]', '[2000-01-01,2030-01-01)', 0);
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+drop table uv_fpo_rule_tab cascade;
+NOTICE: drop cascades to view uv_fpo_rule_view
+-- via a BEGIN ATOMIC function body
+create table uv_fpo_atomic_tab (id int4range, valid_at tsrange, b float);
+insert into uv_fpo_atomic_tab values ('[1,1]', '[2000-01-01, 2030-01-01)', 0);
+create view uv_fpo_atomic_view as select id, valid_at, b from uv_fpo_atomic_tab;
+create function uv_fpo_atomic_fn() returns void language sql begin atomic
+ update uv_fpo_atomic_view
+ for portion of valid_at from '2010-01-01' to '2020-01-01'
+ set b = 99 where id = '[1,1]';
+end;
+create trigger uv_fpo_atomic_instead
+ instead of update on uv_fpo_atomic_view
+ for each row execute function uv_fpo_instead_trig();
+-- Would crash if we checked at parse-time:
+select uv_fpo_atomic_fn();
+ERROR: views with INSTEAD OF triggers do not support FOR PORTION OF
+CONTEXT: SQL function "uv_fpo_atomic_fn" statement 1
+drop function uv_fpo_atomic_fn();
+drop table uv_fpo_atomic_tab cascade;
+NOTICE: drop cascades to view uv_fpo_atomic_view
+drop function uv_fpo_instead_trig();
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index f4a7e0327b0..2ef9aa32f36 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -2182,3 +2182,42 @@ delete from uv_fpo_instead_view
drop view uv_fpo_instead_view;
drop function uv_fpo_instead_trig();
+
+-- Forbid INSTEAD OF triggers with FOR PORTION OF even if the FOR PORTION OF
+-- statement is parsed before the trigger exists.
+-- This can happen in at least a couple ways: a rewrite rule or a BEGIN ATOMIC function.
+create function uv_fpo_instead_trig() returns trigger language plpgsql as
+$$ begin return new; end $$;
+
+-- via a rewrite rule
+create table uv_fpo_rule_tab (id int4range, valid_at tsrange, b float);
+insert into uv_fpo_rule_tab values ('[1,1]', '[2000-01-01, 2030-01-01)', 0);
+create view uv_fpo_rule_view as select id, valid_at, b from uv_fpo_rule_tab;
+create rule uv_fpo_rule as on insert to uv_fpo_rule_tab do also
+ update uv_fpo_rule_view
+ for portion of valid_at from '2010-01-01' to '2020-01-01'
+ set b = 99 where id = '[1,1]';
+create trigger uv_fpo_rule_instead
+ instead of update on uv_fpo_rule_view
+ for each row execute function uv_fpo_instead_trig();
+-- Would crash if we checked at parse-time:
+insert into uv_fpo_rule_tab values ('[2,2]', '[2000-01-01,2030-01-01)', 0);
+drop table uv_fpo_rule_tab cascade;
+
+-- via a BEGIN ATOMIC function body
+create table uv_fpo_atomic_tab (id int4range, valid_at tsrange, b float);
+insert into uv_fpo_atomic_tab values ('[1,1]', '[2000-01-01, 2030-01-01)', 0);
+create view uv_fpo_atomic_view as select id, valid_at, b from uv_fpo_atomic_tab;
+create function uv_fpo_atomic_fn() returns void language sql begin atomic
+ update uv_fpo_atomic_view
+ for portion of valid_at from '2010-01-01' to '2020-01-01'
+ set b = 99 where id = '[1,1]';
+end;
+create trigger uv_fpo_atomic_instead
+ instead of update on uv_fpo_atomic_view
+ for each row execute function uv_fpo_instead_trig();
+-- Would crash if we checked at parse-time:
+select uv_fpo_atomic_fn();
+drop function uv_fpo_atomic_fn();
+drop table uv_fpo_atomic_tab cascade;
+drop function uv_fpo_instead_trig();
--
2.47.3
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-29 07:38 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-02 07:25 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-07-02 14:29 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-06 06:34 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-07-08 01:45 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-08 02:07 ` Re: [PATCH] Fix null pointer dereference in PG19 Tom Lane <[email protected]>
2026-07-08 16:48 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
@ 2026-07-09 08:32 ` Peter Eisentraut <[email protected]>
2026-07-09 15:22 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Peter Eisentraut @ 2026-07-09 08:32 UTC (permalink / raw)
To: Paul A Jungwirth <[email protected]>; Tom Lane <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers
On 08.07.26 18:48, Paul A Jungwirth wrote:
> On Tue, Jul 7, 2026 at 7:07 PM Tom Lane <[email protected]> wrote:
>>
>> Paul A Jungwirth <[email protected]> writes:
>>> On Tue, Apr 21, 2026 at 8:24 AM Tom Lane <[email protected]> wrote:
>>>> Checking this at parse time is completely the wrong thing.
>>>> The view could have gained (or lost) triggers by the time
>>>> it's executed.
>>
>>> But INSTEAD OF triggers are selected in the rewriter, which uses the
>>> same relcache snapshot as parse analysis. And a concurrent change
>>> can't sneak in different triggers, because that causes a relcache
>>> invalidation, so we redo the parse & rewrite phases.
>>
>> You have forgotten about views and rewrite rules. Those go to disk in
>> post-parser form, and will be rewritten only at execution sometime
>> later, *without* a re-parse.
>
> Ah yes, thank you! I should have been able to work that out.
>
> Here is another patch series. No code changes, but I inserted a new
> patch with tests showing that parse-time checking crashes, but
> rewrite-time and exec-time checking catches the forbidden statement.
> So the series is:
>
> v1: parse-time check, tests pass
> v2: add tests that crash the server
> v3: rewrite-time check: tests pass
> v4: exec-time check: tests pass
What do you want to do with these? Here you list them as four different
versions, but the attachments are four incremental patches of the same
version. Do you propose to apply all of them?
It seems we definitely do want the exec-time check. And I can see maybe
the parse-time check as an additional user-friendliness feature. But
maybe we don't need to check three times?
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] Fix null pointer dereference in PG19
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Aleksander Alekseev <[email protected]>
2026-06-29 07:38 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-02 07:25 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-07-02 14:29 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-06 06:34 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-07-08 01:45 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-08 02:07 ` Re: [PATCH] Fix null pointer dereference in PG19 Tom Lane <[email protected]>
2026-07-08 16:48 ` Re: [PATCH] Fix null pointer dereference in PG19 Paul A Jungwirth <[email protected]>
2026-07-09 08:32 ` Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
@ 2026-07-09 15:22 ` Paul A Jungwirth <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Paul A Jungwirth @ 2026-07-09 15:22 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Tom Lane <[email protected]>; Aleksander Alekseev <[email protected]>; pgsql-hackers
On Thu, Jul 9, 2026 at 1:32 AM Peter Eisentraut <[email protected]> wrote:
>
> > Here is another patch series. No code changes, but I inserted a new
> > patch with tests showing that parse-time checking crashes, but
> > rewrite-time and exec-time checking catches the forbidden statement.
> > So the series is:
> >
> > v1: parse-time check, tests pass
> > v2: add tests that crash the server
> > v3: rewrite-time check: tests pass
> > v4: exec-time check: tests pass
>
> What do you want to do with these? Here you list them as four different
> versions, but the attachments are four incremental patches of the same
> version. Do you propose to apply all of them?
>
> It seems we definitely do want the exec-time check. And I can see maybe
> the parse-time check as an additional user-friendliness feature. But
> maybe we don't need to check three times?
I should have called them part{1-4} instead of v{1-4}. My intent is to
squash them, but I thought separating the changes clarified the
problem and how v3/v4 fix it. Either squash v1-3 if you want to check
at rewrite time, or v1-4 if you want to check at exec time.
Each part removes the check from the prior part, so there is only one
check, it's just a question of when we do it.
Checking at v1 is Aleksander's original patch. Then v2 adds tests that
crash the server. v3 moves the check to rewrite time, and the tests
pass. v4 moves the check again to exec time. But v4 is a behavior
change: an UPDATE that touches zero rows no longer fails. I thought
that was weird; that's why I recommended leaving it out (in
https://www.postgresql.org/message-id/CA%2BrenyXPEJYOVov3XtzTp3NusGkObc9UeW-Nm78K8_nsWSnnfg%40mail.g...
before I added the crashing tests).
I like your idea of keeping the parse-time check too. Then we can show
line numbers. Want a patch with that?
If we check at both rewrite-time and exec-time, the exec-time check
will never happen (as far as I can tell). If we want something there,
maybe it should be an Assert or an elog.
I can give you one squashed file if you like. After reading your
email, my preference is to check at parse time, check again at rewrite
time, and Assert at exec time.
Yours,
--
Paul ~{:-)
[email protected]
^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2026-07-09 15:22 UTC | newest]
Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-06-24 08:52 Re: [PATCH] Fix null pointer dereference in PG19 Peter Eisentraut <[email protected]>
2026-06-24 11:12 ` Aleksander Alekseev <[email protected]>
2026-06-24 16:02 ` Paul A Jungwirth <[email protected]>
2026-06-25 10:32 ` Aleksander Alekseev <[email protected]>
2026-06-29 07:38 ` Paul A Jungwirth <[email protected]>
2026-07-02 07:25 ` Peter Eisentraut <[email protected]>
2026-07-02 14:29 ` Paul A Jungwirth <[email protected]>
2026-07-06 06:34 ` Peter Eisentraut <[email protected]>
2026-07-08 01:45 ` Paul A Jungwirth <[email protected]>
2026-07-08 02:07 ` Tom Lane <[email protected]>
2026-07-08 16:48 ` Paul A Jungwirth <[email protected]>
2026-07-09 08:32 ` Peter Eisentraut <[email protected]>
2026-07-09 15:22 ` Paul A Jungwirth <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox