explain refactoring v4

OK, here it is again. Changes are the same as the previous version,
but this one should apply cleanly after today's pgindent run.

http://archives.postgresql.org/message-id/603c8f070906051803nd4a9e3ekb72bcc379ad73f16@mail.gmail.com

(Rebasing this wasn't too bad, but I have to say that one thing that's
frustrating is that there's no obvious rule for how continuation lines
should be indented. If it were easy for patch authors and committers
to understand what to do, pgindent would change less stuff, which
would make for less rebasing. Just to take one example, in explain.h,
the level of hanging indent for the parameters of ExplainOnePlan is
different than the level for ExplainPrintPlan. That's presumably
because ExplainPrintPlan is two characters longer than ExplainOnePlan,
but beyond that I'm lost.)

...Robert

Hi Robert, Hi all,

On Friday 12 June 2009 06:32:11 Robert Haas wrote:
> OK, here it is again. Changes are the same as the previous version,
> but this one should apply cleanly after today's pgindent run.
Not much to say here.
Patch applies cleanly, the changes look sensible and as far as I can see they
did not introduce visible changes.

Andres

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> OK, here it is again. Changes are the same as the previous version,
> but this one should apply cleanly after today's pgindent run.

As I was poking through this I noticed that it makes at least one small
change in the output format: what had been "Subquery Scan ss" will now
be "Subquery Scan on ss", because of the unification of code that wasn't
really entirely consistent into one subroutine ExplainScanTarget.

This is not likely to matter to human readers but it might to programs.
OTOH we presumably expect programs to migrate to using a more
program-friendly EXPLAIN output format with 8.5.

Does anyone have strong feelings about whether we need to be
bug-compatible with the old formatting?

regards, tom lane

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> OK, here it is again. Changes are the same as the previous version,
> but this one should apply cleanly after today's pgindent run.

Applied with some minor editorialization/further cleanup.

I left the "Subquery Scan on ss" business as-is. If anyone complains
it would be an easy thing to suppress the "on", but it'd be ugly, and
I'm not convinced anyone will care. It's not like it's the first time
we ever changed the output of EXPLAIN ...

regards, tom lane

On Fri, Jul 24, 2009 at 4:02 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> OK, here it is again.  Changes are the same as the previous version,
>> but this one should apply cleanly after today's pgindent run.
>
> As I was poking through this I noticed that it makes at least one small
> change in the output format: what had been "Subquery Scan ss" will now
> be "Subquery Scan on ss", because of the unification of code that wasn't
> really entirely consistent into one subroutine ExplainScanTarget.

Wow, nice catch.

> This is not likely to matter to human readers but it might to programs.
> OTOH we presumably expect programs to migrate to using a more
> program-friendly EXPLAIN output format with 8.5.
>
> Does anyone have strong feelings about whether we need to be
> bug-compatible with the old formatting?

I kind of doubt it. Based on previous discussions, I gather that
PGadmin et al have to be adjusted for each release anyway. But I also
don't think it's a big deal if we make it work the way it used to.

...Robert

On Fri, Jul 24, 2009 at 5:11 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> OK, here it is again.  Changes are the same as the previous version,
>> but this one should apply cleanly after today's pgindent run.
>
> Applied with some minor editorialization/further cleanup.

Thanks. I mostly finished the rework of the generic options patch
last night, but I was so sleepy that I couldn't stay up long enough to
fully test it. I'll try to get that out tonight, or at the latest
tomorrow. Hopefully your minor editorialization and further cleanup
won't create too many merge conflicts.

> I left the "Subquery Scan on ss" business as-is.  If anyone complains
> it would be an easy thing to suppress the "on", but it'd be ugly, and
> I'm not convinced anyone will care.  It's not like it's the first time
> we ever changed the output of EXPLAIN ...

Fine with me either way.

...Robert

On Fri, Jul 24, 2009 at 23:11, Robert Haas<robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jul 24, 2009 at 4:02 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> This is not likely to matter to human readers but it might to programs.
>> OTOH we presumably expect programs to migrate to using a more
>> program-friendly EXPLAIN output format with 8.5.
>>
>> Does anyone have strong feelings about whether we need to be
>> bug-compatible with the old formatting?
>
> I kind of doubt it.  Based on previous discussions, I gather that
> PGadmin et al have to be adjusted for each release anyway.  But I also
> don't think it's a big deal if we make it work the way it used to.

They do. Though for now, the old pgadmin still works (AFAIK) against
HEAD. But that's usually just a matter of time anyway, and it's not
something that's officially supported, AFAIK.

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