Re: Using results from INSERT ... RETURNING

(Sorry, forgot to CC hackers)

Robert Haas wrote:
> With regard to the changes in explain.c, I think that the way you've
> capitalized INSERT, UPDATE, and DELETE is not consistent with our
> usual style for labelling nodes.  Also, you've failed to set sname, so
> this reads from uninitialized memory when using JSON or XML format.  I
> think that you should handle XML/JSON format by setting sname to "Dml"
> and then emit an "operation" field down around where we do if
> (strategy) ExplainPropertyText("Strategy", ...).

You're right, I should fix that.

> I am not sure that I like the name Dml for the node type.  Most of our
> node types are descriptions of the action that will be performed, like
> Sort or HashJoin; Dml is the name of the feature we're trying to
> implement, but it's not the name of the action we're performing.  Not
> sure what would be better, though.  Write?  Modify?

Dml was the first name I came up with and it stuck, but it could be
better.  I don't really like Write or Modify either.

> Can you explain the motivation for changing the Append stuff as part
> of this patch?  It's not immediately clear to me why that needs to be
> done as part of this patch or what we get out of it.

It seemed to me that the Append on top was only a workaround for the
fact that we didn't have a node for DML operations that would select the
correct result relation.  I don't see why an Append node should do this
at all if we have a special node for handling DML.

> What is your general impression about the level of maturity of this
> code?  Are you submitting this as complete and ready for commit, or is
> it a WIP?  If the latter, what are the known issues?

Aside from the EXPLAIN stuff you brought up, there are no issues that
I'm aware of.  There are a few spots that could be prettier, but I have
no good ideas for them.

> I'll try to provide some more feedback on this after I look it over some more.

Thanks!

Regards,
Marko Tiikkaja