I'm reading through the Explain XML patch. Just a couple of quick
observations:

There are a few whitespace oddities which like:

        switch (nodeTag(plan))
        {
                case T_IndexScan:
+               {
+                       StringInfo index;
+                       index = makeStringInfo();

+ appendStringInfo(index, "name=\"%s\"", explain_get_index_name(((IndexScan *) 
plan)->indexid)); 


I suspect this is actually introduced by the mail archiving software.
Something to beware of, if you have locally archived copies of the emails it's
probably better to use those to extract patches :(


Also, this change is bogus:

        {"without", WITHOUT, UNRESERVED_KEYWORD},
        {"work", WORK, UNRESERVED_KEYWORD},
        {"write", WRITE, UNRESERVED_KEYWORD},
!       {"xml", XML_P, UNRESERVED_KEYWORD},
        {"xmlattributes", XMLATTRIBUTES, COL_NAME_KEYWORD},
        {"xmlconcat", XMLCONCAT, COL_NAME_KEYWORD},
        {"xmlelement", XMLELEMENT, COL_NAME_KEYWORD},
--- 415,422 ----
        {"without", WITHOUT, UNRESERVED_KEYWORD},
        {"work", WORK, UNRESERVED_KEYWORD},
        {"write", WRITE, UNRESERVED_KEYWORD},
!       {"xml", XML, UNRESERVED_KEYWORD},
!       {"xmlp", XML_P, UNRESERVED_KEYWORD},
        {"xmlattributes", XMLATTRIBUTES, COL_NAME_KEYWORD},
        {"xmlconcat", XMLCONCAT, COL_NAME_KEYWORD},
        {"xmlelement", XMLELEMENT, COL_NAME_KEYWORD},


Just use the existing XML_P keyword. The "_P" just stands for "parser" to
disambiguate it (I'm not sure a bison keyword could conflict with anything...
but anyways...).



-- 
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com
  Ask me about EnterpriseDB's 24x7 Postgres support!

-- 
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