From 2e36ee327a7971610ac395418c9e22bf8145850d Mon Sep 17 00:00:00 2001 From: Dean Rasheed 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