Re: Ticket 117: Explain Buffers

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?

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

>> 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]

My usecase is that most of the time, I want to do explain. So I d ot.
Then to do explain analyze today i have to do:
Explain Options -> Analyze
Explain
Explain Options -> Analyze (so I don't accidentally run with analyze
the next time again)

I think most people will set what's under Explain Options to "this is
how I like it". But that's just not relevant for analyze - it's not a
preference, it's a different operation.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/