Re: bug: json format and auto_explain

On Mon, Dec 7, 2009 at 11:07 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> writes:
>> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Looks like auto_explain is under the illusion that it need not call
>>> ExplainBeginOutput/ExplainEndOutput.
>
>> Explain{Begin/End}Output are static functions, so we cannot call them
>> from an external contrib module. Instead, I'll suggest to call them
>> automatically from ExplainPrintPlan. The original codes in ExplainPrintPlan
>> was moved into ExplainOneResult, that name might be debatable.
>
> This isn't an adequate solution I'm afraid --- what about the other
> functions that are exported by explain.h?
>
> I too am not totally thrilled with the idea of exporting
> Explain{Begin/End}Output, but it might be the best solution.
> We might also need to think about refactoring those functions:
> there seem to be two different things going on there, one being
> format-specific initialization which will certainly be necessary,
> and one being output of a wrapper structure which might or might
> not be appropriate for what auto_explain or other callers want.
>
> In any case you need to expend more effort on the comments for the
> functions.  Inadequate specification of ExplainPrintPlan's call
> requirements is what got us into this problem in the first place,
> and the proposed patch makes that worse not better.

There's an awful lot of layers of function calls happening in
explain.c for no really discernable purpose that I can see.
ExplainOneQuery() calls either ExplainOneUtility() if it has a utility
command or ExplainOneQuery_hook() if that's defined or else it plans
the query and then calls ExplainOnePlan(). ExplainOneUtility() in
turn calls ExplainExecuteQuery for execute statements and handles
everything else internally. The placement of the hook seems pretty
dubious to me (does anyone actually use it?), and the whole structure
seems like it could probably be flattened a bit.

...Robert