Re: Planning counters in pg_stat_statements (using pgss_store)

From: Julien Rouhaud <rjuju123(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: Sergei Kornilov <sk(at)zsrv(dot)org>, "imai(dot)yoshikazu(at)fujitsu(dot)com" <imai(dot)yoshikazu(at)fujitsu(dot)com>, legrand legrand <legrand_legrand(at)hotmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Planning counters in pg_stat_statements (using pgss_store)
Date: 2020-03-31 07:33:21
Message-ID: 20200331073321.bhnx2p63gp5px37f@nol
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 31, 2020 at 04:10:47PM +0900, Fujii Masao wrote:
>
>
> On 2020/03/31 15:03, Julien Rouhaud wrote:
> > 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.
>
> Yes. So, basically any planner activity that happens during
> the execution phase of the statement is not tracked.
>
> > 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?
>
> Looks good to me! The comment about why this treatment is necessary only in
> pgss_planner() should be added.

Yes of course. It also requires some changes in the macro to make it safe when
called with an expression.

v12 attached!

Attachment Content-Type Size
v12-0001-Add-planning-counters-to-pg_stat_statements.patch text/x-diff 47.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2020-03-31 07:36:39 Re: Corruption during WAL replay
Previous Message Fujii Masao 2020-03-31 07:10:47 Re: Planning counters in pg_stat_statements (using pgss_store)