Re: [PATCH] Add extra statistics to explain for Nested Loop
Justin Pryzby <pryzby@telsasoft.com>
From: Justin Pryzby <pryzby@telsasoft.com>
To: Ekaterina Sokolova <e.sokolova@postgrespro.ru>
Cc: Julien Rouhaud <rjuju123@gmail.com>, Greg Stark <stark@mit.edu>, Lukas Fittl <lukas@fittl.com>, pgsql-hackers@lists.postgresql.org
Date: 2022-04-02T12:38:26Z
Lists: pgsql-hackers
Attachments
- 0001-explain.c-refactor-ExplainNode.patch (text/x-diff)
This message lost track of the email headers so CFBOT isn't processing the new patches. Which I'm attempting to remedy now. https://www.postgresql.org/message-id/flat/ae576cac3f451d318374f2a2e494aab1@postgrespro.ru On Fri, Apr 01, 2022 at 11:46:47PM +0300, Ekaterina Sokolova wrote: > 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.
Commits
-
Fix wrong units in two ExplainPropertyFloat calls.
- f90c708a0486 14.0 cited