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

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: e(dot)sokolova(at)postgrespro(dot)ru
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Add extra statistics to explain for Nested Loop
Date: 2020-10-31 01:20:53
Message-ID: 20201031012053.arjh7eehmlzud45p@development
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hello Ekaterina,

seems like an interesting and useful improvement. I did a quick review
of the patch - attached is a 0002 patch with a couple minor changes (the
0001 is just your v1 patch, to keep cfbot happy).

1) There's a couple changes to follow project code style (e.g. brackets
after "if" on a separate line, no brackets around single-line blocks,
etc.). I've reverted some unnecessary whitespace changes. Minor stuff.

2) I don't think InstrEndLoop needs to check if min_t == 0 before
initializing it in the first loop. It certainly has to be 0, no? Same
for min_tuples. I also suggest comment explaining that we don't have to
initialize the max values.

3) In ExplainNode, in the part processing per-worker stats, I think some
of the fields are incorrectly referencing planstate->instrument instead
of using the 'instrument' variable from WorkerInstrumentation.

In general, I agree with Andres this might add overhead to explain
analyze, although I doubt it's going to be measurable. But maybe try
doing some measurements for common and worst-cases.

I wonder if we should have another option EXPLAIN option enabling this.
I.e. by default we'd not collect/print this, and users would have to
pass some option to EXPLAIN. Or maybe we could tie this to VERBOSE?

Also, why print this only for nested loop, and not for all nodes with
(nloops > 1)? I see there was some discussion why checking nodeTag is
necessary to identify NL, but that's not my point.


Tomas Vondra
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
0001-extra_statistics_v1.patch text/plain 21.2 KB
0002-minor-tweaks.patch text/plain 4.8 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2020-10-31 01:46:17 Re: Stats collector's idx_blks_hit value is highly misleading in practice
Previous Message Tom Lane 2020-10-31 01:04:35 Re: Should the function get_variable_numdistinct consider the case when stanullfrac is 1.0?