On Sat, Dec 28, 2019 at 12:42 AM legrand legrand <legrand_legr...@hotmail.com> wrote: > > Hello, > Thank you for this patch. > > I have tried to use an other patch with yours: > "Planning counters in pg_stat_statements (using pgss_store)" > https://www.postgresql.org/message-id/CAOBaU_Y12bn0tOdN9RMBZn29bfYYH11b2CwKO1RO7dX9fQ3aZA%40mail.gmail.com > > setting > shared_preload_libraries='pg_stat_statements' > pg_stat_statements.track=all > and creating the extension > > > When trying following syntax: > > create table b1 (id integer, x numeric(10,3)); > create incremental materialized view mv1 as select id, count(*),sum(x) from > b1 group by id; > insert into b1 values (1,1) > > I got an ASSERT FAILURE in pg_stat_statements.c > on > Assert(query != NULL); > > comming from matview.c > refresh_matview_datafill(dest_old, query, queryEnv, NULL); > or > refresh_matview_datafill(dest_new, query, queryEnv, NULL); > > > If this (last) NULL field was replaced by the query text, a comment or just > "n/a", > it would fix the problem. > > Could this be investigated ?
I digged deeper into this. I found a bug in the pg_stat_statements patch, as the new pgss_planner_hook() doesn't check for a non-zero queryId, which I think should avoid that problem. This however indeed raises the question on whether the query text should be provided, and if the behavior is otherwise correct. If I understand correctly, for now this specific query won't go through parse_analysis, thus won't get a queryId and will be ignored in pgss_ExecutorEnd, so it'll be entirely invisible, except with auto_explain which will only show an orphan plan like this: 2019-12-28 12:03:29.334 CET [9399] LOG: duration: 0.180 ms plan: HashAggregate (cost=0.04..0.06 rows=1 width=60) Group Key: new_16385_0.id -> Named Tuplestore Scan (cost=0.00..0.02 rows=1 width=52)
