From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> |
Cc: | Euler Taveira de Oliveira <euler(at)timbira(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: bug: json format and auto_explain |
Date: | 2009-12-07 16:07:43 |
Message-ID: | 5029.1260202063@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2009-12-07 16:09:59 | Re: Adding support for SE-Linux security |
Previous Message | Euler Taveira de Oliveira | 2009-12-07 16:05:41 | Re: bug: json format and auto_explain |