On 4 April 2018 at 18:51, Tom Lane <t...@sss.pgh.pa.us> wrote: > Simon Riggs <si...@2ndquadrant.com> writes: >> On 4 April 2018 at 17:19, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> If the MERGE patch has broken this, I'm going to push back on that >>> and push back on it hard, because it probably means there are a >>> whole bunch of other raw-grammar-output-only node types that can >>> now get past the parser stage as well, as children of these nodes. >>> The answer to that is not to add a ton of new infrastructure, it's >>> "you did MERGE wrong". > >> MERGE contains multiple actions of Insert, Update and Delete and these >> could be output in various debug modes. I'm not clear what meaning we >> might attach to them if we looked since that differs from normal >> INSERTs, UPDATEs, DELETEs, but lets see. > > What I'm complaining about is that that's a very poorly designed parsetree > representation. It may not be the worst one I've ever seen, but it's > got claims in that direction. You're repurposing InsertStmt et al to > do something that's *not* an INSERT statement, but by chance happens > to share some (not all) of the same fields. This is bad because it > invites confusion, and then bugs of commission or omission (eg, assuming > that some particular processing has happened or not happened to subtrees > of that parse node). The most egregious way in which it's a bad idea > is that, as I said, InsertStmt doesn't even exist in post-parse-analysis > trees so far as the normal type of INSERT is concerned. This just opens > a whole batch of ways to screw up. We have some types of raw parse nodes > that are replaced entirely during parse analysis, and we have some others > where it's convenient to use the same node type before and after parse > analysis, but we don't have any that are one way in one context and the > other way in other contexts. And this is not the place to start. > > I think you'd have been better off to fold all of those fields into > MergeAction, or perhaps make several variants of MergeAction.
OK, that can be changed, will check and report back tomorrow. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services