Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)
| От | Chao Li |
|---|---|
| Тема | Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) |
| Дата | |
| Msg-id | 8D1CD3EB-BF72-4C73-AF24-D88581AC01BE@gmail.com обсуждение |
| Ответ на | Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) (Richard Guo <guofenglinux@gmail.com>) |
| Список | pgsql-hackers |
> On Apr 14, 2026, at 11:27, Richard Guo <guofenglinux@gmail.com> wrote: > > On Mon, Apr 13, 2026 at 8:04 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote: >> On Mon, 13 Apr 2026, 09:20 Richard Guo, <guofenglinux@gmail.com> 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 targetlistpassed to ReplaceVarsFromTargetList() by rewriteRuleAction(). I haven't tried it, but I imagine it could reusesome 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 heavieras 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 preferablefor 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/
В списке pgsql-hackers по дате отправления: