From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <1FE6C83B-8CAB-4643-9D97-3F8C93C6B12F@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_5EA9C5ED-CDDB-4480-BA2C-2E2744E20049"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: Bug: Rule actions see wrong values for generated columns (NEW.gen
 reads OLD value)
Date: Tue, 14 Apr 2026 10:49:38 +0800
In-Reply-To: 
 <CAEZATCWYqyJJJcMNEj-LEvCSwog2H_HS61hTROZLKYuaiy5YTg@mail.gmail.com>
Cc: Richard Guo <guofenglinux@gmail.com>,
 SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Peter Eisentraut <peter@eisentraut.org>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
References: 
 <CAHg+QDexGTmCZzx=73gXkY2ZADS6LRhpnU+-8Y_QmrdTS6yUhA@mail.gmail.com>
 <CAMbWs4-SJ6B26ciFu_K_2M1ObDA=GirV87N1BKeAtSRmQGATUA@mail.gmail.com>
 <22B4A33A-99F3-46F5-BE0C-426A9E1D9ABA@gmail.com>
 <CAMbWs48=QrLy8gZprTfCmEtzoaUku54JXvAcnoRSo+OC8vUpwA@mail.gmail.com>
 <CAEZATCWYqyJJJcMNEj-LEvCSwog2H_HS61hTROZLKYuaiy5YTg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/1FE6C83B-8CAB-4643-9D97-3F8C93C6B12F%40gmail.com>
Precedence: bulk


--Apple-Mail=_5EA9C5ED-CDDB-4480-BA2C-2E2744E20049
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On Apr 13, 2026, at 19:03, Dean Rasheed <dean.a.rasheed@gmail.com> =
wrote:
>=20
> On Mon, 13 Apr 2026, 09:20 Richard Guo, <guofenglinux@gmail.com> =
wrote:
> On Mon, Apr 13, 2026 at 4:21=E2=80=AFPM Chao Li =
<li.evan.chao@gmail.com> wrote:
> > I think the issue is that rewriteTargetListIU() removes generated =
columns from the target list, as described by this comment:
>=20
> > Later, when the rule action is rewritten, =
ReplaceVarsFromTargetList() cannot find a target list entry for NEW.gen. =
For UPDATE rules, the missing NEW column is handled with =
REPLACEVARS_CHANGE_VARNO, so it falls back to referencing the original =
target relation row, which gives the old value.
>=20
> I came to the same conclusion.
>=20
> > One possible fix is to build a new target list that adds generated =
columns back when there are rules to fire. I tried the solution locally =
with some quick and dirty code and it seems to fix both stored and =
virtual generated columns for me.
>=20
> I think a simpler fix might be to expand generated column references
> in the NEW relation to their generation expressions before
> ReplaceVarsFromTargetList resolves NEW references, so that the base
> column Vars within the expressions can be correctly resolved.
> Something like attached.
>=20
> - Richard
>=20
> One thing about that approach is that it leads to 2 full rewrites of =
the rule action using ReplaceVarsFromTargetList(). I think that could be =
avoided by using including generated column expressions in the =
targetlist passed to ReplaceVarsFromTargetList() by rewriteRuleAction(). =
I haven't tried it, but I imagine it could reuse some code from =
expand_generated_columns_internal().
>=20
> Regards,
> Dean

I think Dean=E2=80=99s comment aligns with my implementation. My =
implementation just adds generated columns back to target list when =
needed, and pass the new target list to ReplaceVarsFromTargetList.=20

Please see the attached diff for my implementation:

* The diff is based on Richard=E2=80=99s v1 patch.
* As this diff is only to show my implementation rather than proposing a =
patch, so the diff is not well polished, and I didn=E2=80=99t touch the =
change to expand_generated_columns_internal of v1 patch.
* The v1 patch has a typo in generated_virtual.sql where =E2=80=9CSTORED=E2=
=80=9D should be =E2=80=9CVIRTUAL=E2=80=9D, I fixed that as it impacts =
the tests.
* I updated the test cases by adding one more column, so that the =
generated column is built upon two columns.

I am really new to this area, so please don=E2=80=99t be surprised if I =
missed something or made something wrong. I am trying to learn by =
resolving problems.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/


--Apple-Mail=_5EA9C5ED-CDDB-4480-BA2C-2E2744E20049
Content-Disposition: attachment;
	filename=chao_impl.diff
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="chao_impl.diff"
Content-Transfer-Encoding: 7bit

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 0ba9931b2fd..e7cf63cb47a 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -332,6 +332,57 @@ acquireLocksOnSubLinks(Node *node, acquireLocksOnSubLinks_context *context)
 	return expression_tree_walker(node, acquireLocksOnSubLinks, context);
 }
 
+static List *
+add_generated_columns_to_targetlist(List *targetList,
+									CmdType commandType,
+									Relation target_relation,
+									RangeTblEntry *target_rte,
+									int target_rt_index,
+									int result_relation,
+									int nomatch_varno)
+{
+	TupleDesc	tupdesc = RelationGetDescr(target_relation);
+	List	   *result;
+
+	if (!(tupdesc->constr &&
+		  (tupdesc->constr->has_generated_stored ||
+		   tupdesc->constr->has_generated_virtual)))
+		return targetList;
+
+	result = list_copy(targetList);
+
+	for (int i = 0; i < tupdesc->natts; i++)
+	{
+		Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
+		Node	   *expr;
+		TargetEntry *te;
+
+		if (!attr->attgenerated)
+			continue;
+
+		expr = build_generation_expression(target_relation, i + 1);
+		ChangeVarNodes(expr, 1, target_rt_index, 0);
+		expr = ReplaceVarsFromTargetList(expr,
+										 target_rt_index,
+										 0,
+										 target_rte,
+										 targetList,
+										 result_relation,
+										 (commandType == CMD_UPDATE) ?
+										 REPLACEVARS_CHANGE_VARNO :
+										 REPLACEVARS_SUBSTITUTE_NULL,
+										 nomatch_varno,
+										 NULL);
+
+		te = makeTargetEntry((Expr *) expr,
+							 i + 1,
+							 pstrdup(NameStr(attr->attname)),
+							 false);
+		result = lappend(result, te);
+	}
+
+	return result;
+}
 
 /*
  * rewriteRuleAction -
@@ -645,24 +696,16 @@ rewriteRuleAction(Query *parsetree,
 	{
 		RangeTblEntry *new_rte = rt_fetch(new_varno, sub_action->rtable);
 		Relation	new_rel;
+		List	   *new_targetlist;
 
-		/*
-		 * First, expand any generated column references in the NEW relation.
-		 * This must happen before ReplaceVarsFromTargetList, because the
-		 * query's target list won't contain entries for generated columns
-		 * (they are removed by rewriteTargetListIU).  Without this step,
-		 * ReplaceVarsFromTargetList would fall through to
-		 * REPLACEVARS_CHANGE_VARNO (for UPDATE) or
-		 * REPLACEVARS_SUBSTITUTE_NULL (for INSERT), producing wrong results
-		 * when the generated column depends on columns being modified.
-		 */
 		new_rel = relation_open(new_rte->relid, NoLock);
-		sub_action = (Query *)
-			expand_generated_columns_internal((Node *) sub_action,
-											  new_rel, new_varno,
-											  new_rte,
-											  sub_action->resultRelation,
-											  true);
+		new_targetlist = add_generated_columns_to_targetlist(parsetree->targetList,
+															 event,
+															 new_rel,
+															 new_rte,
+															 new_varno,
+															 sub_action->resultRelation,
+															 current_varno);
 		relation_close(new_rel, NoLock);
 
 		sub_action = (Query *)
@@ -670,7 +713,7 @@ rewriteRuleAction(Query *parsetree,
 									  new_varno,
 									  0,
 									  new_rte,
-									  parsetree->targetList,
+									  new_targetlist,
 									  sub_action->resultRelation,
 									  (event == CMD_UPDATE) ?
 									  REPLACEVARS_CHANGE_VARNO :
diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out
index 363bd7ccad5..be421b738bc 100644
--- a/src/test/regress/expected/generated_stored.out
+++ b/src/test/regress/expected/generated_stored.out
@@ -1600,20 +1600,22 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
 
 -- rules
 -- NEW.b in a rule action should reflect the generated column's new value
-CREATE TABLE gtest_rule (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
+CREATE TABLE gtest_rule (a int, c int, b int GENERATED ALWAYS AS (a + c) STORED);
 CREATE TABLE gtest_rule_log (op text, old_b int, new_b int);
 CREATE RULE gtest_rule_upd AS ON UPDATE TO gtest_rule
   DO ALSO INSERT INTO gtest_rule_log VALUES ('UPD', OLD.b, NEW.b);
 CREATE RULE gtest_rule_ins AS ON INSERT TO gtest_rule
   DO ALSO INSERT INTO gtest_rule_log VALUES ('INS', NULL, NEW.b);
-INSERT INTO gtest_rule (a) VALUES (1);
+INSERT INTO gtest_rule (a, c) VALUES (1, 5);
 UPDATE gtest_rule SET a = 10;
+UPDATE gtest_rule SET c = 7;
 SELECT * FROM gtest_rule_log;
  op  | old_b | new_b 
 -----+-------+-------
- INS |       |     2
- UPD |     2 |    20
-(2 rows)
+ INS |       |     6
+ UPD |     6 |    15
+ UPD |    15 |    17
+(3 rows)
 
 DROP TABLE gtest_rule, gtest_rule_log;
 -- sanity check of system catalog
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index 5bbd2f684ce..35b54f98ca1 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1522,20 +1522,22 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
 
 -- rules
 -- NEW.b in a rule action should reflect the generated column's new value
-CREATE TABLE gtest_rule (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
+CREATE TABLE gtest_rule (a int, c int, b int GENERATED ALWAYS AS (a + c) VIRTUAL);
 CREATE TABLE gtest_rule_log (op text, old_b int, new_b int);
 CREATE RULE gtest_rule_upd AS ON UPDATE TO gtest_rule
   DO ALSO INSERT INTO gtest_rule_log VALUES ('UPD', OLD.b, NEW.b);
 CREATE RULE gtest_rule_ins AS ON INSERT TO gtest_rule
   DO ALSO INSERT INTO gtest_rule_log VALUES ('INS', NULL, NEW.b);
-INSERT INTO gtest_rule (a) VALUES (1);
+INSERT INTO gtest_rule (a, c) VALUES (1, 5);
 UPDATE gtest_rule SET a = 10;
+UPDATE gtest_rule SET c = 7;
 SELECT * FROM gtest_rule_log;
  op  | old_b | new_b 
 -----+-------+-------
- INS |       |     2
- UPD |     2 |    20
-(2 rows)
+ INS |       |     6
+ UPD |     6 |    15
+ UPD |    15 |    17
+(3 rows)
 
 DROP TABLE gtest_rule, gtest_rule_log;
 -- sanity check of system catalog
diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql
index 3265a1e88c6..bda1fe760e8 100644
--- a/src/test/regress/sql/generated_stored.sql
+++ b/src/test/regress/sql/generated_stored.sql
@@ -800,14 +800,15 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
 
 -- rules
 -- NEW.b in a rule action should reflect the generated column's new value
-CREATE TABLE gtest_rule (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
+CREATE TABLE gtest_rule (a int, c int, b int GENERATED ALWAYS AS (a + c) STORED);
 CREATE TABLE gtest_rule_log (op text, old_b int, new_b int);
 CREATE RULE gtest_rule_upd AS ON UPDATE TO gtest_rule
   DO ALSO INSERT INTO gtest_rule_log VALUES ('UPD', OLD.b, NEW.b);
 CREATE RULE gtest_rule_ins AS ON INSERT TO gtest_rule
   DO ALSO INSERT INTO gtest_rule_log VALUES ('INS', NULL, NEW.b);
-INSERT INTO gtest_rule (a) VALUES (1);
+INSERT INTO gtest_rule (a, c) VALUES (1, 5);
 UPDATE gtest_rule SET a = 10;
+UPDATE gtest_rule SET c = 7;
 SELECT * FROM gtest_rule_log;
 DROP TABLE gtest_rule, gtest_rule_log;
 
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index cd1cd938975..b7c4c711e61 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -813,14 +813,15 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
 
 -- rules
 -- NEW.b in a rule action should reflect the generated column's new value
-CREATE TABLE gtest_rule (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
+CREATE TABLE gtest_rule (a int, c int, b int GENERATED ALWAYS AS (a + c) VIRTUAL);
 CREATE TABLE gtest_rule_log (op text, old_b int, new_b int);
 CREATE RULE gtest_rule_upd AS ON UPDATE TO gtest_rule
   DO ALSO INSERT INTO gtest_rule_log VALUES ('UPD', OLD.b, NEW.b);
 CREATE RULE gtest_rule_ins AS ON INSERT TO gtest_rule
   DO ALSO INSERT INTO gtest_rule_log VALUES ('INS', NULL, NEW.b);
-INSERT INTO gtest_rule (a) VALUES (1);
+INSERT INTO gtest_rule (a, c) VALUES (1, 5);
 UPDATE gtest_rule SET a = 10;
+UPDATE gtest_rule SET c = 7;
 SELECT * FROM gtest_rule_log;
 DROP TABLE gtest_rule, gtest_rule_log;
 

--Apple-Mail=_5EA9C5ED-CDDB-4480-BA2C-2E2744E20049--