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]
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
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
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
