On Tue, Aug 15, 2023 at 08:49:37AM +0900, Michael Paquier wrote: > Hmm. One issue with the patch is that we finish by considering > DEALLOCATE ALL and DEALLOCATE $1 as the same things, compiling the > same query IDs. The difference is made in the Nodes by assigning NULL > to the name but we would now ignore it. Wouldn't it be better to add > an extra field to DeallocateStmt to track separately the named > deallocate queries and ALL in monitoring?
In short, I would propose something like that, with a new boolean field in DeallocateStmt that's part of the jumbling. Dagfinn, Julien, what do you think about the attached? -- Michael
From ad91672367f408663824b36b6dd517513d7f0a2a Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Fri, 18 Aug 2023 09:12:58 +0900
Subject: [PATCH v2] Track DEALLOCATE statements in pg_stat_activity
Treat the statement name as a constant when jumbling. A boolean field
is added to DeallocateStmt to make a distinction between the ALL and
named cases.
---
src/include/nodes/parsenodes.h | 8 +++-
src/backend/parser/gram.y | 8 ++++
.../pg_stat_statements/expected/utility.out | 41 +++++++++++++++++++
.../pg_stat_statements/pg_stat_statements.c | 8 +---
contrib/pg_stat_statements/sql/utility.sql | 13 ++++++
5 files changed, 70 insertions(+), 8 deletions(-)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 2565348303..2b356336eb 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3925,8 +3925,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;
/*
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b3bdf947b6..df34e96f89 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11936,6 +11936,8 @@ DeallocateStmt: DEALLOCATE name
DeallocateStmt *n = makeNode(DeallocateStmt);
n->name = $2;
+ n->isall = false;
+ n->location = @2;
$$ = (Node *) n;
}
| DEALLOCATE PREPARE name
@@ -11943,6 +11945,8 @@ DeallocateStmt: DEALLOCATE name
DeallocateStmt *n = makeNode(DeallocateStmt);
n->name = $3;
+ n->isall = false;
+ n->location = @3;
$$ = (Node *) n;
}
| DEALLOCATE ALL
@@ -11950,6 +11954,8 @@ DeallocateStmt: DEALLOCATE name
DeallocateStmt *n = makeNode(DeallocateStmt);
n->name = NULL;
+ n->isall = true;
+ n->location = -1;
$$ = (Node *) n;
}
| DEALLOCATE PREPARE ALL
@@ -11957,6 +11963,8 @@ DeallocateStmt: DEALLOCATE name
DeallocateStmt *n = makeNode(DeallocateStmt);
n->name = NULL;
+ n->isall = true;
+ n->location = -1;
$$ = (Node *) n;
}
;
diff --git a/contrib/pg_stat_statements/expected/utility.out b/contrib/pg_stat_statements/expected/utility.out
index 93735d5d85..f331044f3e 100644
--- 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 55b957d251..06b65aeef5 100644
--- 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 87666d9135..5f7d4a467f 100644
--- 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';
--
2.40.1
signature.asc
Description: PGP signature
