Re: Normalization of utility queries in pg_stat_statements
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Cc: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Date: 2023-02-16T00:34:51Z
Lists: pgsql-hackers
Attachments
- v2-0001-Refactor-regression-tests-of-pg_stat_statements.patch (text/x-diff) patch v2-0001
- v2-0002-Add-more-test-for-utility-queries-in-pg_stat_stat.patch (text/x-diff) patch v2-0002
- v2-0003-Apply-normalization-to-A_Const-and-utilities-in-p.patch (text/x-diff) patch v2-0003
- v2-0004-Remove-normalization-of-A_Const-nodes.patch (text/x-diff) patch v2-0004
On Wed, Feb 08, 2023 at 12:05:24PM +0900, Michael Paquier wrote: > Thoughts and comments are welcome. 0001 and 0002 are useful on their > own to keep track of utilities that use Const and A_Const after going > through the query jumbling, even if an approach based on query string > or the automated query jumbling for utilities is used (the query > string approach a bit its value). I'll add that to the next commit > fest. While wondering about this stuff about the last few days and discussing with bertrand, I have changed my mind on the point that there is no need to be that aggressive yet with the normalization of the A_Const nodes, because the query string normalization of pg_stat_statements is not prepared yet to handle cases where a A_Const value uses a non-quoted value with whitespaces. The two cases where I saw an impact is on the commands that can define an isolation level: SET TRANSACTION and BEGIN. For example, applying normalization to A_Const nodes does the following as of HEAD: 1) BEGIN TRANSACTION READ ONLY, READ WRITE, DEFERRABLE, NOT DEFERRABLE; BEGIN TRANSACTION $1 ONLY, $2 WRITE, $3, $4 DEFERRABLE 2) SET TRANSACTION ISOLATION LEVEL READ COMMITTED; SET TRANSACTION ISOLATION LEVEL $1 COMMITTED On top of that, specifying a different isolation level may cause these commands to be grouped, which is not really cool. All that could be done incrementally later on, in 17~ or later depending on the adjustments that make sense. Attached is an updated patch set. 0003 is basically the same as v3, that I have kept around for clarity in case one wants to see the effect of a A_Const normalization to all the related commands, though I am not proposing that for an upstream integration. 0002 has been completed with a couple of commands to track all the commands with A_Const, so as we never lose sight of what happens. 0004 is what I think could be done for PG16, where normalization affects only Const. At the end of the day, this reflects the following commands that use Const nodes because they use directly queries, so the same rules as SELECT and DMLs apply to them: - DECLARE - EXPLAIN - CREATE MATERIALIZED VIEW - CTAS, SELECT INTO Comments and thoughts welcome. Thanks, -- Michael
Commits
-
Reflect normalization of query strings for utilities in pg_stat_statements
- daa8365a9007 16.0 landed
-
Refine query jumbling handling for CallStmt
- a4e003338d18 16.0 landed
-
Ignore IntoClause.viewQuery in query jumbling
- d69cd3a2e29c 16.0 landed
-
Improve cleanup phases in regression tests of pg_stat_statements
- 9a714b9d6ec0 16.0 landed
-
Refactor more the regression tests of pg_stat_statements
- d0028e35a05a 16.0 landed
-
doc: Update pg_stat_statements about query ID calculation of utilities
- 6a88a058614b 16.0 landed
-
Expand regression tests of pg_stat_statements for utility queries
- de2aca288569 16.0 landed
-
Refactor tests of pg_stat_statements for planning, utility and level tracking
- e8dbdb15dbc8 16.0 landed