On Sun, Sep 6, 2009 at 6:10 AM, Marko Tiikkaja
<marko.tiikkaja@cs.helsinki.fi> wrote:
> Fixed a couple of bugs and renovated ExecInitDml() a bit. Patch attached.
Hi, I'm reviewing this patch for this CommitFest.
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", ...).
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?
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.
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?
I'll try to provide some more feedback on this after I look it over some more.
...Robert