On Mon, Mar 2, 2020 at 1:01 PM legrand legrand <legrand_legr...@hotmail.com> wrote: > > Julien Rouhaud wrote > > On Sun, Mar 1, 2020 at 3:55 PM legrand legrand > > < > > > legrand_legrand@ > > > > 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'?
