Re: bug: json format and auto_explain

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

In response to

Responses

Browse pgsql-hackers by date

  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