Re: pgsql: Include information on buffer usage during planningphase, in EX - Mailing list pgsql-committers

From Julien Rouhaud
Subject Re: pgsql: Include information on buffer usage during planningphase, in EX
Date
Msg-id 20200403124943.4p4drqwujifi4kv6@nol
Whole thread Raw
In response to Re: pgsql: Include information on buffer usage during planning phase,in EX  (Fujii Masao <masao.fujii@oss.nttdata.com>)
List pgsql-committers
On Fri, Apr 03, 2020 at 09:27:38PM +0900, Fujii Masao wrote:
> 
> 
> On 2020/04/03 16:43, Julien Rouhaud wrote:
> > On Fri, Apr 03, 2020 at 03:24:41PM +0900, Fujii Masao wrote:
> > > 
> > > 
> > > On 2020/04/03 12:30, Tom Lane wrote:
> > > > Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> > > > > On 2020/04/03 11:58, Michael Paquier wrote:
> > > > > > prion did not like that:
> > > > > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2020-04-03%2002%3A33%3A13
> > > > 
> > > > dory failed as well.  The problem is that in text mode, a Buffers line
> > > > won't appear at all if there were zero buffer accesses.  I don't think
> > > > we really want to change that,
> > > 
> > > Yes.
> > > 
> > > > so probably the thing to do is adapt
> > > > the filter functions in explain.sql so that they suppress Buffers lines
> > > > altogether in text output.  Kind of annoying, but ...
> > > 
> > > I'm thinking to suppress only Buffers line just after Planning Time line,
> > > by applying something like the following changes to explain_filter().
> > > Thought?
> > 
> > 
> > +1, that's a better workaround.
> > 
> > 
> > > 
> > >   declare
> > >       ln text;
> > > +    ignore_output boolean;
> > >   begin
> > >       for ln in execute $1
> > >       loop
> > > +        IF ignore_output THEN
> > > +            ignore_output := false;
> > > +            CONTINUE WHEN (ln ~~ '  Buffers: %');
> > > +        END IF;
> > > +        ignore_output := ln ~~ 'Planning Time: %';
> > >           -- Replace any numeric word with just 'N'
> > >           ln := regexp_replace(ln, '\m\d+\M', 'N', 'g');
> > >           -- In sort output, the above won't match units-suffixed numbers
> > 
> > 
> > I'm not sure of what's plpgsql behavior here, but it's probably better to
> > initialize ignore_output to false.
> 
> Thanks for the review! Yes, that's necessary.
> 
> > Nitpicking, but I think that
> > planning_time_found, or something similar, would be better here.
> 
> Yeah, that sounds better.
> 
> Attached is the updated version of the patch.


Thanks Fuji-san!  Just to be extra safe I locally added an extra

RAISE notice '%', ln;

before the continue and I confirm that this is only filtering the intended
lines.

Minor thinko:

+    -- test output stable. This is necessary because whether text-mode
+    -- buffers output for the planning appears or not varies depending
+    -- on the compile option. It appears if the server is built with
+    -- -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE,
+    -- but not otherwise.

those lines begin with a leading tabulation while the rest of the sql file only
contains space indentation.  Other than that it all looks good to me.



pgsql-committers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: pgsql: Include information on buffer usage during planning phase,in EX
Next
From: Tom Lane
Date:
Subject: Re: pgsql: Include information on buffer usage during planning phase, in EX