Thread: testing with pg_stat_statements

testing with pg_stat_statements

From
Andrew Dunstan
Date:
After the recent case where the SQL/JSON patches had an error that only
exhibited when pg_stat_statement was preloaded, I decided to see if
there were any other such cases in our committed code. So I tested it
out with a modified buildfarm client with this line added in the initdb
step where it's adding the extra_config to postgresql.conf:

    print $handle "shared_preload_libraries = 'pg_stat_statements'\n";

The good news is that it didn't actually find anything amiss. The bad
news is that it generated a bunch of diffs along these lines:


 EXPLAIN (verbose, costs off) SELECT * FROM functest_sri1();
-              QUERY PLAN             
---------------------------------------
+              QUERY PLAN              
+---------------------------------------
  Seq Scan on temp_func_test.functest3
    Output: functest3.a
-(2 rows)
+ Query Identifier: 4255315482610697537
+(3 rows)


ISTM there's probably a good case for suppressing the "Query Identifier"
lines if costs are off. The main reason for saying "costs off" is to
have predictable results, AFAICT, and clearly the query identifier is
not predictable. It would be a trivial change in explain.c. Another
possibility would be to add another option to supporess the query
identifier separately, but that seems like overkill.

Thoughts?


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com




Re: testing with pg_stat_statements

From
Julien Rouhaud
Date:
On Fri, Sep 17, 2021 at 3:08 AM Andrew Dunstan <andrew@dunslane.net> wrote:
>
> So I tested it
> out with a modified buildfarm client with this line added in the initdb
> step where it's adding the extra_config to postgresql.conf:
>
>     print $handle "shared_preload_libraries = 'pg_stat_statements'\n";
>
> The good news is that it didn't actually find anything amiss.

That's something I also regularly do when working on patches, so I
confirm that there's no problem with pg_stat_statements, at least to
my knowledge.

> The bad
> news is that it generated a bunch of diffs along these lines:
>
>
>  EXPLAIN (verbose, costs off) SELECT * FROM functest_sri1();
> -              QUERY PLAN
> ---------------------------------------
> +              QUERY PLAN
> +---------------------------------------
>   Seq Scan on temp_func_test.functest3
>     Output: functest3.a
> -(2 rows)
> + Query Identifier: 4255315482610697537
> +(3 rows)
>
>
> ISTM there's probably a good case for suppressing the "Query Identifier"
> lines if costs are off. The main reason for saying "costs off" is to
> have predictable results, AFAICT, and clearly the query identifier is
> not predictable. It would be a trivial change in explain.c. Another
> possibility would be to add another option to supporess the query
> identifier separately, but that seems like overkill.

Yes that's something that I also find annoying.  I'm not sure if
removing the queryid when costs is disabled is the best way forward,
but I have no strong objection.

Note that we do have a test in explain.sql to make sure that a queryid
is computed and outputed in explain plans, but it doesn't rely on
costs being disabled so that wouldn't be a problem.