From d555595875cbc4e382c8ecf2d9f43dfefc8971b4 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" 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