Re: Ticket 117: Explain Buffers

Le 23/01/2010 16:40, Magnus Hagander a écrit :
> 2010/1/23 Guillaume Lelarge <guillaume(at)lelarge(dot)info>:
>> Le 23/01/2010 15:14, Magnus Hagander a écrit :
>>> 2010/1/23 Guillaume Lelarge <guillaume(at)lelarge(dot)info>:
>>>> Hi,
>>>>
>>>> This patch handles two new parameters for EXPLAIN: COSTS and BUFFERS.
>>>> I'm not sure that we need to add support for format. I would like to
>>>> have your opinion on this?
>>>
>>> Not really. The whole idea of using EXPLAIN on the menu is to get the
>>> graphical output, so I don't see where the user would care aobut the
>>> format of the text behind it.
>>>
>>
>> That was also what I thought... but, if so, why do we propose VERBOSE
>> option? in this case, we don't get a graphical EXPLAIN, but a textual
>> EXPLAIN VERBOSE.
>
> Shows how often I use it. I thought we put that stuff in the main
> explain output, not flick to text mode.
>
> Couldn't we put the verbose stuff in the text output and still show
> the graphical one?
>

Done with Euler's patch.

> Hmm. That might at least be easier to do if we use a machine format explain?
>
>>> That said, we might want to consider switching to using one of the
>>> machine readable formats for newer versions, and then just implement
>>> parsing of these options there. In the end, that should lead to less
>>> complicated code.. (mind you, I didn't even look at your code so I
>>> don't know how complex it is :D)
>>>
>>
>> We can't switch completely if we want to still have compatibility with
>> the older releases. Anyways, this is something we'll have to do to ease
>> the process.
>
> Right. What I'd envision us doing is a simple
> if (version < 9.0)
> run_old_explain_code();
> else
> run_new_explain_code();
>
> and only the codepath for run_new_explain_code() - which would use a
> machine format output - would have the implementation of any of these
> new parameters.
>
> It would be slightly more work to get started since the new parser
> needs to be done (hopefully there are classes that'll do 95% of that
> for us already though), but not having to maintain manual parsing of
> these new options should be a win in the long run.
>
> And eventually, some 5 years from now or so, we can retire the old
> explain code because we no longer support pre-9.0 versions :D
>

OK. I'll add a ticket so that we can take care of this in another patch.

>>> While you're whacking around the explain code, one thing that has
>>> always nagged me, is that ANALYZE is an option for EXPLAIN. IMO we
>>> should have "Explain" and "Explain Analyze" as separate commands,
>>> because they actually do different things. And then options for things
>>> like verbose, costs, buffers etc. If we're going to change that ever,
>>> now is probably the best time :-)
>>>
>>> Thoughts?
>>>
>>
>> I don't quite get why we should do that. You can already have an EXPLAIN
>> or an EXPLAIN ANALYZE. Or is it simply to really differentiate them?
>> because one won't launch the query and the other one will?
>
> Now we have:
> explain
> Explain Options -> [verbose, analyze, newstuff]
>
> I'd prefer
> Explain
> Explain Analyze
> Explain Options -> [verbose, newstuff]
>

Done. F7 launches an explain, Shift-F7 launches an explain analyze.

New patch attached.

--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com