Re: Planning counters in pg_stat_statements (using pgss_store)

On Mon, Mar 2, 2020 at 1:01 PM legrand legrand
<legrand_legrand(at)hotmail(dot)com> wrote:
>
> Julien Rouhaud wrote
> > On Sun, Mar 1, 2020 at 3:55 PM legrand legrand
> > &lt;
>
> > legrand_legrand@
>
> > &gt; wrote:
> >>
> >> >> I like the idea of adding a check for a non-zero queryId in the new
> >> >> pgss_planner_hook() (zero queryid shouldn't be reserved for
> >> >> utility_statements ?).
> >>
> >> > Some assert hit later, I can say that it's not always true. For
> >> > instance a CREATE TABLE AS won't run parse analysis for the underlying
> >> > query, as this has already been done for the original statement, but
> >> > will still call the planner. I'll change pgss_planner_hook to ignore
> >> > such cases, as pgss_store would otherwise think that it's a utility
> >> > statement. That'll probably incidentally fix the IVM incompatibility.
> >>
> >> Today with or without test on parse->queryId != UINT64CONST(0),
> >> CTAS is collected as a utility_statement without planning counter.
> >> This seems to me respectig the rule, not sure that this needs any
> >> new (risky) change to the actual (quite stable) patch.
> >
> > But the queryid ends up not being computed the same way:
> >
> > # select queryid, query, plans, calls from pg_stat_statements where
> > query like 'create table%';
> > queryid | query | plans | calls
> > ---------------------+--------------------------------+-------+-------
> > 8275950546884151007 | create table test as select 1; | 1 | 0
> > 7546197440584636081 | create table test as select 1 | 0 | 1
> > (2 rows)
> >
> > That's because CreateTableAsStmt->query doesn't have a query
> > location/len, as transformTopLevelStmt is only setting that for the
> > top level Query. That's probably an oversight in ab1f0c82257, but I'm
> > not sure what's the best way to fix that. Should we pass that
> > information to all transformXXX function, or let transformTopLevelStmt
> > handle that.
>
>
> arf, this was not the case in my testing env (that is not up to date) :o(
> and would not have appeared at all with the proposed test on
> parse->queryId != UINT64CONST(0) ...

I'm not sure what was the exact behavior you had, but that shouldn't
have changed since previous version. The underlying query isn't a top
level statement, so maybe you didn't set pg_stat_statements.track =
'all'?