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

Hi, hackers. Thank you for your attention to this topic.

Julien Rouhaud wrote:
> +static void show_loop_info(Instrumentation *instrument, bool isworker,
> +                           ExplainState *es);
> 
> I think this should be done as a separate refactoring commit.
Sure. I divided the patch. Now Justin's refactor commit is separated. 
Also I actualized it a bit.

> Most of the comments I have are easy to fix.  But I think that the real 
> problem
> is the significant overhead shown by Ekaterina that for now would apply 
> even if
> you don't consume the new stats, for instance if you have 
> pg_stat_statements.
> And I'm still not sure of what is the best way to avoid that.
I took your advice about InstrumentOption. Now INSTRUMENT_EXTRA exists.
So currently it's no overheads during basic load. Operations using 
INSTRUMENT_ALL contain overheads (because of INSTRUMENT_EXTRA is a part 
of INSTRUMENT_ALL), but they are much less significant than before. I 
apply new overhead statistics collected by pgbench with auto _explain 
enabled.

> Why do you need to initialize min_t and min_tuples but not max_t and
> max_tuples while both will initially be 0 and possibly updated 
> afterwards?
We need this initialization for min values so comment about it located 
above the block of code with initialization.

I am convinced that the latest changes have affected the patch in a 
positive way. I'll be pleased to hear your thoughts on this.

-- 
Ekaterina Sokolova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company