On Sun, Aug 13, 2023 at 02:48:22PM +0800, Julien Rouhaud wrote: > On Sun, Aug 13, 2023 at 03:25:33PM +0900, Michael Paquier wrote: >> Perhaps not as much, actually, because I was just reminded that >> DEALLOCATE is something that pg_stat_statements ignores. So this >> makes harder the introduction of tests. > > Maybe it's time to revisit that? According to [1] the reason why > pg_stat_statements currently ignores DEALLOCATE is because it indeed bloated > the entries but also because at that time the suggestion for jumbling only > this > one was really hackish.
Good point. The argument of the other thread does not really hold much these days now that query jumbling can happen for all the utility nodes. > Now that we do have a sensible approach to jumble utility statements, I think > it would be beneficial to be able to track those, for instance to be easily > diagnose a driver that doesn't rely on the extended protocol. Fine by me. Would you like to write a patch? I've begun typing an embryon of patch a few days ago, and did not look yet at the rest. Please see the attached. >> Anyway, I guess that your own >> extension modules have a need for a query ID compiled with these >> fields ignored? > > My module has a need to track statements and still work efficiently after > that. > So anything that can lead to virtually an infinite number of > pg_stat_statements > entries is a problem for me, and I guess to all the other modules / tools that > track pg_stat_statements. I guess the reason why my module is still ignoring > DEALLOCATE is because it existed before pg 9.4 (with a homemade queryid as it > wasn't exposed before that), and it just stayed there since with the rest of > still problematic statements. Yeah, probably. -- Michael
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 2565348303..b7aeebe3b4 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3925,8 +3925,10 @@ 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 means DEALLOCATE ALL. */
+ char *name pg_node_attr(query_jumble_ignore);
+ /* 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..680c7f3683 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11936,6 +11936,7 @@ DeallocateStmt: DEALLOCATE name
DeallocateStmt *n = makeNode(DeallocateStmt);
n->name = $2;
+ n->location = @2;
$$ = (Node *) n;
}
| DEALLOCATE PREPARE name
@@ -11943,6 +11944,7 @@ DeallocateStmt: DEALLOCATE name
DeallocateStmt *n = makeNode(DeallocateStmt);
n->name = $3;
+ n->location = @3;
$$ = (Node *) n;
}
| DEALLOCATE ALL
@@ -11950,6 +11952,7 @@ DeallocateStmt: DEALLOCATE name
DeallocateStmt *n = makeNode(DeallocateStmt);
n->name = NULL;
+ n->location = -1;
$$ = (Node *) n;
}
| DEALLOCATE PREPARE ALL
@@ -11957,6 +11960,7 @@ DeallocateStmt: DEALLOCATE name
DeallocateStmt *n = makeNode(DeallocateStmt);
n->name = NULL;
+ 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..823d78802b 100644
--- a/contrib/pg_stat_statements/expected/utility.out
+++ b/contrib/pg_stat_statements/expected/utility.out
@@ -472,6 +472,45 @@ 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 | 2 | PREPARE stat_select AS SELECT $1 AS a
+ 1 | 1 | SELECT $1 as a
+ 1 | 1 | SELECT pg_stat_statements_reset()
+(3 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/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';
signature.asc
Description: PGP signature
