Show names of DEALLOCATE as constants in pg_stat_statements

This commit switches query jumbling so as prepared statement names are
treated as constants in DeallocateStmt.  A boolean field is added to
DeallocateStmt to make a distinction between ALL and named prepared
statements, as "name" was used to make this difference before, NULL
meaning DEALLOCATE ALL.

Prior to this commit, DEALLOCATE was not tracked in pg_stat_statements,
for the reason that it was not possible to treat its name parameter as a
constant.  Now that query jumbling applies to all the utility nodes,
this reason does not apply anymore.

Like 638d42a3c520, this can be a huge advantage for monitoring where
prepared statement names are randomly generated, preventing bloat in
pg_stat_statements.  A couple of tests are added to track the new
behavior.

Author: Dagfinn Ilmari Mannsåker, Michael Paquier
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/ZMhT9kNtJJsHw6jK@paquier.xyz

diff --git a/contrib/pg_stat_statements/expected/utility.out b/contrib/pg_stat_statements/expected/utility.out
index 93735d5d851fc5f1d4069bba596507c8e2894f63..f331044f3ea781deeb3f51d793b56683c2daf4fe 100644 (file)
--- a/contrib/pg_stat_statements/expected/utility.out
+++ b/contrib/pg_stat_statements/expected/utility.out
@@ -472,6 +472,47 @@ SELECT pg_stat_statements_reset();
  
 (1 row)
 
+-- Execution statements
+SELECT 1 as a;
+ a 
+---
+ 1
+(1 row)
+
+PREPARE stat_select AS SELECT $1 AS a;
+EXECUTE stat_select (1);
+ a 
+---
+ 1
+(1 row)
+
+DEALLOCATE stat_select;
+PREPARE stat_select AS SELECT $1 AS a;
+EXECUTE stat_select (2);
+ a 
+---
+ 2
+(1 row)
+
+DEALLOCATE PREPARE stat_select;
+DEALLOCATE ALL;
+DEALLOCATE PREPARE ALL;
+SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
+ calls | rows |                 query                 
+-------+------+---------------------------------------
+     2 |    0 | DEALLOCATE $1
+     2 |    0 | DEALLOCATE ALL
+     2 |    2 | PREPARE stat_select AS SELECT $1 AS a
+     1 |    1 | SELECT $1 as a
+     1 |    1 | SELECT pg_stat_statements_reset()
+(5 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--------------------------
+ 
+(1 row)
+
 -- SET statements.
 -- These use two different strings, still they count as one entry.
 SET work_mem = '1MB';

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 55b957d2515a2f2a856752302c640de71c182733..06b65aeef5ca3c7eab84ee6ba44f7b2ee50e7e56 100644 (file)
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -104,8 +104,7 @@ static const uint32 PGSS_PG_MAJOR_VERSION = PG_VERSION_NUM / 100;
  * ignores.
  */
 #define PGSS_HANDLED_UTILITY(n)        (!IsA(n, ExecuteStmt) && \
-                                   !IsA(n, PrepareStmt) && \
-                                   !IsA(n, DeallocateStmt))
+                                   !IsA(n, PrepareStmt))
 
 /*
  * Extension version number, for supporting older extension versions' objects
@@ -830,8 +829,7 @@ pgss_post_parse_analyze(ParseState *pstate, Query *query, JumbleState *jstate)
 
    /*
     * Clear queryId for prepared statements related utility, as those will
-    * inherit from the underlying statement's one (except DEALLOCATE which is
-    * entirely untracked).
+    * inherit from the underlying statement's one.
     */
    if (query->utilityStmt)
    {
@@ -1116,8 +1114,6 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
     * calculated from the query tree) would be used to accumulate costs of
     * ensuing EXECUTEs.  This would be confusing, and inconsistent with other
     * cases where planning time is not included at all.
-    *
-    * Likewise, we don't track execution of DEALLOCATE.
     */
    if (pgss_track_utility && pgss_enabled(exec_nested_level) &&
        PGSS_HANDLED_UTILITY(parsetree))

diff --git a/contrib/pg_stat_statements/sql/utility.sql b/contrib/pg_stat_statements/sql/utility.sql
index 87666d9135fdbef6d5e2fae4cdd10c4d866c5846..5f7d4a467f040aafb6c3e54ac2d6bc1aacd7ec61 100644 (file)
--- a/contrib/pg_stat_statements/sql/utility.sql
+++ b/contrib/pg_stat_statements/sql/utility.sql
@@ -237,6 +237,19 @@ DROP DOMAIN domain_stats;
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 SELECT pg_stat_statements_reset();
 
+-- Execution statements
+SELECT 1 as a;
+PREPARE stat_select AS SELECT $1 AS a;
+EXECUTE stat_select (1);
+DEALLOCATE stat_select;
+PREPARE stat_select AS SELECT $1 AS a;
+EXECUTE stat_select (2);
+DEALLOCATE PREPARE stat_select;
+DEALLOCATE ALL;
+DEALLOCATE PREPARE ALL;
+SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
+SELECT pg_stat_statements_reset();
+
 -- SET statements.
 -- These use two different strings, still they count as one entry.
 SET work_mem = '1MB';

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1b0e9e58195e59dc6c87d1435ac272d47871fddd..7d2032885edfd67b01277a439698e61ab19600dd 100644 (file)
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11953,6 +11953,8 @@ DeallocateStmt: DEALLOCATE name
                        DeallocateStmt *n = makeNode(DeallocateStmt);
 
                        n->name = $2;
+                       n->isall = false;
+                       n->location = @2;
                        $$ = (Node *) n;
                    }
                | DEALLOCATE PREPARE name
@@ -11960,6 +11962,8 @@ DeallocateStmt: DEALLOCATE name
                        DeallocateStmt *n = makeNode(DeallocateStmt);
 
                        n->name = $3;
+                       n->isall = false;
+                       n->location = @3;
                        $$ = (Node *) n;
                    }
                | DEALLOCATE ALL
@@ -11967,6 +11971,8 @@ DeallocateStmt: DEALLOCATE name
                        DeallocateStmt *n = makeNode(DeallocateStmt);
 
                        n->name = NULL;
+                       n->isall = true;
+                       n->location = -1;
                        $$ = (Node *) n;
                    }
                | DEALLOCATE PREPARE ALL
@@ -11974,6 +11980,8 @@ DeallocateStmt: DEALLOCATE name
                        DeallocateStmt *n = makeNode(DeallocateStmt);
 
                        n->name = NULL;
+                       n->isall = true;
+                       n->location = -1;
                        $$ = (Node *) n;
                    }
        ;

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 5217132331fbb0e4991ca4044ed0cdb19db8ab66..fef4c714b8e827cf0326c9e76d3b516deca4df02 100644 (file)
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3929,8 +3929,12 @@ typedef struct ExecuteStmt
 typedef struct DeallocateStmt
 {
    NodeTag     type;
-   char       *name;           /* The name of the plan to remove */
-   /* NULL means DEALLOCATE ALL */
+   /* The name of the plan to remove, NULL if DEALLOCATE ALL */
+   char       *name pg_node_attr(query_jumble_ignore);
+   /* true if DEALLOCATE ALL */
+   bool        isall;
+   /* token location, or -1 if unknown */
+   int         location pg_node_attr(query_jumble_location);
 } DeallocateStmt;
 
 /*