I took a look at this patch today, and I'm pretty skeptical about whether it's on the right track. It adds a new kind of RTE called RTE_ALIAS, which doesn't seem particularly descriptive and alias is used elsewhere to mean something fairly different. More generally, I'm not convinced that adding a new type of RTE is the right way to handle this. The changes in pull_up_subqueries_recurse, pullup_replace_vars_callback, preprocess_targetlist, and build_joinrel_tlist seem like weird hacks. Those functions aren't directly related to this feature; why do they need to know about it?
I wonder if we shouldn't be trying to handle resolution of these names at an earlier processing stage, closer to the processor. I notice that set_returning_clause_references() seems to have already solved the hard part of this problem, which is frobbing target list entries to return values from the new row rather than, as they naturally would, the old row. In fact, we can already get approximately the desired effect already: rhaas=# update foo as after set a = before.a + 1 from foo as before where before.a = after.a returning before.a, after.a; a | a ---+--- 1 | 2 (1 row) Now this is a hack, because we don't really want to add an extra scan/join just to get the behavior we want. But it seems to me significant that this processing makes Vars that refer to the target table refer to the new values, and if we got rid of it, they'd refer to the old values. Can't we contrive to make AFTER.x parse into the same Var node that x currently does? Then we don't need an RTE for it. And maybe BEFORE.x ought to parse to the same node that just plain x does but with some marking, or some other node wrapped around it (like a TargetEntry with some flag set?) that suppresses this processing. I'm just shooting from the hip here; that might be wrong in detail, or even in broad strokes, but it just looks to me like the additional RTE kind is going to bleed into a lot of places. This patch also has significant style issues. Conforming to PostgreSQL coding style is essential; if neither the author nor the reviewer fixes problems in this area, then that is essentially making it the committer's job, and the committer may not feel like taking time to do that. Here's a selection of issues that I noticed while reading this through: we use spaces around operators; the patch adds two blank lines that shouldn't be there to the middle of the variable declarations section; variables should be declared in the innermost possible scope; single-statement blocks shouldn't have curly braces; there shouldn't be whitespace before a closing parenthesis; there should be a space after if and before the subsequent parenthesis; braces should be uncuddled; code that does non-obvious things isn't commented. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
