Re: Implementing Incremental View Maintenance

On Sat, Dec 28, 2019 at 12:42 AM legrand legrand
<legrand_legrand(at)hotmail(dot)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)