Re: Planning counters in pg_stat_statements (using pgss_store)

On Tue, Mar 31, 2020 at 12:21:43PM +0900, Fujii Masao wrote:
>
> On 2020/03/31 3:16, Julien Rouhaud wrote:
> > On Mon, Mar 30, 2020 at 6:36 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> > >
> > > While testing the patched pgss, I found that the patched version
> > > may track the statements that the original version doesn't.
> > > Please imagine the case where the following queries are executed,
> > > with pgss.track = top.
> > >
> > > PREPARE hoge AS SELECT * FROM t;
> > > EXPLAIN EXECUTE hoge;
> > >
> > > The pgss view returned "PREPARE hoge AS SELECT * FROM t"
> > > in the patched version, but not in the orignal version.
> > >
> > > Is this problematic?
> >
> > Oh indeed. That's a side effect of having different the executed query
> > and the planned query being different.
> >
> > I guess the question is to chose if the top level executed query of a
> > utilty statement containing an optimisable query, should the top level
> > planner call of that optimisable statement be considered at top level
> > or not. I tend to think that's the correct behavior here, as this is
> > also what would happen if a regular DML was provided. What do you
> > think?
>
> TBH, not sure if that's ok yet...
>
> I'm now just wondering if both plan_nested_level and
> exec_nested_level should be incremented in pgss_ProcessUtility().
> This is just a guess, so I need more investigation about this.

Yeah, after a second thought I realize that my comparison was wrong. Allowing
*any* top-level planner call when pgss.track = top would mean that we should
also consider all planner calls from queries executed for FK checks and such,
which is definitely not the intended behavior.

FTR with this patch such calls still don't get tracked, but only because those
query don't get a queryid assigned, not because the nesting level says so.

How about simply passing (plan_nested_level + exec_nested_level) for
pgss_enabled call in pgss_planner_hook?