testing with pg_stat_statements

Started by Andrew Dunstanover 4 years ago2 messages
#1Andrew Dunstan
andrew@dunslane.net

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

#2Julien Rouhaud
rjuju123@gmail.com
In reply to: Andrew Dunstan (#1)
Re: testing with pg_stat_statements

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.