Chao Li <[email protected]> writes:
>> On Jul 23, 2026, at 21:34, Tom Lane <[email protected]> wrote:
>> I didn't check the code, but it looks like someone thought it'd be a
>> good idea to record the FOR PORTION OF target column by name rather
>> than column number. That'll be an initdb-forcing catalog change, so
>> good thing we found it now.
> If nobody works on this problem tonight, I can try to debug it tomorrow.
Actually, since I'm sitting here at the AI workshop, I thought I'd
try using Claude Code to generate a patch. Here's what it came up
with, after only light steering by me (I told it to factor the patch
this way, rather than making independent v19 and v20 patches).
I would have used a much shorter test case, and some of the comments
seem overly verbose too, but otherwise not bad.
It was Claude's idea to have 2 patches at all: it thought we should
avoid a post-beta2 catversion bump in v19. I might agree with it
except I see catversion was bumped just a few days ago in v19,
so there's no benefit in not doing so again.
regards, tom lane
From ef69b040181c876bc24f4619a25d0e84ccbec0dd Mon Sep 17 00:00:00 2001
From: Tom Lane <[email protected]>
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 <[email protected]>
Discussion: https://postgr.es/m/canwcazyfepj5oi45gi4q9y6lya4_oiaxxunnwe-1ym-i0ff...@mail.gmail.com
Backpatch-through: 19
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
---
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
From f738f351e992ac4d87bbdf11fcce8c61d4dc99d8 Mon Sep 17 00:00:00 2001
From: Tom Lane <[email protected]>
Date: Thu, 23 Jul 2026 14:30:56 +0000
Subject: [PATCH 2/2] Remove the vestigial range_name field of ForPortionOfExpr
The preceding commit stopped relying on the range column name recorded
in ForPortionOfExpr, since a stored copy of the node can outlive the
column name it was parsed from. Nothing reads the field now, so remove
it rather than leave a trap for the next reader.
ForPortionOfState.fp_rangeName goes too; it was only ever copied from
range_name, and was never read at all. The one remaining use of the
recorded name, the resname of the TargetEntry that sets the range
column, can come from the relation's tuple descriptor instead.
This can't be back-patched, as it changes the stored form of rules.
XXX: needs catversion bump
Discussion: https://postgr.es/m/canwcazyfepj5oi45gi4q9y6lya4_oiaxxunnwe-1ym-i0ff...@mail.gmail.com
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
---
src/backend/executor/nodeModifyTable.c | 2 --
src/backend/parser/analyze.c | 3 +--
src/include/nodes/execnodes.h | 1 -
src/include/nodes/primnodes.h | 12 ++++--------
4 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index b9781eb3b95..1dbf0ffff9e 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -5641,7 +5641,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
/* Create state for FOR PORTION OF operation */
fpoState = makeNode(ForPortionOfState);
- fpoState->fp_rangeName = forPortionOf->range_name;
fpoState->fp_rangeType = forPortionOf->rangeType;
fpoState->fp_rangeAttno = forPortionOf->rangeVar->varattno;
fpoState->fp_targetRange = targetRange;
@@ -5928,7 +5927,6 @@ ExecInitForPortionOf(ModifyTableState *mtstate, EState *estate,
leafState = makeNode(ForPortionOfState);
- leafState->fp_rangeName = fpoState->fp_rangeName;
leafState->fp_rangeType = fpoState->fp_rangeType;
leafState->fp_targetRange = fpoState->fp_targetRange;
map = ExecGetChildToRootMap(resultRelInfo);
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 562e4facd74..adc98c1c969 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1596,7 +1596,7 @@ transformForPortionOfClause(ParseState *pstate,
/* Make a TLE to set the range column */
result->rangeTargetList = NIL;
tle = makeTargetEntry((Expr *) rangeTLEExpr, range_attno,
- forPortionOf->range_name, false);
+ pstrdup(NameStr(attr->attname)), false);
result->rangeTargetList = lappend(result->rangeTargetList, tle);
/* Mark the range column as requiring update permissions */
@@ -1606,7 +1606,6 @@ transformForPortionOfClause(ParseState *pstate,
else
result->rangeTargetList = NIL;
- result->range_name = forPortionOf->range_name;
result->location = forPortionOf->location;
result->targetLocation = forPortionOf->target_location;
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index e64fd8c7ea3..e95ac3eda35 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -476,7 +476,6 @@ typedef struct ForPortionOfState
{
NodeTag type;
- char *fp_rangeName; /* the column named in FOR PORTION OF */
Oid fp_rangeType; /* the base type (not domain) of the FOR
* PORTION OF expression */
int fp_rangeAttno; /* the attno of the range column */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index c6be41280c2..dbd35e7777b 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -2427,20 +2427,16 @@ typedef struct OnConflictExpr
* In the executor we'll also build an intersect expression between the
* targeted range and the range column, so that we can update the start/end
* bounds of the UPDATE'd record.
+ *
+ * Note that the range column is identified only by rangeVar. We must not
+ * record its name here, because this node can be stored on disk (in a rule
+ * or a SQL function body) and the column could be renamed afterwards.
*----------
*/
typedef struct ForPortionOfExpr
{
NodeTag type;
Var *rangeVar; /* Range column */
-
- /*
- * 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 */
--
2.53.0
