Re: auto_explain produces invalid JSON

Andrew Dunstan <andrew@dunslane.net>

From: Andrew Dunstan <andrew@dunslane.net>
To: Peter Eisentraut <peter_e@gmx.net>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2012-02-11T18:18:59Z
Lists: pgsql-hackers

On 02/10/2012 01:14 PM, Peter Eisentraut wrote:
> The auto_explain module appears to create invalid JSON (in all versions
> since 9.0).  For example, with the settings
>
> auto_explain.log_format = 'json'
> auto_explain.log_min_duration = 0
>
> the query
>
> select * from pg_type;
>
> produces this in the log:
>
> LOG:  duration: 529.808 ms  plan:
>          [
>            "Query Text": "select * from pg_type;",
>            "Plan": {
>              "Node Type": "Seq Scan",
>              "Relation Name": "pg_type",
>              "Alias": "pg_type",
>              "Startup Cost": 0.00,
>              "Total Cost": 9.87,
>              "Plan Rows": 287,
>              "Plan Width": 611
>            }
>          ]
>
> Note that at the top level, it uses the array delimiters [ ] for what is
> actually an object (key/value).  Running this through a JSON parser will
> fail.
>
> By contrast, EXPLAIN (FORMAT JSON) on the command line produces:
>
>              QUERY PLAN
> -----------------------------------
>   [
>     {
>       "Plan": {
>         "Node Type": "Seq Scan",
>         "Relation Name": "pg_type",
>         "Alias": "pg_type",
>         "Startup Cost": 0.00,
>         "Total Cost": 9.87,
>         "Plan Rows": 287,
>         "Plan Width": 611
>       }
>     }
>   ]
> (1 row)
>



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

cheers

andrew