On Mon, Dec 14, 2009 at 12:59 PM, Robert Haas <robertmh...@gmail.com> wrote:
>> The new explain formats break if you have a multi-query statement.
> I will fix this, unless someone else beats me to it.

Proposed patch attached.  The problem with JSON output is pretty
simple - ExplainSeparatePlans() still thinks it's responsible for
comma-separating JSON plans, but in fact the new grouping_stack
machinery is smart enough to handle that on its own, so they each add
a comma.  The YAML format also inserts a needless separator, but the
real problem is that ExplainBeginGroup() and ExplainYAMLLineStarting()
have an odd division of labor for marking each list element with "- ",
with each providing one character.   That fails to work as expected.
I also noticed that ExplainDummyGroup() is hosed for the YAML format,
so I fixed that as well.

Along the way, I also made the YAML format use escape_yaml() in
situations where the JSON format uses escape_json().  Right now it
doesn't matter because all the values are known to not need escaping,
but it seems safer this way.  And, it took me a bit of time to
understand the YAML format as it was not really commented, so I added
some comments here as well.

...Robert
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index eb825e8..2067636 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1603,7 +1603,10 @@ ExplainProperty(const char *qlabel, const char *value, bool numeric,
 		case EXPLAIN_FORMAT_YAML:
 			ExplainYAMLLineStarting(es);
 			appendStringInfo(es->str, "%s: ", qlabel);
-			escape_yaml(es->str, value);
+			if (numeric)
+				appendStringInfoString(es->str, value);
+			else
+				escape_yaml(es->str, value);
 			break;
 	}
 }
@@ -1691,23 +1694,15 @@ ExplainOpenGroup(const char *objtype, const char *labelname,
 			break;
 
 		case EXPLAIN_FORMAT_YAML:
-
-			/*
-			 * In YAML format, the grouping stack is an integer list.  0 means
-			 * we've emitted nothing at this grouping level AND this grouping
-			 * level is unlabelled and must be marked with "- ".  See
-			 * ExplainYAMLLineStarting().
-			 */
 			ExplainYAMLLineStarting(es);
 			if (labelname)
 			{
-				escape_yaml(es->str, labelname);
-				appendStringInfoChar(es->str, ':');
+				appendStringInfo(es->str, "%s:", labelname);
 				es->grouping_stack = lcons_int(1, es->grouping_stack);
 			}
 			else
 			{
-				appendStringInfoString(es->str, "- ");
+				appendStringInfoChar(es->str, '-');
 				es->grouping_stack = lcons_int(0, es->grouping_stack);
 			}
 			es->indent++;
@@ -1782,15 +1777,8 @@ ExplainDummyGroup(const char *objtype, const char *labelname, ExplainState *es)
 		case EXPLAIN_FORMAT_YAML:
 			ExplainYAMLLineStarting(es);
 			if (labelname)
-			{
-				escape_yaml(es->str, labelname);
-				appendStringInfoString(es->str, ": ");
-			}
-			else
-			{
-				appendStringInfoString(es->str, "- ");
-			}
-			escape_yaml(es->str, objtype);
+				appendStringInfo(es->str, "%s:", labelname);
+			appendStringInfoString(es->str, objtype);
 			break;
 	}
 }
@@ -1867,15 +1855,19 @@ ExplainSeparatePlans(ExplainState *es)
 	switch (es->format)
 	{
 		case EXPLAIN_FORMAT_TEXT:
+		case EXPLAIN_FORMAT_YAML:
 			/* add a blank line */
 			appendStringInfoChar(es->str, '\n');
 			break;
 
 		case EXPLAIN_FORMAT_XML:
-		case EXPLAIN_FORMAT_JSON:
-		case EXPLAIN_FORMAT_YAML:
 			/* nothing to do */
 			break;
+
+		case EXPLAIN_FORMAT_JSON:
+			/* must have a comma between array elements */
+			appendStringInfoChar(es->str, ',');
+			break;
 	}
 }
 
@@ -1928,12 +1920,6 @@ ExplainJSONLineEnding(ExplainState *es)
 
 /*
  * Indent a YAML line.
- *
- * YAML lines are ordinarily indented by two spaces per indentation level.
- * The text emitted for each property begins just prior to the preceding
- * line-break, except for the first property in an unlabelled group, for which
- * it begins immediately after the "- " that introduces the group.  The first
- * property of the group appears on the same line as the opening "- ".
  */
 static void
 ExplainYAMLLineStarting(ExplainState *es)
@@ -1941,6 +1927,7 @@ ExplainYAMLLineStarting(ExplainState *es)
 	Assert(es->format == EXPLAIN_FORMAT_YAML);
 	if (linitial_int(es->grouping_stack) == 0)
 	{
+		appendStringInfoChar(es->str, ' ');
 		linitial_int(es->grouping_stack) = 1;
 	}
 	else
@@ -1996,8 +1983,7 @@ escape_json(StringInfo buf, const char *str)
 }
 
 /*
- * 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.
+ * YAML is a superset of JSON: if we find quotable characters, we call escape_json
  */
 static void
 escape_yaml(StringInfo buf, const char *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