Thread: Ignore 2PC transaction GIDs in query jumbling
Hi all, 31de7e6 has silenced savepoint names in the query jumbling, and something similar can be done for 2PC transactions once the GID is ignored in TransactionStmt. This leads to the following grouping in pg_stat_statements, for instance, which is something that matters with workloads that heavily rely on 2PC: COMMIT PREPARED $1 PREPARE TRANSACTION $1 ROLLBACK PREPARED $1 Thoughts or comments? -- Michael
Attachment
Hi, On Tue, Aug 01, 2023 at 09:38:14AM +0900, Michael Paquier wrote: > > 31de7e6 has silenced savepoint names in the query jumbling, and > something similar can be done for 2PC transactions once the GID is > ignored in TransactionStmt. This leads to the following grouping in > pg_stat_statements, for instance, which is something that matters with > workloads that heavily rely on 2PC: > COMMIT PREPARED $1 > PREPARE TRANSACTION $1 > ROLLBACK PREPARED $1 Having an application relying on 2pc leads to pg_stat_statements being virtually unusable on the whole instance, so +1 for the patch. FTR we had to entirely ignore all those statements in powa years ago to try to make the tool usable in such case for some users who where using 2pc, it would be nice to be able to track them back for pg16+. The patch LGTM.
On Tue, Aug 01, 2023 at 09:28:08AM +0800, Julien Rouhaud wrote: > Having an application relying on 2pc leads to pg_stat_statements being > virtually unusable on the whole instance, so +1 for the patch. Cool, thanks for the feedback! > FTR we had to entirely ignore all those statements in powa years ago to try to > make the tool usable in such case for some users who where using 2pc, it would > be nice to be able to track them back for pg16+. That would be nice for your users. Are there more query patterns you'd be interested in grouping in the backend? I had a few folks aiming for CallStmt and SetStmt, but both a a bit tricky without a custom routine. -- Michael
Attachment
On Tue, Aug 01, 2023 at 11:00:32AM +0900, Michael Paquier wrote: > On Tue, Aug 01, 2023 at 09:28:08AM +0800, Julien Rouhaud wrote: > > > FTR we had to entirely ignore all those statements in powa years ago to try to > > make the tool usable in such case for some users who where using 2pc, it would > > be nice to be able to track them back for pg16+. > > That would be nice for your users. Indeed, although I will need to make that part a runtime variable based on the server version rather than a hardcoded string since the extension framework doesn't provide a way to do that cleanly across major pg versions. > Are there more query patterns you'd > be interested in grouping in the backend? I had a few folks aiming > for CallStmt and SetStmt, but both a a bit tricky without a custom > routine. Looking at the rest of the ignored patterns, the only remaining one would be DEALLOCATE, which AFAICS doesn't have a query_jumble_ignore tag for now.
On Tue, Aug 01, 2023 at 10:22:09AM +0800, Julien Rouhaud wrote: > Looking at the rest of the ignored patterns, the only remaining one would be > DEALLOCATE, which AFAICS doesn't have a query_jumble_ignore tag for now. This one seems to be simple as well with a location field, looking quickly at its Node. -- Michael
Attachment
On Tue, Aug 01, 2023 at 11:37:49AM +0900, Michael Paquier wrote: > On Tue, Aug 01, 2023 at 10:22:09AM +0800, Julien Rouhaud wrote: > > Looking at the rest of the ignored patterns, the only remaining one would be > > DEALLOCATE, which AFAICS doesn't have a query_jumble_ignore tag for now. > > This one seems to be simple as well with a location field, looking > quickly at its Node. Agreed, it should be as trivial to implement as for the 2pc commands :)
On Tue, Aug 01, 2023 at 10:46:58AM +0800, Julien Rouhaud wrote: > On Tue, Aug 01, 2023 at 11:37:49AM +0900, Michael Paquier wrote: >> On Tue, Aug 01, 2023 at 10:22:09AM +0800, Julien Rouhaud wrote: >>> Looking at the rest of the ignored patterns, the only remaining one would be >>> DEALLOCATE, which AFAICS doesn't have a query_jumble_ignore tag for now. >> >> This one seems to be simple as well with a location field, looking >> quickly at its Node. > > Agreed, it should be as trivial to implement as for the 2pc commands :) 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. Anyway, I guess that your own extension modules have a need for a query ID compiled with these fields ignored? For now, I have applied the 2PC bits independently, as of 638d42a. -- Michael
Attachment
On Sun, Aug 13, 2023 at 03:25:33PM +0900, Michael Paquier wrote: > On Tue, Aug 01, 2023 at 10:46:58AM +0800, Julien Rouhaud wrote: > > > > Agreed, it should be as trivial to implement as for the 2pc commands :) > > 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. 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. > 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. > For now, I have applied the 2PC bits independently, as of 638d42a. Thanks! [1] https://www.postgresql.org/message-id/flat/alpine.DEB.2.10.1404011631560.2557%40sto
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
Attachment
Michael Paquier <michael@paquier.xyz> writes:
> 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.
As far as I could tell the only thing missing was removing
DeallocateStmt from the list of unhandled utility statement types (and
updating comments to match). Updated patch attached.
- ilmari
From 7f11e362ef8c097a78463435a4bd1ab1031ea233 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 14 Aug 2023 11:59:44 +0100
Subject: [PATCH] Track DEALLOCATE statements in pg_stat_activity
Treat the statement name as a constant when jumbling.
---
.../pg_stat_statements/expected/utility.out | 40 +++++++++++++++++++
.../pg_stat_statements/pg_stat_statements.c | 8 +---
contrib/pg_stat_statements/sql/utility.sql | 13 ++++++
src/backend/parser/gram.y | 4 ++
src/include/nodes/parsenodes.h | 6 ++-
5 files changed, 63 insertions(+), 8 deletions(-)
diff --git a/contrib/pg_stat_statements/expected/utility.out b/contrib/pg_stat_statements/expected/utility.out
index 93735d5d85..3681374869 100644
--- a/contrib/pg_stat_statements/expected/utility.out
+++ b/contrib/pg_stat_statements/expected/utility.out
@@ -472,6 +472,46 @@ 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
+-------+------+---------------------------------------
+ 4 | 0 | DEALLOCATE $1
+ 2 | 2 | PREPARE stat_select AS SELECT $1 AS a
+ 1 | 1 | SELECT $1 as a
+ 1 | 1 | SELECT pg_stat_statements_reset()
+(4 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';
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/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;
/*
--
2.39.2
On Mon, Aug 14, 2023 at 12:11:13PM +0100, Dagfinn Ilmari Mannsåker wrote: > As far as I could tell the only thing missing was removing > DeallocateStmt from the list of unhandled utility statement types (and > updating comments to match). Updated patch attached. 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? -- Michael
Attachment
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
Attachment
Michael Paquier <michael@paquier.xyz> writes:
> 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?
I don't have a particularly strong opinion on whether we should
distinguish DEALLOCATE ALL from DEALLOCATE <stmt> (call it +0.5), but
this seems like a reasonable way to do it unless we want to invent a
query_jumble_ignore_unless_null attribute (which seems like overkill for
this one case).
- ilmari
> Michael
>
> From ad91672367f408663824b36b6dd517513d7f0a2a Mon Sep 17 00:00:00 2001
> From: Michael Paquier <michael@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';
On Fri, Aug 18, 2023 at 11:31:03AM +0100, Dagfinn Ilmari Mannsåker wrote: > I don't have a particularly strong opinion on whether we should > distinguish DEALLOCATE ALL from DEALLOCATE <stmt> (call it +0.5), but The difference looks important to me, especially for monitoring. And pgbouncer may also use both of them, actually? (Somebody, please correct me here if necessary.) > this seems like a reasonable way to do it unless we want to invent a > query_jumble_ignore_unless_null attribute (which seems like overkill for > this one case). I don't really want to have a NULL-based property for that :) -- Michael
Attachment
On Sat, Aug 19, 2023 at 01:47:48PM +0900, Michael Paquier wrote: > On Fri, Aug 18, 2023 at 11:31:03AM +0100, Dagfinn Ilmari Mannsåker wrote: >> I don't have a particularly strong opinion on whether we should >> distinguish DEALLOCATE ALL from DEALLOCATE <stmt> (call it +0.5), but > > The difference looks important to me, especially for monitoring. After thinking a few days about it, I'd rather still make the difference between both, so applied this way. > And pgbouncer may also use both of them, actually? (Somebody, please > correct me here if necessary.) I cannot see any signs of that in pgbouncer, btw. -- Michael