Re: pgsql: Include information on buffer usage during planning phase, in EX

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(dot)fujii(at)oss(dot)nttdata(dot)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.