public inbox for [email protected]
help / color / mirror / Atom feedBug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
16+ messages / 4 participants
[nested] [flat]
* Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
@ 2026-04-13 01:59 SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-04-13 01:59 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi hackers,
NEW.<generated_coulmn> is resolved to the OLD row's value
for update or NULL for insert cases in a DO ALSO rule action for
generated columns. This bug affects both stored and virtual
generated columns. Reporting here to see if this is a known issue
with generated columns.
Repro:
CREATE TABLE t (id int PRIMARY KEY, a int,
gen int GENERATED ALWAYS AS (a * 2) VIRTUAL);
CREATE TABLE t_log (op text, old_gen int, new_gen int);
CREATE RULE t_log AS ON UPDATE TO t
DO ALSO INSERT INTO t_log VALUES ('UPD', OLD.gen, NEW.gen);
INSERT INTO t (id, a) VALUES (1, 5);
UPDATE t SET a = 100 WHERE id = 1;
postgres=# SELECT * FROM t;
id | a | gen
----+-----+-----
1 | 100 | 200
(1 row)
postgres=# SELECT * FROM t_log;
op | old_gen | new_gen
-----+---------+---------
UPD | 10 | 10
(1 row)
Thanks,
Satya
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
@ 2026-04-13 02:35 ` Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Richard Guo @ 2026-04-13 02:35 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
On Mon, Apr 13, 2026 at 10:59 AM SATYANARAYANA NARLAPURAM
<[email protected]> wrote:
> NEW.<generated_coulmn> is resolved to the OLD row's value
> for update or NULL for insert cases in a DO ALSO rule action for
> generated columns. This bug affects both stored and virtual
> generated columns. Reporting here to see if this is a known issue
> with generated columns.
I didn't find related item in open items. This does not seem to be a
known issue. I think we should fix it anyway.
cc-ing Peter.
- Richard
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
@ 2026-04-13 07:21 ` Chao Li <[email protected]>
2026-04-13 07:27 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 08:20 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
0 siblings, 2 replies; 16+ messages in thread
From: Chao Li @ 2026-04-13 07:21 UTC (permalink / raw)
To: Richard Guo <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
> On Apr 13, 2026, at 10:35, Richard Guo <[email protected]> wrote:
>
> On Mon, Apr 13, 2026 at 10:59 AM SATYANARAYANA NARLAPURAM
> <[email protected]> wrote:
>> NEW.<generated_coulmn> is resolved to the OLD row's value
>> for update or NULL for insert cases in a DO ALSO rule action for
>> generated columns. This bug affects both stored and virtual
>> generated columns. Reporting here to see if this is a known issue
>> with generated columns.
>
> I didn't find related item in open items. This does not seem to be a
> known issue. I think we should fix it anyway.
>
> cc-ing Peter.
>
> - Richard
>
Hi Richard and Satya,
I reproduced the bug following Satya’s procedure and spent some time debugging it.
I think the issue is that rewriteTargetListIU() removes generated columns from the target list, as described by this comment:
```
if (att_tup->attgenerated)
{
/*
* virtual generated column stores a null value; stored generated
* column will be fixed in executor
*/
new_tle = NULL;
}
```
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.
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.
Do either of you plan to propose a patch for this? If so, please go ahead and I can review it. Otherwise, I can propose a patch in a couple of days.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
@ 2026-04-13 07:27 ` SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 08:30 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
1 sibling, 1 reply; 16+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-04-13 07:27 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Richard Guo <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
Hi Chao,
On Mon, Apr 13, 2026 at 12:21 AM Chao Li <[email protected]> wrote:
>
>
> > On Apr 13, 2026, at 10:35, Richard Guo <[email protected]> wrote:
> >
> > On Mon, Apr 13, 2026 at 10:59 AM SATYANARAYANA NARLAPURAM
> > <[email protected]> wrote:
> >> NEW.<generated_coulmn> is resolved to the OLD row's value
> >> for update or NULL for insert cases in a DO ALSO rule action for
> >> generated columns. This bug affects both stored and virtual
> >> generated columns. Reporting here to see if this is a known issue
> >> with generated columns.
> >
> > I didn't find related item in open items. This does not seem to be a
> > known issue. I think we should fix it anyway.
> >
> > cc-ing Peter.
> >
> > - Richard
> >
>
> Hi Richard and Satya,
>
> I reproduced the bug following Satya’s procedure and spent some time
> debugging it.
>
> I think the issue is that rewriteTargetListIU() removes generated columns
> from the target list, as described by this comment:
> ```
> if (att_tup->attgenerated)
> {
> /*
> * virtual generated column stores a null value; stored
> generated
> * column will be fixed in executor
> */
> new_tle = NULL;
> }
> ```
>
> 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.
>
> 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.
>
> Do either of you plan to propose a patch for this? If so, please go ahead
> and I can review it. Otherwise, I can propose a patch in a couple of days.
I have a patch with me, let me post it shortly.
Thanks,
Satya
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 07:27 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
@ 2026-04-13 08:30 ` Richard Guo <[email protected]>
2026-04-13 08:42 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Richard Guo @ 2026-04-13 08:30 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <[email protected]>; +Cc: Chao Li <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
On Mon, Apr 13, 2026 at 4:27 PM SATYANARAYANA NARLAPURAM
<[email protected]> wrote:
> On Mon, Apr 13, 2026 at 12:21 AM Chao Li <[email protected]> wrote:
>> Do either of you plan to propose a patch for this? If so, please go ahead and I can review it. Otherwise, I can propose a patch in a couple of days.
> I have a patch with me, let me post it shortly.
Please feel free to share your patches. I've been looking into this
as well, but I'd be happy to see independent takes on it.
- Richard
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 07:27 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 08:30 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
@ 2026-04-13 08:42 ` SATYANARAYANA NARLAPURAM <[email protected]>
0 siblings, 0 replies; 16+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-04-13 08:42 UTC (permalink / raw)
To: Richard Guo <[email protected]>; +Cc: Chao Li <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
Hi Richard,
On Mon, Apr 13, 2026 at 1:30 AM Richard Guo <[email protected]> wrote:
> On Mon, Apr 13, 2026 at 4:27 PM SATYANARAYANA NARLAPURAM
> <[email protected]> wrote:
> > On Mon, Apr 13, 2026 at 12:21 AM Chao Li <[email protected]> wrote:
> >> Do either of you plan to propose a patch for this? If so, please go
> ahead and I can review it. Otherwise, I can propose a patch in a couple of
> days.
>
> > I have a patch with me, let me post it shortly.
>
> Please feel free to share your patches. I've been looking into this
> as well, but I'd be happy to see independent takes on it.
>
I reviewed your patch, it looks good. I ran the tests you added and a few
additional tests and they are passing. Please continue with your patch.
Maybe a test like "WHERE NEW.b > threshold" to confirm the expansion
happens before qual evaluation would be good.
Thanks,
Satya
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
@ 2026-04-13 08:20 ` Richard Guo <[email protected]>
2026-04-13 11:03 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
1 sibling, 1 reply; 16+ messages in thread
From: Richard Guo @ 2026-04-13 08:20 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
On Mon, Apr 13, 2026 at 4:21 PM Chao Li <[email protected]> wrote:
> I think the issue is that rewriteTargetListIU() removes generated columns from the target list, as described by this comment:
> 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.
I came to the same conclusion.
> 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.
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.
- Richard
Attachments:
[application/octet-stream] v1-0001-Fix-incorrect-NEW-references-to-generated-columns.patch (11.0K, 2-v1-0001-Fix-incorrect-NEW-references-to-generated-columns.patch)
download | inline diff:
From 7cc29faf775b2368d4381856d6f853df01ab01c2 Mon Sep 17 00:00:00 2001
From: Richard Guo <[email protected]>
Date: Mon, 13 Apr 2026 16:54:23 +0900
Subject: [PATCH v1] Fix incorrect NEW references to generated columns in rule
actions
---
src/backend/rewrite/rewriteHandler.c | 75 ++++++++++++++-----
.../regress/expected/generated_stored.out | 18 +++++
.../regress/expected/generated_virtual.out | 18 +++++
src/test/regress/sql/generated_stored.sql | 13 ++++
src/test/regress/sql/generated_virtual.sql | 13 ++++
5 files changed, 118 insertions(+), 19 deletions(-)
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 021c73f1b67..0ba9931b2fd 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -98,7 +98,8 @@ static List *matchLocks(CmdType event, Relation relation,
static Query *fireRIRrules(Query *parsetree, List *activeRIRs);
static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist);
static Node *expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
- RangeTblEntry *rte, int result_relation);
+ RangeTblEntry *rte, int result_relation,
+ bool include_stored);
/*
@@ -642,11 +643,33 @@ rewriteRuleAction(Query *parsetree,
if ((event == CMD_INSERT || event == CMD_UPDATE) &&
sub_action->commandType != CMD_UTILITY)
{
+ RangeTblEntry *new_rte = rt_fetch(new_varno, sub_action->rtable);
+ Relation new_rel;
+
+ /*
+ * 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);
+ relation_close(new_rel, NoLock);
+
sub_action = (Query *)
ReplaceVarsFromTargetList((Node *) sub_action,
new_varno,
0,
- rt_fetch(new_varno, sub_action->rtable),
+ new_rte,
parsetree->targetList,
sub_action->resultRelation,
(event == CMD_UPDATE) ?
@@ -4530,28 +4553,38 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length,
/*
- * Expand virtual generated columns
+ * Expand generated columns
*
- * If the table contains virtual generated columns, build a target list
- * containing the expanded expressions and use ReplaceVarsFromTargetList() to
- * do the replacements.
+ * If the table contains generated columns, build a target list containing the
+ * expanded expressions and use ReplaceVarsFromTargetList() to do the
+ * replacements.
*
* Vars matching rt_index at the current query level are replaced by the
- * virtual generated column expressions from rel, if there are any.
+ * generated column expressions from rel, if there are any.
*
* The caller must also provide rte, the RTE describing the target relation,
* in order to handle any whole-row Vars referencing the target, and
* result_relation, the index of the result relation, if this is part of an
* INSERT/UPDATE/DELETE/MERGE query.
+ *
+ * If include_stored is false, only virtual generated columns are expanded.
+ * If include_stored is true, both stored and virtual generated columns are
+ * expanded. The latter is needed when expanding NEW references in rule
+ * actions, because rewriteTargetListIU removes both types from the target
+ * list, and both need to be expanded before ReplaceVarsFromTargetList can
+ * correctly resolve them.
*/
static Node *
expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
- RangeTblEntry *rte, int result_relation)
+ RangeTblEntry *rte, int result_relation,
+ bool include_stored)
{
TupleDesc tupdesc;
tupdesc = RelationGetDescr(rel);
- if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
+ if (tupdesc->constr &&
+ (tupdesc->constr->has_generated_virtual ||
+ (include_stored && tupdesc->constr->has_generated_stored)))
{
List *tlist = NIL;
@@ -4559,7 +4592,8 @@ expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
{
Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
- if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL ||
+ (include_stored && attr->attgenerated == ATTRIBUTE_GENERATED_STORED))
{
Node *defexpr;
TargetEntry *te;
@@ -4572,12 +4606,11 @@ expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
}
}
- Assert(list_length(tlist) > 0);
-
- node = ReplaceVarsFromTargetList(node, rt_index, 0, rte, tlist,
- result_relation,
- REPLACEVARS_CHANGE_VARNO, rt_index,
- NULL);
+ if (tlist != NIL)
+ node = ReplaceVarsFromTargetList(node, rt_index, 0, rte, tlist,
+ result_relation,
+ REPLACEVARS_CHANGE_VARNO, rt_index,
+ NULL);
}
return node;
@@ -4604,7 +4637,8 @@ expand_generated_columns_in_expr(Node *node, Relation rel, int rt_index)
rte->rtekind = RTE_RELATION;
rte->relid = RelationGetRelid(rel);
- node = expand_generated_columns_internal(node, rel, rt_index, rte, 0);
+ node = expand_generated_columns_internal(node, rel, rt_index, rte, 0,
+ false);
}
return node;
@@ -4623,8 +4657,11 @@ build_generation_expression(Relation rel, int attrno)
Node *defexpr;
Oid attcollid;
- Assert(rd_att->constr && rd_att->constr->has_generated_virtual);
- Assert(att_tup->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL);
+ Assert(rd_att->constr &&
+ (rd_att->constr->has_generated_virtual ||
+ rd_att->constr->has_generated_stored));
+ Assert(att_tup->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL ||
+ att_tup->attgenerated == ATTRIBUTE_GENERATED_STORED);
defexpr = build_column_default(rel, attrno);
if (defexpr == NULL)
diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out
index 43cddeac373..363bd7ccad5 100644
--- a/src/test/regress/expected/generated_stored.out
+++ b/src/test/regress/expected/generated_stored.out
@@ -1598,6 +1598,24 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
c | integer | | |
x | integer | | | generated always as (b * 2) stored
+-- 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_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);
+UPDATE gtest_rule SET a = 10;
+SELECT * FROM gtest_rule_log;
+ op | old_b | new_b
+-----+-------+-------
+ INS | | 2
+ UPD | 2 | 20
+(2 rows)
+
+DROP TABLE gtest_rule, gtest_rule_log;
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
attrelid | attname | attgenerated
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index 234061fa1f7..5bbd2f684ce 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1520,6 +1520,24 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
c | integer | | |
x | integer | | | generated always as (b * 2)
+-- 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_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);
+UPDATE gtest_rule SET a = 10;
+SELECT * FROM gtest_rule_log;
+ op | old_b | new_b
+-----+-------+-------
+ INS | | 2
+ UPD | 2 | 20
+(2 rows)
+
+DROP TABLE gtest_rule, gtest_rule_log;
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
attrelid | attname | attgenerated
diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql
index 280021d79b7..3265a1e88c6 100644
--- a/src/test/regress/sql/generated_stored.sql
+++ b/src/test/regress/sql/generated_stored.sql
@@ -798,6 +798,19 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
\d gtest28*
+-- 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_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);
+UPDATE gtest_rule SET a = 10;
+SELECT * FROM gtest_rule_log;
+DROP TABLE gtest_rule, gtest_rule_log;
+
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index 4d9ad3c5dca..cd1cd938975 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -811,6 +811,19 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
\d gtest28*
+-- 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_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);
+UPDATE gtest_rule SET a = 10;
+SELECT * FROM gtest_rule_log;
+DROP TABLE gtest_rule, gtest_rule_log;
+
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 08:20 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
@ 2026-04-13 11:03 ` Dean Rasheed <[email protected]>
2026-04-14 02:49 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-14 03:27 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
0 siblings, 2 replies; 16+ messages in thread
From: Dean Rasheed @ 2026-04-13 11:03 UTC (permalink / raw)
To: Richard Guo <[email protected]>; +Cc: Chao Li <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
On Mon, 13 Apr 2026, 09:20 Richard Guo, <[email protected]> wrote:
> On Mon, Apr 13, 2026 at 4:21 PM Chao Li <[email protected]> wrote:
> > I think the issue is that rewriteTargetListIU() removes generated
> columns from the target list, as described by this comment:
>
> > 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.
>
> I came to the same conclusion.
>
> > 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.
>
> 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.
>
> - Richard
>
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().
Regards,
Dean
>
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 08:20 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 11:03 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
@ 2026-04-14 02:49 ` Chao Li <[email protected]>
1 sibling, 0 replies; 16+ messages in thread
From: Chao Li @ 2026-04-14 02:49 UTC (permalink / raw)
To: Dean Rasheed <[email protected]>; +Cc: Richard Guo <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
> On Apr 13, 2026, at 19:03, Dean Rasheed <[email protected]> wrote:
>
> On Mon, 13 Apr 2026, 09:20 Richard Guo, <[email protected]> wrote:
> On Mon, Apr 13, 2026 at 4:21 PM Chao Li <[email protected]> wrote:
> > I think the issue is that rewriteTargetListIU() removes generated columns from the target list, as described by this comment:
>
> > 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.
>
> I came to the same conclusion.
>
> > 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.
>
> 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.
>
> - Richard
>
> 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().
>
> Regards,
> Dean
I think Dean’s 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.
Please see the attached diff for my implementation:
* The diff is based on Richard’s 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’t touch the change to expand_generated_columns_internal of v1 patch.
* The v1 patch has a typo in generated_virtual.sql where “STORED” should be “VIRTUAL”, 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’t 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/
Attachments:
[application/octet-stream] chao_impl.diff (8.0K, 2-chao_impl.diff)
download | inline diff:
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;
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 08:20 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 11:03 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
@ 2026-04-14 03:27 ` Richard Guo <[email protected]>
2026-04-14 06:28 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
1 sibling, 1 reply; 16+ messages in thread
From: Richard Guo @ 2026-04-14 03:27 UTC (permalink / raw)
To: Dean Rasheed <[email protected]>; +Cc: Chao Li <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
On Mon, Apr 13, 2026 at 8:04 PM Dean Rasheed <[email protected]> wrote:
> On Mon, 13 Apr 2026, 09:20 Richard Guo, <[email protected]> wrote:
>> 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.
> 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().
I considered it, but I'm afraid it doesn't work directly, because
replace_rte_variables_mutator returns the callback's replacement node
without recursing into its children.
Take Satya's repro as an example. If we add the generation expression
for gen to the UPDATE's targetlist, the list would be:
TargetEntry 1: resno=2, expr=Const(100) -- a = 100
TargetEntry 2: resno=3, expr=Var(3, 2) * 2 -- gen = NEW.a * 2
When ReplaceVarsFromTargetList processes Var(3, 3) (NEW.gen) in the
rule action, it finds resno=3 and substitutes Var(3, 2) * 2. However,
replace_rte_variables_mutator returns this replacement directly to its
caller; it does not recurse into the replacement's children to look
for further matching Vars. So the inner Var(3, 2) (NEW.a) is never
processed, even though resno=2 with Const(100) is right there in the
targetlist. The Var(3, 2) survives into the planner and would cause
problems.
It could be made to work by pre-resolving the generation expressions'
base column Vars before adding them to the UPDATE's targetlist. For
each generated column, we'd call ReplaceVarsFromTargetList on the
generation expression to resolve its base column Vars, then add the
fully resolved expression to the targetlist. But this seems to add
code complexity. And I'm not sure about the performance difference
between these two approaches. I expect that rule action trees are
typically small.
- Richard
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 08:20 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 11:03 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
2026-04-14 03:27 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
@ 2026-04-14 06:28 ` Chao Li <[email protected]>
2026-04-17 10:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Chao Li @ 2026-04-14 06:28 UTC (permalink / raw)
To: Richard Guo <[email protected]>; +Cc: Dean Rasheed <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
> On Apr 14, 2026, at 11:27, Richard Guo <[email protected]> wrote:
>
> On Mon, Apr 13, 2026 at 8:04 PM Dean Rasheed <[email protected]> wrote:
>> On Mon, 13 Apr 2026, 09:20 Richard Guo, <[email protected]> wrote:
>>> 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.
>
>> 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().
>
> I considered it, but I'm afraid it doesn't work directly, because
> replace_rte_variables_mutator returns the callback's replacement node
> without recursing into its children.
>
> Take Satya's repro as an example. If we add the generation expression
> for gen to the UPDATE's targetlist, the list would be:
>
> TargetEntry 1: resno=2, expr=Const(100) -- a = 100
> TargetEntry 2: resno=3, expr=Var(3, 2) * 2 -- gen = NEW.a * 2
>
> When ReplaceVarsFromTargetList processes Var(3, 3) (NEW.gen) in the
> rule action, it finds resno=3 and substitutes Var(3, 2) * 2. However,
> replace_rte_variables_mutator returns this replacement directly to its
> caller; it does not recurse into the replacement's children to look
> for further matching Vars. So the inner Var(3, 2) (NEW.a) is never
> processed, even though resno=2 with Const(100) is right there in the
> targetlist. The Var(3, 2) survives into the planner and would cause
> problems.
>
> It could be made to work by pre-resolving the generation expressions'
> base column Vars before adding them to the UPDATE's targetlist. For
> each generated column, we'd call ReplaceVarsFromTargetList on the
> generation expression to resolve its base column Vars, then add the
> fully resolved expression to the targetlist. But this seems to add
> code complexity. And I'm not sure about the performance difference
> between these two approaches. I expect that rule action trees are
> typically small.
>
> - Richard
My implementation has pre-resolved the generation expressions, that’s why all tests passed. But I agree my change is heavier as I had to add a new static helper function.
If we think rule actions are usually small enough that the extra full-tree pass would not be an issue, then v1 may be preferable for simplicity.
My only comment on v1 is the typo in generated_virtual.sql where “STORED” should be “VIRTUAL”.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 08:20 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 11:03 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
2026-04-14 03:27 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-14 06:28 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
@ 2026-04-17 10:35 ` Dean Rasheed <[email protected]>
2026-04-20 05:25 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Dean Rasheed @ 2026-04-17 10:35 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Richard Guo <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
On Tue, 14 Apr 2026 at 07:29, Chao Li <[email protected]> wrote:
>
> > On Apr 14, 2026, at 11:27, Richard Guo <[email protected]> wrote:
> >
> > It could be made to work by pre-resolving the generation expressions'
> > base column Vars before adding them to the UPDATE's targetlist. For
> > each generated column, we'd call ReplaceVarsFromTargetList on the
> > generation expression to resolve its base column Vars, then add the
> > fully resolved expression to the targetlist. But this seems to add
> > code complexity. And I'm not sure about the performance difference
> > between these two approaches. I expect that rule action trees are
> > typically small.
> >
> My implementation has pre-resolved the generation expressions, that’s why all tests passed. But I agree my change is heavier as I had to add a new static helper function.
>
> If we think rule actions are usually small enough that the extra full-tree pass would not be an issue, then v1 may be preferable for simplicity.
>
> My only comment on v1 is the typo in generated_virtual.sql where “STORED” should be “VIRTUAL”.
>
I don't quite buy the argument that the rule action is typically
small. We have no idea how big it might be. Note that, at that point
in the code, sub_action is the combination of both the original query
and the rule action.
I do accept though that rules are not widely used, and that it's not
worth optimising too much, if it means a lot of extra complexity.
However, IMO, it is slightly simpler and neater to put the expanded
generated columns in the replacement list used by
ReplaceVarsFromTargetList() on sub_action.
In the attached v2 patch, I've done that by refactoring
expand_generated_columns_internal(), renaming it to
get_generated_columns(), and making it just return the list of
generated column expressions, rather than doing the rewrite -- I never
particularly liked the separation of concerns between
expand_generated_columns_internal() and
expand_generated_columns_in_expr(), especially after the rest of the
code expanding virtual generated columns was moved out of the
rewriter, so that expand_generated_columns_in_expr() became the only
caller of expand_generated_columns_internal(). Doing this simplifies
the function, since it's no longer necessary to pass it node, rte, and
result_relation.
With that change, all rewriteRuleAction() needs to do is get the
generated columns, rewrite any new.attribute references in them, and
then use that list plus the original target list as the replacement
list when rewriting sub_action.
It is a slightly bigger patch overall, but it feels a little neater
and more logical to me, but I accept that that's a subjective thing.
Note: I included a minor comment update needed for
build_generation_expression(), and the test fix noted above.
Regards,
Dean
Attachments:
[text/x-patch] v2-0001-Fix-incorrect-NEW-references-to-generated-columns.patch (11.9K, 2-v2-0001-Fix-incorrect-NEW-references-to-generated-columns.patch)
download | inline diff:
From 2e36ee327a7971610ac395418c9e22bf8145850d Mon Sep 17 00:00:00 2001
From: Dean Rasheed <[email protected]>
Date: Fri, 17 Apr 2026 10:30:14 +0100
Subject: [PATCH v2] Fix incorrect NEW references to generated columns in rule
actions.
---
src/backend/rewrite/rewriteHandler.c | 100 ++++++++++++------
.../regress/expected/generated_stored.out | 18 ++++
.../regress/expected/generated_virtual.out | 18 ++++
src/test/regress/sql/generated_stored.sql | 12 +++
src/test/regress/sql/generated_virtual.sql | 12 +++
5 files changed, 125 insertions(+), 35 deletions(-)
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 021c73f1b67..cd950fd4a07 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -97,8 +97,7 @@ static List *matchLocks(CmdType event, Relation relation,
int varno, Query *parsetree, bool *hasUpdate);
static Query *fireRIRrules(Query *parsetree, List *activeRIRs);
static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist);
-static Node *expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
- RangeTblEntry *rte, int result_relation);
+static List *get_generated_columns(Relation rel, int rt_index, bool include_stored);
/*
@@ -642,12 +641,46 @@ rewriteRuleAction(Query *parsetree,
if ((event == CMD_INSERT || event == CMD_UPDATE) &&
sub_action->commandType != CMD_UTILITY)
{
+ RangeTblEntry *new_rte = rt_fetch(new_varno, sub_action->rtable);
+ Relation new_rel;
+ List *gen_cols;
+
+ /*
+ * The target list does not contain entries for generated columns
+ * (they are removed by rewriteTargetListIU), so we must build entries
+ * for them here, so that new.gen_col can be rewritten correctly.
+ */
+ new_rel = relation_open(new_rte->relid, NoLock);
+ gen_cols = get_generated_columns(new_rel, new_varno, true);
+ relation_close(new_rel, NoLock);
+
+ /*
+ * The generated column expressions typically refer to new.attribute,
+ * so they must be rewritten before they can be used as replacements.
+ */
+ gen_cols = (List *)
+ ReplaceVarsFromTargetList((Node *) gen_cols,
+ new_varno,
+ 0,
+ new_rte,
+ parsetree->targetList,
+ sub_action->resultRelation,
+ (event == CMD_UPDATE) ?
+ REPLACEVARS_CHANGE_VARNO :
+ REPLACEVARS_SUBSTITUTE_NULL,
+ current_varno,
+ NULL);
+
+ /*
+ * Now rewrite new.attribute in sub_action, using both the target list
+ * and the rewritten generated column expressions.
+ */
sub_action = (Query *)
ReplaceVarsFromTargetList((Node *) sub_action,
new_varno,
0,
- rt_fetch(new_varno, sub_action->rtable),
- parsetree->targetList,
+ new_rte,
+ list_concat(gen_cols, parsetree->targetList),
sub_action->resultRelation,
(event == CMD_UPDATE) ?
REPLACEVARS_CHANGE_VARNO :
@@ -4530,36 +4563,31 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length,
/*
- * Expand virtual generated columns
- *
- * If the table contains virtual generated columns, build a target list
- * containing the expanded expressions and use ReplaceVarsFromTargetList() to
- * do the replacements.
+ * Get a table's generated columns
*
- * Vars matching rt_index at the current query level are replaced by the
- * virtual generated column expressions from rel, if there are any.
+ * If include_stored is true, both stored and virtual generated columns are
+ * returned. Otherwise, only virtual generated columns are returned.
*
- * The caller must also provide rte, the RTE describing the target relation,
- * in order to handle any whole-row Vars referencing the target, and
- * result_relation, the index of the result relation, if this is part of an
- * INSERT/UPDATE/DELETE/MERGE query.
+ * Returns a list of TargetEntry, one for each generated column, containing
+ * the attribute numbers and generation expressions.
*/
-static Node *
-expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
- RangeTblEntry *rte, int result_relation)
+static List *
+get_generated_columns(Relation rel, int rt_index, bool include_stored)
{
+ List *gen_cols = NIL;
TupleDesc tupdesc;
tupdesc = RelationGetDescr(rel);
- if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
+ if (tupdesc->constr &&
+ (tupdesc->constr->has_generated_virtual ||
+ (include_stored && tupdesc->constr->has_generated_stored)))
{
- List *tlist = NIL;
-
for (int i = 0; i < tupdesc->natts; i++)
{
Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
- if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL ||
+ (include_stored && attr->attgenerated == ATTRIBUTE_GENERATED_STORED))
{
Node *defexpr;
TargetEntry *te;
@@ -4568,19 +4596,12 @@ expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
ChangeVarNodes(defexpr, 1, rt_index, 0);
te = makeTargetEntry((Expr *) defexpr, i + 1, 0, false);
- tlist = lappend(tlist, te);
+ gen_cols = lappend(gen_cols, te);
}
}
-
- Assert(list_length(tlist) > 0);
-
- node = ReplaceVarsFromTargetList(node, rt_index, 0, rte, tlist,
- result_relation,
- REPLACEVARS_CHANGE_VARNO, rt_index,
- NULL);
}
- return node;
+ return gen_cols;
}
/*
@@ -4597,6 +4618,7 @@ expand_generated_columns_in_expr(Node *node, Relation rel, int rt_index)
if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
{
RangeTblEntry *rte;
+ List *vcols;
rte = makeNode(RangeTblEntry);
/* eref needs to be set, but the actual name doesn't matter */
@@ -4604,14 +4626,19 @@ expand_generated_columns_in_expr(Node *node, Relation rel, int rt_index)
rte->rtekind = RTE_RELATION;
rte->relid = RelationGetRelid(rel);
- node = expand_generated_columns_internal(node, rel, rt_index, rte, 0);
+ vcols = get_generated_columns(rel, rt_index, false);
+
+ if (vcols)
+ node = ReplaceVarsFromTargetList(node, rt_index, 0, rte, vcols, 0,
+ REPLACEVARS_CHANGE_VARNO, rt_index,
+ NULL);
}
return node;
}
/*
- * Build the generation expression for the virtual generated column.
+ * Build the generation expression for a generated column.
*
* Error out if there is no generation expression found for the given column.
*/
@@ -4623,8 +4650,11 @@ build_generation_expression(Relation rel, int attrno)
Node *defexpr;
Oid attcollid;
- Assert(rd_att->constr && rd_att->constr->has_generated_virtual);
- Assert(att_tup->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL);
+ Assert(rd_att->constr &&
+ (rd_att->constr->has_generated_virtual ||
+ rd_att->constr->has_generated_stored));
+ Assert(att_tup->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL ||
+ att_tup->attgenerated == ATTRIBUTE_GENERATED_STORED);
defexpr = build_column_default(rel, attrno);
if (defexpr == NULL)
diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out
index 4d329c60994..43535561a98 100644
--- a/src/test/regress/expected/generated_stored.out
+++ b/src/test/regress/expected/generated_stored.out
@@ -1604,6 +1604,24 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
c | integer | | |
x | integer | | | generated always as (b * 2) stored
+-- rule actions referring to generated columns:
+-- 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_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);
+UPDATE gtest_rule SET a = 10;
+SELECT * FROM gtest_rule_log;
+ op | old_b | new_b
+-----+-------+-------
+ INS | | 2
+ UPD | 2 | 20
+(2 rows)
+
+DROP TABLE gtest_rule, gtest_rule_log;
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
attrelid | attname | attgenerated
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index fc41c480d40..cd482d6ee03 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1526,6 +1526,24 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
c | integer | | |
x | integer | | | generated always as (b * 2)
+-- rule actions referring to generated columns:
+-- 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) 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);
+UPDATE gtest_rule SET a = 10;
+SELECT * FROM gtest_rule_log;
+ op | old_b | new_b
+-----+-------+-------
+ INS | | 2
+ UPD | 2 | 20
+(2 rows)
+
+DROP TABLE gtest_rule, gtest_rule_log;
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
attrelid | attname | attgenerated
diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql
index 1064839dcd2..795c7e4cd66 100644
--- a/src/test/regress/sql/generated_stored.sql
+++ b/src/test/regress/sql/generated_stored.sql
@@ -801,6 +801,18 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
\d gtest28*
+-- rule actions referring to generated columns:
+-- 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_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);
+UPDATE gtest_rule SET a = 10;
+SELECT * FROM gtest_rule_log;
+DROP TABLE gtest_rule, gtest_rule_log;
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index 9b32413e3a9..37b5af2497d 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -814,6 +814,18 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
\d gtest28*
+-- rule actions referring to generated columns:
+-- 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) 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);
+UPDATE gtest_rule SET a = 10;
+SELECT * FROM gtest_rule_log;
+DROP TABLE gtest_rule, gtest_rule_log;
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
--
2.43.0
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 08:20 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 11:03 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
2026-04-14 03:27 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-14 06:28 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-17 10:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
@ 2026-04-20 05:25 ` Richard Guo <[email protected]>
2026-04-20 07:29 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Richard Guo @ 2026-04-20 05:25 UTC (permalink / raw)
To: Dean Rasheed <[email protected]>; +Cc: Chao Li <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
On Fri, Apr 17, 2026 at 7:35 PM Dean Rasheed <[email protected]> wrote:
> I don't quite buy the argument that the rule action is typically
> small. We have no idea how big it might be. Note that, at that point
> in the code, sub_action is the combination of both the original query
> and the rule action.
>
> I do accept though that rules are not widely used, and that it's not
> worth optimising too much, if it means a lot of extra complexity.
> However, IMO, it is slightly simpler and neater to put the expanded
> generated columns in the replacement list used by
> ReplaceVarsFromTargetList() on sub_action.
Fair point.
> In the attached v2 patch, I've done that by refactoring
> expand_generated_columns_internal(), renaming it to
> get_generated_columns(), and making it just return the list of
> generated column expressions, rather than doing the rewrite -- I never
> particularly liked the separation of concerns between
> expand_generated_columns_internal() and
> expand_generated_columns_in_expr(), especially after the rest of the
> code expanding virtual generated columns was moved out of the
> rewriter, so that expand_generated_columns_in_expr() became the only
> caller of expand_generated_columns_internal(). Doing this simplifies
> the function, since it's no longer necessary to pass it node, rte, and
> result_relation.
>
> With that change, all rewriteRuleAction() needs to do is get the
> generated columns, rewrite any new.attribute references in them, and
> then use that list plus the original target list as the replacement
> list when rewriting sub_action.
Yeah, this is a better approach. The change looks good to me.
A nitpick: For the comment "The generated column expressions typically
refer to new.attribute ...", maybe we can remove "typically", as
generation expressions always refer to columns of the same relation.
- Richard
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 08:20 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 11:03 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
2026-04-14 03:27 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-14 06:28 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-17 10:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
2026-04-20 05:25 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
@ 2026-04-20 07:29 ` Richard Guo <[email protected]>
2026-04-20 08:42 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Richard Guo @ 2026-04-20 07:29 UTC (permalink / raw)
To: Dean Rasheed <[email protected]>; +Cc: Chao Li <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
On Mon, Apr 20, 2026 at 2:25 PM Richard Guo <[email protected]> wrote:
> Yeah, this is a better approach. The change looks good to me.
>
> A nitpick: For the comment "The generated column expressions typically
> refer to new.attribute ...", maybe we can remove "typically", as
> generation expressions always refer to columns of the same relation.
I noticed a couple of issues after a further look.
1. The ReplaceVarsFromTargetList call on "gen_cols" fails to handle
hasSubLinks. This can cause error if targetList contains SubLinks:
update t set a = (select max(a) from t);
ERROR: replace_rte_variables inserted a SubLink, but has noplace to record it
2. The same bug fixed in this patch also exists in rule quals:
create table t (a int, b int generated always as (a *2));
insert into t values (1);
create rule rule_qual as on update to t where new.b > 100
do instead nothing;
update t set a = 100;
select * from t;
a | b
-----+-----
100 | 200
(1 row)
I think we should apply the same fix to CopyAndAddInvertedQual.
Attached v3 fixes them.
- Richard
Attachments:
[application/octet-stream] v3-0001-Fix-incorrect-NEW-references-to-generated-columns.patch (15.6K, 2-v3-0001-Fix-incorrect-NEW-references-to-generated-columns.patch)
download | inline diff:
From c095a378f82ec0bd570eb29feb4e3ca59907bc88 Mon Sep 17 00:00:00 2001
From: Richard Guo <[email protected]>
Date: Mon, 20 Apr 2026 15:35:54 +0900
Subject: [PATCH v3] Fix incorrect NEW references to generated columns in rule
rewriting
---
src/backend/rewrite/rewriteHandler.c | 145 +++++++++++++-----
.../regress/expected/generated_stored.out | 34 ++++
.../regress/expected/generated_virtual.out | 34 ++++
src/test/regress/sql/generated_stored.sql | 23 +++
src/test/regress/sql/generated_virtual.sql | 23 +++
5 files changed, 221 insertions(+), 38 deletions(-)
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 021c73f1b67..77b2c9bc622 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -97,8 +97,7 @@ static List *matchLocks(CmdType event, Relation relation,
int varno, Query *parsetree, bool *hasUpdate);
static Query *fireRIRrules(Query *parsetree, List *activeRIRs);
static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist);
-static Node *expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
- RangeTblEntry *rte, int result_relation);
+static List *get_generated_columns(Relation rel, int rt_index, bool include_stored);
/*
@@ -642,12 +641,46 @@ rewriteRuleAction(Query *parsetree,
if ((event == CMD_INSERT || event == CMD_UPDATE) &&
sub_action->commandType != CMD_UTILITY)
{
+ RangeTblEntry *new_rte = rt_fetch(new_varno, sub_action->rtable);
+ Relation new_rel;
+ List *gen_cols;
+
+ /*
+ * The target list does not contain entries for generated columns
+ * (they are removed by rewriteTargetListIU), so we must build entries
+ * for them here, so that new.gen_col can be rewritten correctly.
+ */
+ new_rel = relation_open(new_rte->relid, NoLock);
+ gen_cols = get_generated_columns(new_rel, new_varno, true);
+ relation_close(new_rel, NoLock);
+
+ /*
+ * The generated column expressions refer to new.attribute, so they
+ * must be rewritten before they can be used as replacements.
+ */
+ gen_cols = (List *)
+ ReplaceVarsFromTargetList((Node *) gen_cols,
+ new_varno,
+ 0,
+ new_rte,
+ parsetree->targetList,
+ sub_action->resultRelation,
+ (event == CMD_UPDATE) ?
+ REPLACEVARS_CHANGE_VARNO :
+ REPLACEVARS_SUBSTITUTE_NULL,
+ current_varno,
+ &sub_action->hasSubLinks);
+
+ /*
+ * Now rewrite new.attribute in sub_action, using both the target list
+ * and the rewritten generated column expressions.
+ */
sub_action = (Query *)
ReplaceVarsFromTargetList((Node *) sub_action,
new_varno,
0,
- rt_fetch(new_varno, sub_action->rtable),
- parsetree->targetList,
+ new_rte,
+ list_concat(gen_cols, parsetree->targetList),
sub_action->resultRelation,
(event == CMD_UPDATE) ?
REPLACEVARS_CHANGE_VARNO :
@@ -2368,18 +2401,50 @@ CopyAndAddInvertedQual(Query *parsetree,
ChangeVarNodes(new_qual, PRS2_OLD_VARNO, rt_index, 0);
/* Fix references to NEW */
if (event == CMD_INSERT || event == CMD_UPDATE)
+ {
+ RangeTblEntry *rte = rt_fetch(rt_index, parsetree->rtable);
+ Relation rel;
+ List *gen_cols;
+
+ /*
+ * As in rewriteRuleAction, build entries for generated columns so
+ * that new.gen_col in the rule qualification can be rewritten
+ * correctly.
+ */
+ rel = relation_open(rte->relid, NoLock);
+ gen_cols = get_generated_columns(rel, PRS2_NEW_VARNO, true);
+ relation_close(rel, NoLock);
+
+ /*
+ * The generated column expressions refer to new.attribute, so they
+ * must be rewritten before they can be used as replacements.
+ */
+ gen_cols = (List *)
+ ReplaceVarsFromTargetList((Node *) gen_cols,
+ PRS2_NEW_VARNO,
+ 0,
+ rte,
+ parsetree->targetList,
+ parsetree->resultRelation,
+ (event == CMD_UPDATE) ?
+ REPLACEVARS_CHANGE_VARNO :
+ REPLACEVARS_SUBSTITUTE_NULL,
+ rt_index,
+ &parsetree->hasSubLinks);
+
new_qual = ReplaceVarsFromTargetList(new_qual,
PRS2_NEW_VARNO,
0,
- rt_fetch(rt_index,
- parsetree->rtable),
- parsetree->targetList,
+ rte,
+ list_concat(gen_cols,
+ parsetree->targetList),
parsetree->resultRelation,
(event == CMD_UPDATE) ?
REPLACEVARS_CHANGE_VARNO :
REPLACEVARS_SUBSTITUTE_NULL,
rt_index,
&parsetree->hasSubLinks);
+ }
/* And attach the fixed qual */
AddInvertedQual(parsetree, new_qual);
@@ -4530,36 +4595,31 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length,
/*
- * Expand virtual generated columns
- *
- * If the table contains virtual generated columns, build a target list
- * containing the expanded expressions and use ReplaceVarsFromTargetList() to
- * do the replacements.
+ * Get a table's generated columns
*
- * Vars matching rt_index at the current query level are replaced by the
- * virtual generated column expressions from rel, if there are any.
+ * If include_stored is true, both stored and virtual generated columns are
+ * returned. Otherwise, only virtual generated columns are returned.
*
- * The caller must also provide rte, the RTE describing the target relation,
- * in order to handle any whole-row Vars referencing the target, and
- * result_relation, the index of the result relation, if this is part of an
- * INSERT/UPDATE/DELETE/MERGE query.
+ * Returns a list of TargetEntry, one for each generated column, containing
+ * the attribute numbers and generation expressions.
*/
-static Node *
-expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
- RangeTblEntry *rte, int result_relation)
+static List *
+get_generated_columns(Relation rel, int rt_index, bool include_stored)
{
+ List *gen_cols = NIL;
TupleDesc tupdesc;
tupdesc = RelationGetDescr(rel);
- if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
+ if (tupdesc->constr &&
+ (tupdesc->constr->has_generated_virtual ||
+ (include_stored && tupdesc->constr->has_generated_stored)))
{
- List *tlist = NIL;
-
for (int i = 0; i < tupdesc->natts; i++)
{
Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
- if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL ||
+ (include_stored && attr->attgenerated == ATTRIBUTE_GENERATED_STORED))
{
Node *defexpr;
TargetEntry *te;
@@ -4568,19 +4628,12 @@ expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
ChangeVarNodes(defexpr, 1, rt_index, 0);
te = makeTargetEntry((Expr *) defexpr, i + 1, 0, false);
- tlist = lappend(tlist, te);
+ gen_cols = lappend(gen_cols, te);
}
}
-
- Assert(list_length(tlist) > 0);
-
- node = ReplaceVarsFromTargetList(node, rt_index, 0, rte, tlist,
- result_relation,
- REPLACEVARS_CHANGE_VARNO, rt_index,
- NULL);
}
- return node;
+ return gen_cols;
}
/*
@@ -4597,6 +4650,7 @@ expand_generated_columns_in_expr(Node *node, Relation rel, int rt_index)
if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
{
RangeTblEntry *rte;
+ List *vcols;
rte = makeNode(RangeTblEntry);
/* eref needs to be set, but the actual name doesn't matter */
@@ -4604,14 +4658,26 @@ expand_generated_columns_in_expr(Node *node, Relation rel, int rt_index)
rte->rtekind = RTE_RELATION;
rte->relid = RelationGetRelid(rel);
- node = expand_generated_columns_internal(node, rel, rt_index, rte, 0);
+ vcols = get_generated_columns(rel, rt_index, false);
+
+ if (vcols)
+ {
+ /*
+ * Passing NULL for outer_hasSubLinks is safe because generation
+ * expressions cannot contain SubLinks, so the replacement cannot
+ * introduce any.
+ */
+ node = ReplaceVarsFromTargetList(node, rt_index, 0, rte, vcols, 0,
+ REPLACEVARS_CHANGE_VARNO, rt_index,
+ NULL);
+ }
}
return node;
}
/*
- * Build the generation expression for the virtual generated column.
+ * Build the generation expression for a generated column.
*
* Error out if there is no generation expression found for the given column.
*/
@@ -4623,8 +4689,11 @@ build_generation_expression(Relation rel, int attrno)
Node *defexpr;
Oid attcollid;
- Assert(rd_att->constr && rd_att->constr->has_generated_virtual);
- Assert(att_tup->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL);
+ Assert(rd_att->constr &&
+ (rd_att->constr->has_generated_virtual ||
+ rd_att->constr->has_generated_stored));
+ Assert(att_tup->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL ||
+ att_tup->attgenerated == ATTRIBUTE_GENERATED_STORED);
defexpr = build_column_default(rel, attrno);
if (defexpr == NULL)
diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out
index 4d329c60994..7866ae0ebbe 100644
--- a/src/test/regress/expected/generated_stored.out
+++ b/src/test/regress/expected/generated_stored.out
@@ -1604,6 +1604,40 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
c | integer | | |
x | integer | | | generated always as (b * 2) stored
+-- rule actions referring to generated columns:
+-- 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_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);
+UPDATE gtest_rule SET a = 10;
+UPDATE gtest_rule SET a = (SELECT max(b) FROM gtest_rule);
+SELECT * FROM gtest_rule_log;
+ op | old_b | new_b
+-----+-------+-------
+ INS | | 2
+ UPD | 2 | 20
+ UPD | 20 | 40
+(3 rows)
+
+DROP RULE gtest_rule_upd ON gtest_rule;
+DROP RULE gtest_rule_ins ON gtest_rule;
+DROP TABLE gtest_rule_log;
+-- rule quals referring to generated columns:
+-- NEW.b in the rule qual should reflect the generated column's new value
+CREATE RULE gtest_rule_qual AS ON UPDATE TO gtest_rule WHERE NEW.b > 100
+ DO INSTEAD NOTHING;
+UPDATE gtest_rule SET a = 100;
+SELECT * FROM gtest_rule;
+ a | b
+----+----
+ 20 | 40
+(1 row)
+
+DROP TABLE gtest_rule;
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
attrelid | attname | attgenerated
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index fc41c480d40..0d9c76c4e77 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1526,6 +1526,40 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
c | integer | | |
x | integer | | | generated always as (b * 2)
+-- rule actions referring to generated columns:
+-- 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) 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);
+UPDATE gtest_rule SET a = 10;
+UPDATE gtest_rule SET a = (SELECT max(b) FROM gtest_rule);
+SELECT * FROM gtest_rule_log;
+ op | old_b | new_b
+-----+-------+-------
+ INS | | 2
+ UPD | 2 | 20
+ UPD | 20 | 40
+(3 rows)
+
+DROP RULE gtest_rule_upd ON gtest_rule;
+DROP RULE gtest_rule_ins ON gtest_rule;
+DROP TABLE gtest_rule_log;
+-- rule quals referring to generated columns:
+-- NEW.b in the rule qual should reflect the generated column's new value
+CREATE RULE gtest_rule_qual AS ON UPDATE TO gtest_rule WHERE NEW.b > 100
+ DO INSTEAD NOTHING;
+UPDATE gtest_rule SET a = 100;
+SELECT * FROM gtest_rule;
+ a | b
+----+----
+ 20 | 40
+(1 row)
+
+DROP TABLE gtest_rule;
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
attrelid | attname | attgenerated
diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql
index 1064839dcd2..6746cd4632b 100644
--- a/src/test/regress/sql/generated_stored.sql
+++ b/src/test/regress/sql/generated_stored.sql
@@ -801,6 +801,29 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
\d gtest28*
+-- rule actions referring to generated columns:
+-- 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_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);
+UPDATE gtest_rule SET a = 10;
+UPDATE gtest_rule SET a = (SELECT max(b) FROM gtest_rule);
+SELECT * FROM gtest_rule_log;
+DROP RULE gtest_rule_upd ON gtest_rule;
+DROP RULE gtest_rule_ins ON gtest_rule;
+DROP TABLE gtest_rule_log;
+
+-- rule quals referring to generated columns:
+-- NEW.b in the rule qual should reflect the generated column's new value
+CREATE RULE gtest_rule_qual AS ON UPDATE TO gtest_rule WHERE NEW.b > 100
+ DO INSTEAD NOTHING;
+UPDATE gtest_rule SET a = 100;
+SELECT * FROM gtest_rule;
+DROP TABLE gtest_rule;
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index 9b32413e3a9..1bac51ea731 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -814,6 +814,29 @@ CREATE TABLE gtest28b (LIKE gtest28a INCLUDING GENERATED);
\d gtest28*
+-- rule actions referring to generated columns:
+-- 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) 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);
+UPDATE gtest_rule SET a = 10;
+UPDATE gtest_rule SET a = (SELECT max(b) FROM gtest_rule);
+SELECT * FROM gtest_rule_log;
+DROP RULE gtest_rule_upd ON gtest_rule;
+DROP RULE gtest_rule_ins ON gtest_rule;
+DROP TABLE gtest_rule_log;
+
+-- rule quals referring to generated columns:
+-- NEW.b in the rule qual should reflect the generated column's new value
+CREATE RULE gtest_rule_qual AS ON UPDATE TO gtest_rule WHERE NEW.b > 100
+ DO INSTEAD NOTHING;
+UPDATE gtest_rule SET a = 100;
+SELECT * FROM gtest_rule;
+DROP TABLE gtest_rule;
-- sanity check of system catalog
SELECT attrelid, attname, attgenerated FROM pg_attribute WHERE attgenerated NOT IN ('', 's', 'v');
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 08:20 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 11:03 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
2026-04-14 03:27 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-14 06:28 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-17 10:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
2026-04-20 05:25 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-20 07:29 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
@ 2026-04-20 08:42 ` Dean Rasheed <[email protected]>
2026-04-21 06:05 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Dean Rasheed @ 2026-04-20 08:42 UTC (permalink / raw)
To: Richard Guo <[email protected]>; +Cc: Chao Li <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
On Mon, 20 Apr 2026 at 08:29, Richard Guo <[email protected]> wrote:
>
> I noticed a couple of issues after a further look.
>
> 1. The ReplaceVarsFromTargetList call on "gen_cols" fails to handle
> hasSubLinks. This can cause error if targetList contains SubLinks:
>
> update t set a = (select max(a) from t);
> ERROR: replace_rte_variables inserted a SubLink, but has noplace to record it
>
Doh, yes, careless copy-and-pasting. The updates look good.
> 2. The same bug fixed in this patch also exists in rule quals:
>
> I think we should apply the same fix to CopyAndAddInvertedQual.
>
Agreed.
> Attached v3 fixes them.
>
LGTM.
Regards,
Dean
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 07:21 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-13 08:20 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-13 11:03 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
2026-04-14 03:27 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-14 06:28 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Chao Li <[email protected]>
2026-04-17 10:35 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
2026-04-20 05:25 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-20 07:29 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Richard Guo <[email protected]>
2026-04-20 08:42 ` Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) Dean Rasheed <[email protected]>
@ 2026-04-21 06:05 ` Richard Guo <[email protected]>
0 siblings, 0 replies; 16+ messages in thread
From: Richard Guo @ 2026-04-21 06:05 UTC (permalink / raw)
To: Dean Rasheed <[email protected]>; +Cc: Chao Li <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>; Peter Eisentraut <[email protected]>
On Mon, Apr 20, 2026 at 5:42 PM Dean Rasheed <[email protected]> wrote:
> On Mon, 20 Apr 2026 at 08:29, Richard Guo <[email protected]> wrote:
> > Attached v3 fixes them.
> LGTM.
Thanks! I've pushed and back-patched to v14 (all supported branches).
Virtual generated columns and some helper functions such as
build_generation_expression were added in v18, so the back-patches in
pre-v18 branches differ slightly from the v18+ patches.
- Richard
^ permalink raw reply [nested|flat] 16+ messages in thread
end of thread, other threads:[~2026-04-21 06:05 UTC | newest]
Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-13 01:59 Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 02:35 ` Richard Guo <[email protected]>
2026-04-13 07:21 ` Chao Li <[email protected]>
2026-04-13 07:27 ` SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 08:30 ` Richard Guo <[email protected]>
2026-04-13 08:42 ` SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-13 08:20 ` Richard Guo <[email protected]>
2026-04-13 11:03 ` Dean Rasheed <[email protected]>
2026-04-14 02:49 ` Chao Li <[email protected]>
2026-04-14 03:27 ` Richard Guo <[email protected]>
2026-04-14 06:28 ` Chao Li <[email protected]>
2026-04-17 10:35 ` Dean Rasheed <[email protected]>
2026-04-20 05:25 ` Richard Guo <[email protected]>
2026-04-20 07:29 ` Richard Guo <[email protected]>
2026-04-20 08:42 ` Dean Rasheed <[email protected]>
2026-04-21 06:05 ` Richard Guo <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox