Re: Add support for printing/reading MergeAction nodes - Mailing list pgsql-hackers

From Simon Riggs
Subject Re: Add support for printing/reading MergeAction nodes
Date
Msg-id CANP8+jJ5uddo7AqthhLPkNz5CWpAq565+t3pK+q_hAGZfhjJ+g@mail.gmail.com
Whole thread Raw
In response to Re: Add support for printing/reading MergeAction nodes  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On 4 April 2018 at 18:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Simon Riggs <simon@2ndquadrant.com> writes:
>> On 4 April 2018 at 17:19, Tom Lane <tgl@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


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Foreign keys and partitioned tables
Next
From: Andres Freund
Date:
Subject: Re: Postgres stucks in deadlock detection