Re: Explain uses incoherent options

From: Guillaume Lelarge <guillaume(at)lelarge(dot)info>
To: Vik Reykja <vikreykja(at)gmail(dot)com>
Cc: PgAdmin Support <pgadmin-support(at)postgresql(dot)org>
Subject: Re: Explain uses incoherent options
Date: 2012-07-28 15:49:35
Message-ID: 1343490575.2011.9.camel@localhost.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-support

On Sat, 2012-07-28 at 01:55 +0200, Vik Reykja wrote:
> On Fri, Jul 27, 2012 at 2:50 PM, Vik Reykja <vikreykja(at)gmail(dot)com> wrote:
>
> > On Fri, Jul 27, 2012 at 2:47 PM, Guillaume Lelarge <guillaume(at)lelarge(dot)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

In response to

Responses

Browse pgadmin-support by date

  From Date Subject
Next Message Vik Reykja 2012-07-28 17:07:44 Re: Explain uses incoherent options
Previous Message Guillaume Lelarge 2012-07-28 15:31:39 Re: [pgadmin-support] ppAdmin.EXE with i7 2630QM