Re: auto_explain produces invalid JSON

Andrew Dunstan <andrew@dunslane.net>

From: Andrew Dunstan <andrew@dunslane.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter_e@gmx.net>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2012-02-13T18:33:58Z
Lists: pgsql-hackers

On 02/13/2012 12:48 PM, Tom Lane wrote:
> Andrew Dunstan<andrew@dunslane.net>  writes:
>> On 02/13/2012 11:15 AM, Tom Lane wrote:
>>> After looking a bit more at the existing explain code, it seems like the
>>> critical issue is that explain.c has ExplainOpenGroup/ExplainCloseGroup
>>> calls around the ExplainPrintPlan call (see ExplainOnePlan), while
>>> auto_explain does not.
>> Yeah, maybe. We'd still have to do it conditionally (have to use
>> ExplainBeginOutput for the XML case), but it would possibly be less kludgy.
> Hm?  I wasn't suggesting removing the ExplainBeginOutput call, but more
> like
>
> 			ExplainBeginOutput(&es);
> +			ExplainOpenGroup(...);
> 			ExplainQueryText(&es, queryDesc);
> 			ExplainPrintPlan(&es, queryDesc);
> +			ExplainCloseGroup(...);
> 			ExplainEndOutput(&es);
>
> Details still TBD; the point is just that it's not clear to me why
> auto_explain should need a formatting concept that doesn't already exist
> within explain.c.
>
> 			

This will introduce an extra level of nesting for no good reason.

But this would work:

-           ExplainBeginOutput(&es);
+           if (auto_explain_log_format != EXPLAIN_FORMAT_JSON)
+               ExplainBeginOutput(&es);
+           else
+               ExplainOpenGroup(NULL, NULL, true,& es);
             ExplainQueryText(&es, queryDesc);
             ExplainPrintPlan(&es, queryDesc);
-           ExplainEndOutput(&es);
+           if (auto_explain_log_format != EXPLAIN_FORMAT_JSON)
+               ExplainEndOutput(&es);
+           else
+               ExplainCloseGroup(NULL, NULL, true, &es);

cheers

andrew