Skip site navigation (1) Skip section navigation (2)

Re: generic explain options v3

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: generic explain options v3
Date: 2009-07-27 02:52:24
Message-ID: 603c8f070907261952r600f5a73r499f44f5fabff25d@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On Sun, Jul 26, 2009 at 7:40 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> Here's the update.  There are a few things that I'm not entirely happy
>> with here, but not quite sure what to do about either.
>
> Committed with a few editorializations.

Thanks.

>> - ExplainPrintPlan() is now almost trivial.  It seems like there
>> should be some way to get rid of this altogether, but I'm not quite
>> sure how.  I thought about ripping pstmt and rtable out of
>> ExplainState and just storying queryDesc there.  But that involves
>> changing a lot of code, and while it makes some things simpler, it
>> makes other parts more complex.  I'm not sure whether it's a win or
>> not; I'm also not sure how much brainpower it's worth spending on
>> this.
>
> I think the problem here is that you chose to treat ExplainState.pstmt
> as a parameter, when it's better considered as an internal field.
> I changed it to the latter approach.

Sounds fine.

>> - It's becoming increasingly evident to me that the explain stuff in
>> prepare.c has no business being there and should be moved to
>> explain.c.  I haven't done that here, but it's worth thinking about.
>
> I'm unconvinced.  The reason that code is that way is that the
> alternative would require explain.c to know quite a lot about prepared
> plans, which does not seem like an improvement.

I didn't consider that.  As it is, prepare.c has to know quite a lot
about explaining, so it may be six of one, half a dozen of the other.

>> - The hack needed in ExplainLogLevel is just that.
>
> Yeah, I thought that was okay.  We could alternatively refactor the
> code so that the parameter analysis code is a separate function that
> utility.c could call, but it's unclear that it's worth the trouble.

OK.

It seems I have quite a bit of work in front of me unbreaking the
machine-readable explain patch.  I started grinding through it, but
it's not pretty.  I'll post an updated version when I have it.

...Robert

In response to

pgsql-hackers by date

Next:From: Itagaki TakahiroDate: 2009-07-27 02:54:59
Subject: Re: BUG #4941: pg_stat_statements crash
Previous:From: Tom LaneDate: 2009-07-27 02:28:18
Subject: Re: BUG #4941: pg_stat_statements crash

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group