Hi Hackers, Here is a fix for finding D14 from [0].
Suppose a view exposes an application-time column twice, like this: CREATE VIEW v AS SELECT id, valid_at AS a, valid_at AS b, name FROM t; Then if you say `UPDATE v FOR PORTION OF a ... SET b = ...`, you hit a multiple-assignment error, but it complains about "valid_at", when you are trying to use columns named "a" and "b". Also it is a generic multiple-assignment error, whereas with a regular table we can be more specific and blame the FOR PORTION OF. This commit adds a check in the rewriter so that the error message for views matches the message for tables. It also has an errdetail pointing out that "a" and "b" are really the same column. Note that I'm only fixing FOR PORTION OF. Other multiple-assignment errors still refer to the base table's column, not the view columns. Maybe that even suggests this is not a bug in the first place. But I think it is nice to call out FOR PORTION OF (as we do for tables) when we can. [0] https://www.postgresql.org/message-id/CA%2BrenyV6QLOJYmLo3gbsg1Y%2BCrho8NqME1jJXgPbO_NgxfBaKQ%40mail.gmail.com Yours, -- Paul ~{:-) [email protected]
From be0340d0b263c508cee85ebee6960a09e8ae6ad7 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" <[email protected]> Date: Thu, 3 Sep 2026 12:58:34 -0700 Subject: [PATCH v1] Improve error message for multiple assignment via view with FOR PORTION OF UPDATE FOR PORTION OF forbids SETting the application-time column directly. But consider a view that gives that column two aliases, for example: CREATE VIEW v AS SELECT id, valid_at AS a, valid_at AS b, name FROM t; Then our multiple-assignment error refers to "valid_at", not to "a" or "b". It would be clearer to report the view column the user is SETting and indicate that it conflicts with the FOR PORTION OF clause. We can do this in RewriteQuery. The error now matches what an ordinary table would get. Reported-by: Noah Misch <[email protected]> Author: Paul A. Jungwirth <[email protected]> Backpatch-through: 19 --- src/backend/rewrite/rewriteHandler.c | 24 ++++++++++ src/test/regress/expected/for_portion_of.out | 47 ++++++++++++++++++++ src/test/regress/sql/for_portion_of.sql | 42 +++++++++++++++++ 3 files changed, 113 insertions(+) diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 3e43418e996..be2a78c0ace 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -4288,6 +4288,30 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length, foreach(tl, parsetree->forPortionOf->rangeTargetList) { TargetEntry *tle = (TargetEntry *) lfirst(tl); + ListCell *tl2; + + /* + * Detect multiple-assignment early, so that we can + * report the same error as against a regular table. + * If we detect it later, the error message will use the + * column names of the base table, not the view. + */ + foreach(tl2, parsetree->targetList) + { + TargetEntry *prior = (TargetEntry *) lfirst(tl2); + + if (prior->resjunk || prior->resno != tle->resno) + continue; + + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("cannot update column \"%s\" because it is used in FOR PORTION OF", + prior->resname), + strcmp(prior->resname, tle->resname) != 0 ? + errdetail("Columns \"%s\" and \"%s\" are the same column of relation \"%s\".", + prior->resname, tle->resname, + RelationGetRelationName(rt_entry_relation)) : 0)); + } parsetree->targetList = lappend(parsetree->targetList, tle); } diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out index 64789d1777b..ecf133902ca 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -2793,4 +2793,51 @@ SELECT * FROM fpo_rls ORDER BY valid_at; DROP TABLE fpo_rls; DROP ROLE regress_fpo_rls; +-- +-- Assigning the range column through a view that exposes it twice +-- +-- transformUpdateTargetList() compares the names the user wrote, so it does +-- not notice that two view columns are the same base column. The rewriter +-- must still say why the assignment is rejected. +-- +CREATE TABLE fpo_dup ( + id int, + valid_at daterange, + name text +); +INSERT INTO fpo_dup VALUES + (1, daterange('2000-01-01', '2010-01-01'), 'one'); +CREATE VIEW fpo_dup_v AS + SELECT id, valid_at AS a, valid_at AS b, name, name AS c, name AS d + FROM fpo_dup; +UPDATE fpo_dup_v FOR PORTION OF a FROM '2002-01-01' TO '2003-01-01' + SET b = daterange('1990-01-01', '1991-01-01'); +ERROR: cannot update column "b" because it is used in FOR PORTION OF +DETAIL: Columns "b" and "a" are the same column of relation "fpo_dup". +-- Naming the same view column in both places is caught in parse analysis, as +-- it is for a table. +UPDATE fpo_dup_v FOR PORTION OF a FROM '2002-01-01' TO '2003-01-01' + SET a = daterange('1990-01-01', '1991-01-01'); +ERROR: cannot update column "a" because it is used in FOR PORTION OF +LINE 2: SET a = daterange('1990-01-01', '1991-01-01'); + ^ +-- Two view columns aliasing a column that is not the FOR PORTION OF column +-- collide with each other, not with the clause, so they still get the generic +-- error: the new check must not fire too eagerly. +UPDATE fpo_dup_v FOR PORTION OF a FROM '2002-01-01' TO '2003-01-01' + SET c = 'x', d = 'y'; +ERROR: multiple assignments to same column "name" +-- Not assigning the range column through the view still works. +UPDATE fpo_dup_v FOR PORTION OF a FROM '2002-01-01' TO '2003-01-01' + SET name = 'one^1'; +SELECT * FROM fpo_dup ORDER BY valid_at; + id | valid_at | name +----+-------------------------+------- + 1 | [2000-01-01,2002-01-01) | one + 1 | [2002-01-01,2003-01-01) | one^1 + 1 | [2003-01-01,2010-01-01) | one +(3 rows) + +DROP VIEW fpo_dup_v; +DROP TABLE fpo_dup; RESET datestyle; diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql index b61fe10478e..ed9bf74b3c0 100644 --- a/src/test/regress/sql/for_portion_of.sql +++ b/src/test/regress/sql/for_portion_of.sql @@ -1849,4 +1849,46 @@ SELECT * FROM fpo_rls ORDER BY valid_at; DROP TABLE fpo_rls; DROP ROLE regress_fpo_rls; +-- +-- Assigning the range column through a view that exposes it twice +-- +-- transformUpdateTargetList() compares the names the user wrote, so it does +-- not notice that two view columns are the same base column. The rewriter +-- must still say why the assignment is rejected. +-- + +CREATE TABLE fpo_dup ( + id int, + valid_at daterange, + name text +); +INSERT INTO fpo_dup VALUES + (1, daterange('2000-01-01', '2010-01-01'), 'one'); + +CREATE VIEW fpo_dup_v AS + SELECT id, valid_at AS a, valid_at AS b, name, name AS c, name AS d + FROM fpo_dup; + +UPDATE fpo_dup_v FOR PORTION OF a FROM '2002-01-01' TO '2003-01-01' + SET b = daterange('1990-01-01', '1991-01-01'); + +-- Naming the same view column in both places is caught in parse analysis, as +-- it is for a table. +UPDATE fpo_dup_v FOR PORTION OF a FROM '2002-01-01' TO '2003-01-01' + SET a = daterange('1990-01-01', '1991-01-01'); + +-- Two view columns aliasing a column that is not the FOR PORTION OF column +-- collide with each other, not with the clause, so they still get the generic +-- error: the new check must not fire too eagerly. +UPDATE fpo_dup_v FOR PORTION OF a FROM '2002-01-01' TO '2003-01-01' + SET c = 'x', d = 'y'; + +-- Not assigning the range column through the view still works. +UPDATE fpo_dup_v FOR PORTION OF a FROM '2002-01-01' TO '2003-01-01' + SET name = 'one^1'; +SELECT * FROM fpo_dup ORDER BY valid_at; + +DROP VIEW fpo_dup_v; +DROP TABLE fpo_dup; + RESET datestyle; -- 2.47.3
