(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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to