Re: auto_explain produces invalid JSON

On 02/13/2012 01:33 PM, Andrew Dunstan wrote:
>
>
> 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);
>
>

Except that it causes other problems.

I think we'd probably bet sleeping dogs lie.

cheers

andrew