Re: Explain uses incoherent options

On Sat, 2012-07-28 at 01:55 +0200, Vik Reykja wrote:
> On Fri, Jul 27, 2012 at 2:50 PM, Vik Reykja <vikreykja@gmail.com> wrote:
> 
> > On Fri, Jul 27, 2012 at 2:47 PM, Guillaume Lelarge <guillaume@lelarge.info
> > > wrote:
> >
> >> On Fri, 2012-07-27 at 14:28 +0200, Vik Reykja wrote:
> >> > Steps to reproduce:
> >> > * start a freshly installed pgAdmin 1.16.0-beta2 and open a query
> >> window on
> >> > any database
> >> > * type the query: "select 1"
> >> > * press F7 or choose Explain from the menu
> >> >
> >> > Expected: the explain graph
> >> > Observed: ERROR:  EXPLAIN option TIMING requires ANALYZE
> >> >
> >> > Since timing is on by default, this means the plain F7 is broken out of
> >> the
> >> > box. It works as expected if I disable timing in the explain options.
> >>
> >> You're right. I decided to set it up by default, but it has this issue
> >> when you don't use the ANALYZE option. I'll need to find another way.
> >>
> >
> > Can't it just not use timing when it's not using analyze?  That would seem
> > like the saner solution to me.
> >
> 
> Sorry about not replying to the list, I didn't do that on purpose.
> 

Oh, no problem. I didn't reply because I didn't had the time to reply.

> Anyway, patch attached.  It removes the plain explain blocking when buffers
> and timing are checked, and instead prefers to ignore those settings when
> not doing an analyze.  I thought it was a bit silly that a simple explain
> was disabled just because you want to see buffers when doing an explain
> analyze (and timing wasn't taken into account which is what spawned the
> initial bug report).
> 

I don't like this idea to be honest, but I don't see a better way. The
patch is correct IMO, so I commited it.

> This is my very first hack on pgAdmin, so if I screwed up on style
> guidelines or lack of comments or regression tests or what have you, please
> forgive me.  I'll learn from my mistakes for next time.

No, seems pretty good to me :)

Thanks for your patch.


-- 
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com