From ef69b040181c876bc24f4619a25d0e84ccbec0dd Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 23 Jul 2026 14:30:09 +0000 Subject: [PATCH 1/2] Deparse FOR PORTION OF using the range column's current name ruleutils.c printed the column name recorded in ForPortionOfExpr when deparsing FOR PORTION OF. Since that node is stored on disk, in a rule or in a SQL-standard function body, the recorded name goes stale as soon as the column is renamed, and then we deparse a query that no longer parses: CREATE TABLE emp (id int, valid daterange, salary int); CREATE TABLE t (x int); CREATE RULE r AS ON UPDATE TO t DO INSTEAD UPDATE emp FOR PORTION OF valid FROM '2020-01-01' TO '2021-01-01' SET salary = 1; ALTER TABLE emp RENAME COLUMN valid TO validity; after which pg_get_ruledef() still prints "FOR PORTION OF valid", so dump and restore fails. \sf on an equivalent SQL function body was broken the same way. Fix by looking up the column's current name from the catalogs, using the attribute number in ForPortionOfExpr.rangeVar, as we already do for the target columns of an UPDATE. An error message in the planner used the recorded name too, and could report a stale name for the same reason; make it use get_attname(). That leaves range_name unused, but removing it would change the stored form of rules, so in the back branch it stays, with a comment warning against using it. A follow-up patch removes it in master. Reported-by: John Naylor Discussion: https://postgr.es/m/CANWCAZYFEpJ5Oi45gi4q9Y6LYa4_oiAXxuNNWe-1ym-i0fF8Pw@mail.gmail.com Backpatch-through: 19 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/backend/optimizer/plan/planner.c | 4 +- src/backend/utils/adt/ruleutils.c | 25 +++++-- src/include/nodes/primnodes.h | 9 ++- src/test/regress/expected/for_portion_of.out | 69 ++++++++++++++++++++ src/test/regress/sql/for_portion_of.sql | 34 ++++++++++ 5 files changed, 135 insertions(+), 6 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 3225185d16f..a0ff9159ae0 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -871,7 +871,9 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name, ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot use generated column \"%s\" in FOR PORTION OF", - forPortionOf->range_name))); + get_attname(rte->relid, + forPortionOf->rangeVar->varattno, + false)))); } /* diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 1b44b7a78d2..c5ee3478a54 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -527,6 +527,7 @@ static void get_rte_alias(RangeTblEntry *rte, int varno, bool use_as, static void get_column_alias_list(deparse_columns *colinfo, deparse_context *context); static void get_for_portion_of(ForPortionOfExpr *forPortionOf, + RangeTblEntry *rte, deparse_context *context); static void get_from_clause_coldeflist(RangeTblFunction *rtfunc, deparse_columns *colinfo, @@ -7556,7 +7557,7 @@ get_update_query_def(Query *query, deparse_context *context) generate_relation_name(rte->relid, NIL)); /* Print the FOR PORTION OF, if needed */ - get_for_portion_of(query->forPortionOf, context); + get_for_portion_of(query->forPortionOf, rte, context); /* Print the relation alias, if needed */ get_rte_alias(rte, query->resultRelation, false, context); @@ -7763,7 +7764,7 @@ get_delete_query_def(Query *query, deparse_context *context) generate_relation_name(rte->relid, NIL)); /* Print the FOR PORTION OF, if needed */ - get_for_portion_of(query->forPortionOf, context); + get_for_portion_of(query->forPortionOf, rte, context); /* Print the relation alias, if needed */ get_rte_alias(rte, query->resultRelation, false, context); @@ -13474,16 +13475,32 @@ get_rte_alias(RangeTblEntry *rte, int varno, bool use_as, /* * get_for_portion_of - print FOR PORTION OF if needed + * + * rte is the result relation's RTE, which we need to resolve the range + * column's current name. + * * XXX: Newlines would help here, at least when pretty-printing. But then the * alias and SET will be on their own line with a leading space. */ static void -get_for_portion_of(ForPortionOfExpr *forPortionOf, deparse_context *context) +get_for_portion_of(ForPortionOfExpr *forPortionOf, RangeTblEntry *rte, + deparse_context *context) { if (forPortionOf) { + char *range_name; + + /* + * Get the range column's name from the catalogs, rather than trusting + * forPortionOf->range_name, which is the name the column had when + * this query was stored and may since have been changed by ALTER + * TABLE ... RENAME COLUMN. + */ + range_name = get_rte_attribute_name(rte, + forPortionOf->rangeVar->varattno); + appendStringInfo(context->buf, " FOR PORTION OF %s", - quote_identifier(forPortionOf->range_name)); + quote_identifier(range_name)); /* * Try to write it as FROM ... TO ... if we received it that way, diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index cacef7d4151..c6be41280c2 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -2433,7 +2433,14 @@ typedef struct ForPortionOfExpr { NodeTag type; Var *rangeVar; /* Range column */ - char *range_name; /* Range name */ + + /* + * Don't use range_name to identify the range column: since this node can + * be stored on disk, in a rule or a SQL function body, the name recorded + * here can be stale if the column has been renamed since. Use + * rangeVar->varattno and consult the catalogs. + */ + char *range_name; /* Range name (unreliable, see above) */ Node *targetFrom; /* FOR PORTION OF FROM bound, if given */ Node *targetTo; /* FOR PORTION OF TO bound, if given */ Node *targetRange; /* FOR PORTION OF bounds as a range/multirange */ diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out index 0e217f104ef..376d6e1bdf6 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -2255,6 +2255,75 @@ SELECT * FROM fpo_rule ORDER BY f1; (2 rows) DROP TABLE fpo_rule; +-- Deparsing FOR PORTION OF must use the range column's current name, +-- not the name it had when the rule was created. +CREATE TABLE fpo_rename (f1 bigint, f2 int4range); +CREATE TABLE fpo_rename_src (x int); +CREATE RULE fpo_rename_rule1 AS ON UPDATE TO fpo_rename_src + DO INSTEAD UPDATE fpo_rename FOR PORTION OF f2 FROM 3 TO 6 SET f1 = 2; +CREATE RULE fpo_rename_rule2 AS ON DELETE TO fpo_rename_src + DO INSTEAD DELETE FROM fpo_rename FOR PORTION OF f2 (int4range(3, 6)); +ALTER TABLE fpo_rename RENAME COLUMN f2 TO f3; +ALTER TABLE fpo_rename RENAME COLUMN f1 TO f0; +SELECT pg_get_ruledef(oid) FROM pg_rewrite + WHERE rulename LIKE 'fpo_rename_rule%' ORDER BY rulename; + pg_get_ruledef +---------------------------------------------------------------------------------------------------------------- + CREATE RULE fpo_rename_rule1 AS + + ON UPDATE TO public.fpo_rename_src DO INSTEAD UPDATE fpo_rename FOR PORTION OF f3 FROM 3 TO 6 SET f0 = 2; + CREATE RULE fpo_rename_rule2 AS + + ON DELETE TO public.fpo_rename_src DO INSTEAD DELETE FROM fpo_rename FOR PORTION OF f3 (int4range(3, 6)); +(2 rows) + +INSERT INTO fpo_rename VALUES (1, '[1, 11)'); +UPDATE fpo_rename_src SET x = 0; +SELECT * FROM fpo_rename ORDER BY f3; + f0 | f3 +----+-------- + 1 | [1,3) + 2 | [3,6) + 1 | [6,11) +(3 rows) + +DELETE FROM fpo_rename_src; +SELECT * FROM fpo_rename ORDER BY f3; + f0 | f3 +----+-------- + 1 | [1,3) + 1 | [6,11) +(2 rows) + +-- Likewise for a SQL-standard function body, which is also stored parsed. +CREATE FUNCTION fpo_rename_func() RETURNS void LANGUAGE sql + BEGIN ATOMIC + UPDATE fpo_rename FOR PORTION OF f3 FROM 7 TO 9 SET f0 = 4; + END; +ALTER TABLE fpo_rename RENAME COLUMN f3 TO f4; +\sf fpo_rename_func +CREATE OR REPLACE FUNCTION public.fpo_rename_func() + RETURNS void + LANGUAGE sql +BEGIN ATOMIC + UPDATE fpo_rename FOR PORTION OF f4 FROM 7 TO 9 SET f0 = 4; +END +SELECT fpo_rename_func(); + fpo_rename_func +----------------- + +(1 row) + +SELECT * FROM fpo_rename ORDER BY f4; + f0 | f4 +----+-------- + 1 | [1,3) + 1 | [6,7) + 4 | [7,9) + 1 | [9,11) +(4 rows) + +DROP FUNCTION fpo_rename_func; +DROP TABLE fpo_rename_src; +DROP TABLE fpo_rename; -- UPDATE/DELETE FOR PORTION OF on a GENERATED VIRTUAL range column: CREATE TABLE fpo_gen_virtual ( a int, diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql index a8d29a76b22..46e65ce8aea 100644 --- a/src/test/regress/sql/for_portion_of.sql +++ b/src/test/regress/sql/for_portion_of.sql @@ -1495,6 +1495,40 @@ SELECT * FROM fpo_rule ORDER BY f1; DROP TABLE fpo_rule; +-- Deparsing FOR PORTION OF must use the range column's current name, +-- not the name it had when the rule was created. +CREATE TABLE fpo_rename (f1 bigint, f2 int4range); +CREATE TABLE fpo_rename_src (x int); +CREATE RULE fpo_rename_rule1 AS ON UPDATE TO fpo_rename_src + DO INSTEAD UPDATE fpo_rename FOR PORTION OF f2 FROM 3 TO 6 SET f1 = 2; +CREATE RULE fpo_rename_rule2 AS ON DELETE TO fpo_rename_src + DO INSTEAD DELETE FROM fpo_rename FOR PORTION OF f2 (int4range(3, 6)); + +ALTER TABLE fpo_rename RENAME COLUMN f2 TO f3; +ALTER TABLE fpo_rename RENAME COLUMN f1 TO f0; +SELECT pg_get_ruledef(oid) FROM pg_rewrite + WHERE rulename LIKE 'fpo_rename_rule%' ORDER BY rulename; + +INSERT INTO fpo_rename VALUES (1, '[1, 11)'); +UPDATE fpo_rename_src SET x = 0; +SELECT * FROM fpo_rename ORDER BY f3; +DELETE FROM fpo_rename_src; +SELECT * FROM fpo_rename ORDER BY f3; + +-- Likewise for a SQL-standard function body, which is also stored parsed. +CREATE FUNCTION fpo_rename_func() RETURNS void LANGUAGE sql + BEGIN ATOMIC + UPDATE fpo_rename FOR PORTION OF f3 FROM 7 TO 9 SET f0 = 4; + END; +ALTER TABLE fpo_rename RENAME COLUMN f3 TO f4; +\sf fpo_rename_func +SELECT fpo_rename_func(); +SELECT * FROM fpo_rename ORDER BY f4; + +DROP FUNCTION fpo_rename_func; +DROP TABLE fpo_rename_src; +DROP TABLE fpo_rename; + -- UPDATE/DELETE FOR PORTION OF on a GENERATED VIRTUAL range column: CREATE TABLE fpo_gen_virtual ( a int, -- 2.53.0