Re: [PATCH] Add extra statistics to explain for Nested Loop

<rjuju123(at)gmail(dot)com> wrote:
> You should update the explain_parallel_append() plpgsql function
> created in that test file to make sure that both "rows" and the two
> new counters are changed to "N". There might be other similar changes
> needed.

Thank you for watching this issue. I made the necessary changes in tests
following your advice.

> Now, for the changes themselves. For the min/max time, you're
> aggregating "totaltime - instr->firsttuple". Why removing the startup
> time from the loop execution time? I think this should be kept.

I think it's right remark. I fixed it.

> Also, in explain.c you display the counters in the "Nested loop" node,
> but this should be done in the inner plan node instead, as this is the
> one being looped on. So the condition should probably be "nloops > 1"
> rather than testing if it's a NestLoop.

Condition "nloops > 1" is not the same as checking if it's NestLoop.
This condition will lead to printing extra statistics for nodes with
different types of join, not only Nested Loop Join. If this statistic is
useful for other plan nodes, it makes sense to change the condition.

<andres(at)anarazel(dot)de> wrote:
> I'm a bit worried that further increasing the size of struct
> Instrumentation will increase the overhead of EXPLAIN ANALYZE further -
> in some workloads we spend a fair bit of time in code handling that. It
> would be good to try to find a few bad cases, and see what the overhead
> is.

Thank you for this comment, I will try to figure it out. Do you have
some examples of large overhead dependent on this struct? I think I need
some sample to know which way to think.

Thank you all for the feedback. I hope the new version of my patch will
be more correct and useful.
Please don't hesitate to share any thoughts on this topic!
--
Ekaterina Sokolova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company