Testing 9.0 beta, I found that EXPLAINing certain queries in YAML
format will produce invalid YAML, for example:

explain (format yaml) select * from foo where str_val = 'a: b';

The problem in this case is that a colon followed by whitespace is not
allowed in an unquoted plain YAML string because a parser would
interpret it as the start of a map.

So the current code in escape_yaml() is inadequate for producing valid
YAML. I think it would have to also consider at least the following
characters as special "-"  ":"  "["  "]"  "{"  "}"  ","  "\""  "'"
"|"  "*"  "&". Technically, it would also need to trap empty strings,
and strings with leading or trailing whitespace.

Making escape_yaml() completely bulletproof with this approach would
be quite difficult, and (IMO) not worth the effort, especially given
that an important requirement is that the output be machine readable,
and in my experience YAML parsers are often far from perfect.

I would therefore argue for simply calling escape_json() to produce
double quoted output for all string values, and only have numeric
values unquoted. This is not really any less human readable, and is
far more machine readable.

Patch attached.

 - Dean
*** ./src/backend/commands/explain.c.orig	2010-06-07 08:49:06.000000000 +0100
--- ./src/backend/commands/explain.c	2010-06-07 09:09:44.000000000 +0100
***************
*** 1698,1705 ****
  
  		case EXPLAIN_FORMAT_YAML:
  			ExplainYAMLLineStarting(es);
! 			escape_yaml(es->str, qlabel);
! 			appendStringInfoChar(es->str, ':');
  			foreach(lc, data)
  			{
  				appendStringInfoChar(es->str, '\n');
--- 1698,1704 ----
  
  		case EXPLAIN_FORMAT_YAML:
  			ExplainYAMLLineStarting(es);
! 			appendStringInfo(es->str, "%s: ", qlabel);
  			foreach(lc, data)
  			{
  				appendStringInfoChar(es->str, '\n');
***************
*** 1759,1765 ****
  		case EXPLAIN_FORMAT_YAML:
  			ExplainYAMLLineStarting(es);
  			appendStringInfo(es->str, "%s: ", qlabel);
! 			escape_yaml(es->str, value);
  			break;
  	}
  }
--- 1758,1767 ----
  		case EXPLAIN_FORMAT_YAML:
  			ExplainYAMLLineStarting(es);
  			appendStringInfo(es->str, "%s: ", qlabel);
! 			if (numeric)
! 				appendStringInfoString(es->str, value);
! 			else
! 				escape_yaml(es->str, value);
  			break;
  	}
  }
***************
*** 1857,1864 ****
  			ExplainYAMLLineStarting(es);
  			if (labelname)
  			{
! 				escape_yaml(es->str, labelname);
! 				appendStringInfoChar(es->str, ':');
  				es->grouping_stack = lcons_int(1, es->grouping_stack);
  			}
  			else
--- 1859,1865 ----
  			ExplainYAMLLineStarting(es);
  			if (labelname)
  			{
! 				appendStringInfo(es->str, "%s: ", labelname);
  				es->grouping_stack = lcons_int(1, es->grouping_stack);
  			}
  			else
***************
*** 2152,2173 ****
  }
  
  /*
!  * YAML is a superset of JSON: if we find quotable characters, we call
!  * escape_json.  If not, we emit the property unquoted for better readability.
   */
  static void
  escape_yaml(StringInfo buf, const char *str)
  {
! 	const char *p;
! 
! 	for (p = str; *p; p++)
! 	{
! 		if ((unsigned char) *p < ' ' || strchr("\"\\\b\f\n\r\t", *p))
! 		{
! 			escape_json(buf, str);
! 			return;
! 		}
! 	}
! 
! 	appendStringInfo(buf, "%s", str);
  }
--- 2153,2164 ----
  }
  
  /*
!  * YAML is a superset of JSON: strings that don't contain special characters
!  * could be left unquoted, but for compatibility with a wider range of YAML
!  * parsers, we always emit quoted strings here.
   */
  static void
  escape_yaml(StringInfo buf, const char *str)
  {
! 	escape_json(buf, str);
  }
-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Reply via email to