Re: Explain XML patch

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!