On Sat, Sep 19, 2015 at 5:11 PM, Andres Freund <[email protected]> wrote: > Peter's patch upthread fixes this by pulling expressions from > onConflictSet/Where into the targetlist. I disliked this - much less > than initially - a bit because that seems a bit crufty given that we're > not actually getting data from a child node. This is different to > RETURNING where the targetlist massaging is actually important to get > the data up the tree.
Maybe the massaging is somewhat justified by the fact that it's just as good a place as any to reject system columns, and that needs to happen somewhere. I know that you suggested that this be done during parse analysis, but not sure how attached you are to that. Might also be a good choke point for detecting when unexpected vars/expressions appear in the targetlist due to unforeseen circumstances/bugs. I actually cover a couple of "can't happen" cases at the same time, IIRC. Continuing to follow RETURNING may have some value, even if the analogy is a bit more forced here. > An actually trivial, although not all that pretty, fix is to simply > accept wholerow references in fix_join_expr_mutator(), even if not in > the targetlist. As far as I can see the problem right now really can > only be hit for whole row references. I am concerned about the risk of adding bugs to unrelated code paths that this could create. I must admit that this concern is mostly driven by paranoia, and not a seasoned appreciation of problems that could arise from ordinary post-processing of join expressions. > A variant of the second approach is to have a fix_onconflict_expr() > mutator that has such special handler. I suppose you could add a fix_join_expr_context field that had fix_join_expr_mutator() avoid the special handler for post-processing of join expressions. That might be a bit ugly too, but would involve less code duplication. > Any opinions on either approach? I think that I favor my original solution, although only by a tiny margin. I will avoid offering either a -1 or a +1 to any proposal here, although they all sound basically reasonable to me. A more complete targetlist representation would have been something that I'd probably vote against, since it seems complex and invasive, but that doesn't matter now. In short, I defer to others here. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
