(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
