On 02/11/2012 01:18 PM, Andrew Dunstan wrote:
On 02/10/2012 01:14 PM, Peter Eisentraut wrote:
[ auto-explain JSON output should be an object instead of an array ]
Yeah, looks like this dates back to when we first got JSON output.
Auto-explain does this:
ExplainBeginOutput(&es);
ExplainQueryText(&es, queryDesc);
ExplainPrintPlan(&es, queryDesc);
ExplainEndOutput(&es);
But ExplainBeginOutput says:
case EXPLAIN_FORMAT_JSON:
/* top-level structure is an array of plans */
appendStringInfoChar(es->str, '[');
Now that's not true in the auto-explain case, which prints one query +
one plan.
Since this is an exposed API, I don't think we can just change it. We
probably need a new API that does the right thing for beginning and
ending auto_explain output. (ExplainBeginLabeledOutput?)
PFA a patch along these lines, which seems to do the Right Thing (tm)
cheers
andrew
*** a/contrib/auto_explain/auto_explain.c
--- b/contrib/auto_explain/auto_explain.c
***************
*** 290,299 **** explain_ExecutorEnd(QueryDesc *queryDesc)
es.buffers = (es.analyze && auto_explain_log_buffers);
es.format = auto_explain_log_format;
! ExplainBeginOutput(&es);
ExplainQueryText(&es, queryDesc);
ExplainPrintPlan(&es, queryDesc);
! ExplainEndOutput(&es);
/* Remove last line break */
if (es.str->len > 0 && es.str->data[es.str->len - 1] == '\n')
--- 290,299 ----
es.buffers = (es.analyze && auto_explain_log_buffers);
es.format = auto_explain_log_format;
! ExplainBeginLabeledOutput(&es);
ExplainQueryText(&es, queryDesc);
ExplainPrintPlan(&es, queryDesc);
! ExplainEndLabeledOutput(&es);
/* Remove last line break */
if (es.str->len > 0 && es.str->data[es.str->len - 1] == '\n')
*** a/src/backend/commands/explain.c
--- b/src/backend/commands/explain.c
***************
*** 2258,2263 **** ExplainEndOutput(ExplainState *es)
--- 2258,2302 ----
}
/*
+ * Emit the start-of-output boilerplate, if it's labeled, as auto_explain
+ * JSON is. For non-json cases, fall back on ExplainBeginOutput.
+ *
+ */
+ void
+ ExplainBeginLabeledOutput(ExplainState *es)
+ {
+ switch (es->format)
+ {
+ case EXPLAIN_FORMAT_JSON:
+ /* top level is an object of labeled items */
+ appendStringInfoChar(es->str, '{');
+ es->grouping_stack = lcons_int(0, es->grouping_stack);
+ es->indent++;
+ break;
+ default:
+ ExplainBeginOutput(es);
+ }
+ }
+
+ /*
+ * Companion to ExplainBeginLabeledOutput
+ */
+ void
+ ExplainEndLabeledOutput(ExplainState *es)
+ {
+ switch (es->format)
+ {
+ case EXPLAIN_FORMAT_JSON:
+ es->indent--;
+ appendStringInfoString(es->str, "\n}");
+ es->grouping_stack = list_delete_first(es->grouping_stack);
+ break;
+ default:
+ ExplainEndOutput(es);
+ }
+ }
+
+ /*
* Put an appropriate separator between multiple plans
*/
void
*** a/src/include/commands/explain.h
--- b/src/include/commands/explain.h
***************
*** 71,76 **** extern void ExplainQueryText(ExplainState *es, QueryDesc *queryDesc);
--- 71,78 ----
extern void ExplainBeginOutput(ExplainState *es);
extern void ExplainEndOutput(ExplainState *es);
+ extern void ExplainBeginLabeledOutput(ExplainState *es);
+ extern void ExplainEndLabeledOutput(ExplainState *es);
extern void ExplainSeparatePlans(ExplainState *es);
extern void ExplainPropertyList(const char *qlabel, List *data,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
