Jumble the CALL command in pg_stat_statements
Hi,
The proposal by Bertrand in CC to jumble CALL and SET in [1]/messages/by-id/36e5bffe-e989-194f-85c8-06e7bc88e6f7@amazon.com was
rejected at the time for a more robust solution to jumble DDL.
Michael also in CC made this possible with commit 3db72ebcbe.
The attached patch takes advantage of the jumbling infrastructure
added in the above mentioned commit and jumbles the CALL statement
in pg_stat_statements.
The patch also modifies existing test cases for CALL handling in pg_stat_statements
and adds additional tests which prove that a CALL to an overloaded procedure
will generate a different query_id.
As far as the SET command mentioned in [1]/messages/by-id/36e5bffe-e989-194f-85c8-06e7bc88e6f7@amazon.com is concerned, it is a bit more complex
as it requires us to deal with A_Constants which is not very straightforward. We can surely
deal with SET currently by applying custom query jumbling logic to VariableSetStmt,
but this can be dealt with in a separate discussion.
Regards,
Sami Imseih
Amazon Web Services (AWS)
[1]: /messages/by-id/36e5bffe-e989-194f-85c8-06e7bc88e6f7@amazon.com
Attachments:
0001-v1-Jumble-the-CALL-command-in-pg_stat_statements.patchapplication/octet-stream; name=0001-v1-Jumble-the-CALL-command-in-pg_stat_statements.patchDownload
From 45de0ff3bb5d555317b2e32f2de0100f2f89b4ff Mon Sep 17 00:00:00 2001
From: EC2 Default User <ec2-user@ip-172-31-44-253.ec2.internal>
Date: Wed, 13 Sep 2023 00:18:34 +0000
Subject: [PATCH 1/1] Jumble the CALL command in pg_stat_statements
3db72ebcbe introduced gen_node_support.pl for
An earlier proposal to jumble CALL and SET commands [1]
was rejected in lieu of a more comprehensive solution.
3db72ebcbe allowed gen_node_support.pl to support query jumbling.
This change takes advantage of the infrastructure introduced
in the aforementioned commit and jumbles the CALL command.
[1] https://www.postgresql.org/message-id/flat/36e5bffe-e989-194f-85c8-06e7bc88e6f7%40amazon.com
---
.../pg_stat_statements/expected/utility.out | 22 +++++++++++++++----
contrib/pg_stat_statements/sql/utility.sql | 14 ++++++++++++
src/include/nodes/parsenodes.h | 6 ++---
3 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/contrib/pg_stat_statements/expected/utility.out b/contrib/pg_stat_statements/expected/utility.out
index f331044f3e..1c2f085d3f 100644
--- a/contrib/pg_stat_statements/expected/utility.out
+++ b/contrib/pg_stat_statements/expected/utility.out
@@ -300,6 +300,18 @@ DECLARE
BEGIN
SELECT (i + j)::int INTO r;
END; $$ LANGUAGE plpgsql;
+CREATE OR REPLACE PROCEDURE overload(i int) AS $$
+DECLARE
+ r int;
+BEGIN
+ SELECT (i + i)::int INTO r;
+END; $$ LANGUAGE plpgsql;
+CREATE OR REPLACE PROCEDURE overload(i text) AS $$
+DECLARE
+ r text;
+BEGIN
+ SELECT i::text INTO r;
+END; $$ LANGUAGE plpgsql;
SELECT pg_stat_statements_reset();
pg_stat_statements_reset
--------------------------
@@ -310,13 +322,15 @@ CALL sum_one(3);
CALL sum_one(199);
CALL sum_two(1,1);
CALL sum_two(1,2);
+CALL overload(1);
+CALL overload('A');
SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
calls | rows | query
-------+------+-----------------------------------
- 1 | 0 | CALL sum_one(199)
- 1 | 0 | CALL sum_one(3)
- 1 | 0 | CALL sum_two(1,1)
- 1 | 0 | CALL sum_two(1,2)
+ 1 | 0 | CALL overload($1)
+ 1 | 0 | CALL overload($1)
+ 2 | 0 | CALL sum_one($1)
+ 2 | 0 | CALL sum_two($1,$2)
1 | 1 | SELECT pg_stat_statements_reset()
(5 rows)
diff --git a/contrib/pg_stat_statements/sql/utility.sql b/contrib/pg_stat_statements/sql/utility.sql
index 5f7d4a467f..81181207e5 100644
--- a/contrib/pg_stat_statements/sql/utility.sql
+++ b/contrib/pg_stat_statements/sql/utility.sql
@@ -164,11 +164,25 @@ DECLARE
BEGIN
SELECT (i + j)::int INTO r;
END; $$ LANGUAGE plpgsql;
+CREATE OR REPLACE PROCEDURE overload(i int) AS $$
+DECLARE
+ r int;
+BEGIN
+ SELECT (i + i)::int INTO r;
+END; $$ LANGUAGE plpgsql;
+CREATE OR REPLACE PROCEDURE overload(i text) AS $$
+DECLARE
+ r text;
+BEGIN
+ SELECT i::text INTO r;
+END; $$ LANGUAGE plpgsql;
SELECT pg_stat_statements_reset();
CALL sum_one(3);
CALL sum_one(199);
CALL sum_two(1,1);
CALL sum_two(1,2);
+CALL overload(1);
+CALL overload('A');
SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
-- COPY
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index fef4c714b8..993b545739 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3376,11 +3376,11 @@ typedef struct InlineCodeBlock
typedef struct CallStmt
{
NodeTag type;
- FuncCall *funccall; /* from the parser */
+ FuncCall *funccall pg_node_attr(query_jumble_ignore); /* from the parser */
/* transformed call, with only input args */
- FuncExpr *funcexpr pg_node_attr(query_jumble_ignore);
+ FuncExpr *funcexpr;
/* transformed output-argument expressions */
- List *outargs pg_node_attr(query_jumble_ignore);
+ List *outargs;
} CallStmt;
typedef struct CallContext
--
2.40.1
On Wed, Sep 13, 2023 at 12:48:48AM +0000, Imseih (AWS), Sami wrote:
The patch also modifies existing test cases for CALL handling in pg_stat_statements
and adds additional tests which prove that a CALL to an overloaded procedure
will generate a different query_id.
+CALL overload(1);
+CALL overload('A');
[...]
+ 1 | 0 | CALL overload($1)
+ 1 | 0 | CALL overload($1)That's not surprising to me. We've historically relied on the
function OID in the jumbling of a FuncExpr, so I'm OK with that. This
may look a bit surprising though if you have a schema that enforces
the same function name for several data types. Having a DEFAULT does
this:
CREATE OR REPLACE PROCEDURE overload(i text, j bool DEFAULT true) AS
$$ DECLARE
r text;
BEGIN
SELECT i::text INTO r;
END; $$ LANGUAGE plpgsql;
Then with these three, and a jumbling based on the OID gives:
+CALL overload(1);
+CALL overload('A');
+CALL overload('A', false);
[...]
- 1 | 0 | CALL overload($1)
+ 2 | 0 | CALL overload($1)Still this grouping is much better than having thousands of entries
with different values. I am not sure if we should bother improving
that more than what you suggest that, especially as FuncExpr->args can
itself include Const nodes as far as I recall.
As far as the SET command mentioned in [1] is concerned, it is a bit more complex
as it requires us to deal with A_Constants which is not very straightforward. We can surely
deal with SET currently by applying custom query jumbling logic to VariableSetStmt,
but this can be dealt with in a separate discussion.
As VariableSetStmt is the top-most node structure for SET/RESET
commands, using a custom implementation may be wise in this case,
particularly for the args made of A_Const. I don't really want to go
down to the internals of A_Const outside its internal implementation,
as these can be used for some queries where there are multiple
keywords separated by whitespaces for one single A_Const, like
isolation level values in transaction commands. This would lead to
appending the dollar-based variables in weird ways for some patterns.
Cough.
/* transformed output-argument expressions */
- List *outargs pg_node_attr(query_jumble_ignore);
+ List *outargs;This choice is a bit surprising. How does it influence the jumbling?
For example, if I add a query_jumble_ignore to it, the regression
tests of pg_stat_statements still pass. This is going to require more
test coverage to prove that this addition is useful.
--
Michael
Still this grouping is much better than having thousands of entries
with different values. I am not sure if we should bother improving
that more than what you suggest that, especially as FuncExpr->args can
itself include Const nodes as far as I recall.
I agree.
As far as the SET command mentioned in [1] is concerned, it is a bit more complex
as it requires us to deal with A_Constants which is not very straightforward. We can surely
deal with SET currently by applying custom query jumbling logic to VariableSetStmt,
but this can be dealt with in a separate discussion.
As VariableSetStmt is the top-most node structure for SET/RESET
commands, using a custom implementation may be wise in this case,
I do have a patch for this with test cases, 0001-v1-Jumble-the-SET-command.patch
If you feel this needs a separate discussion I can start one.
In the patch, the custom _jumbleVariableSetStmt jumbles
the kind, name, is_local and number of arguments ( in case of a list )
and tracks the locations for normalization.
This choice is a bit surprising. How does it influence the jumbling?
For example, if I add a query_jumble_ignore to it, the regression
tests of pg_stat_statements still pass. This is going to require more
test coverage to prove that this addition is useful.
CALL with OUT or INOUT args is a bit strange, because
as the doc [1]https://www.postgresql.org/docs/current/sql-call.html mentions "Arguments must be supplied for all procedure parameters
that lack defaults, including OUT parameters. However, arguments
matching OUT parameters are not evaluated, so it's customary
to just write NULL for them."
so for pgss, passing a NULL or some other value into OUT/INOUT args should
be normalized like IN args.
0001-v2-Jumble-the-CALL-command-in-pg_stat_statements.patch adds
these test cases.
Regards,
Sami Imseih
Amazon Web Services (AWS)
Attachments:
0001-v1-Jumble-the-SET-command.patchapplication/octet-stream; name=0001-v1-Jumble-the-SET-command.patchDownload
From ffa3a7cdbb5bce708a59c88a5666e322afbcfc8e Mon Sep 17 00:00:00 2001
From: EC2 Default User <ec2-user@ip-172-31-44-253.ec2.internal>
Date: Wed, 13 Sep 2023 21:06:09 +0000
Subject: [PATCH 1/1] Jumble the SET command
---
contrib/pg_stat_statements/expected/dml.out | 2 +-
.../expected/level_tracking.out | 20 ++++----
.../pg_stat_statements/expected/utility.out | 46 ++++++++++++++-----
contrib/pg_stat_statements/expected/wal.out | 2 +-
contrib/pg_stat_statements/sql/utility.sql | 17 ++++++-
src/backend/nodes/queryjumblefuncs.c | 29 ++++++++++++
src/include/nodes/parsenodes.h | 2 +
7 files changed, 94 insertions(+), 24 deletions(-)
diff --git a/contrib/pg_stat_statements/expected/dml.out b/contrib/pg_stat_statements/expected/dml.out
index 7b9c8f979e..62c6ecd94d 100644
--- a/contrib/pg_stat_statements/expected/dml.out
+++ b/contrib/pg_stat_statements/expected/dml.out
@@ -82,7 +82,7 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
2 | 4 | SELECT * FROM pgss_dml_tab WHERE a > $1 ORDER BY a
1 | 8 | SELECT * FROM pgss_dml_tab WHERE a IN ($1, $2, $3, $4, $5)
1 | 1 | SELECT pg_stat_statements_reset()
- 1 | 0 | SET pg_stat_statements.track_utility = FALSE
+ 1 | 0 | SET pg_stat_statements.track_utility = $1
6 | 6 | UPDATE pgss_dml_tab SET b = $1 WHERE a = $2
1 | 3 | UPDATE pgss_dml_tab SET b = $1 WHERE a > $2
(10 rows)
diff --git a/contrib/pg_stat_statements/expected/level_tracking.out b/contrib/pg_stat_statements/expected/level_tracking.out
index d924c87b41..9a67d14a5a 100644
--- a/contrib/pg_stat_statements/expected/level_tracking.out
+++ b/contrib/pg_stat_statements/expected/level_tracking.out
@@ -49,22 +49,22 @@ BEGIN
END; $$;
SELECT toplevel, calls, query FROM pg_stat_statements
ORDER BY query COLLATE "C", toplevel;
- toplevel | calls | query
-----------+-------+--------------------------------------
+ toplevel | calls | query
+----------+-------+-----------------------------------
f | 1 | DELETE FROM stats_track_tab
t | 1 | DELETE FROM stats_track_tab
- t | 1 | DO $$ +
- | | BEGIN +
- | | DELETE FROM stats_track_tab; +
+ t | 1 | DO $$ +
+ | | BEGIN +
+ | | DELETE FROM stats_track_tab; +
| | END; $$
- t | 1 | DO LANGUAGE plpgsql $$ +
- | | BEGIN +
- | | -- this is a SELECT +
- | | PERFORM 'hello world'::TEXT; +
+ t | 1 | DO LANGUAGE plpgsql $$ +
+ | | BEGIN +
+ | | -- this is a SELECT +
+ | | PERFORM 'hello world'::TEXT; +
| | END; $$
f | 1 | SELECT $1::TEXT
t | 1 | SELECT pg_stat_statements_reset()
- t | 1 | SET pg_stat_statements.track = 'all'
+ t | 1 | SET pg_stat_statements.track = $1
(7 rows)
-- PL/pgSQL function - top-level tracking.
diff --git a/contrib/pg_stat_statements/expected/utility.out b/contrib/pg_stat_statements/expected/utility.out
index f3bc9fcb7c..06169c37d4 100644
--- a/contrib/pg_stat_statements/expected/utility.out
+++ b/contrib/pg_stat_statements/expected/utility.out
@@ -583,11 +583,9 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
1 | 0 | SET TRANSACTION ISOLATION LEVEL READ COMMITTED
1 | 0 | SET TRANSACTION ISOLATION LEVEL REPEATABLE READ
1 | 0 | SET TRANSACTION ISOLATION LEVEL SERIALIZABLE
- 1 | 0 | SET enable_seqscan = off
- 1 | 0 | SET enable_seqscan = on
- 2 | 0 | SET work_mem = '1MB'
- 1 | 0 | SET work_mem = '2MB'
-(15 rows)
+ 2 | 0 | SET enable_seqscan = $1
+ 3 | 0 | SET work_mem = $1
+(13 rows)
SELECT pg_stat_statements_reset();
pg_stat_statements_reset
@@ -665,23 +663,49 @@ SELECT pg_stat_statements_reset();
-- SET statements.
-- These use two different strings, still they count as one entry.
SET work_mem = '1MB';
-Set work_mem = '1MB';
+SET work_mem = '1MB';
SET work_mem = '2MB';
RESET work_mem;
SET enable_seqscan = off;
SET enable_seqscan = on;
+SET enable_seqscan TO DEFAULT;
+SET enable_seqscan FROM CURRENT;
RESET enable_seqscan;
+BEGIN;
+SET LOCAL enable_seqscan = off;
+SET LOCAL enable_seqscan TO off;
+SET TIME ZONE LOCAL;
+SET TIME ZONE 'PST8PDT';
+SET LOCAL TIME ZONE LOCAL;
+SET LOCAL TIME ZONE 'PST8PDT';
+END;
+SET search_path = 'public';
+SET search_path = 'public', a;
+SET search_path = 'public', 'a';
+SET search_path = 'public', 'a', 'c';
+RESET search_path;
SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
calls | rows | query
-------+------+-----------------------------------
+ 1 | 0 | BEGIN
+ 1 | 0 | END
1 | 0 | RESET enable_seqscan
+ 1 | 0 | RESET search_path
1 | 0 | RESET work_mem
1 | 1 | SELECT pg_stat_statements_reset()
- 1 | 0 | SET enable_seqscan = off
- 1 | 0 | SET enable_seqscan = on
- 2 | 0 | SET work_mem = '1MB'
- 1 | 0 | SET work_mem = '2MB'
-(7 rows)
+ 1 | 0 | SET LOCAL TIME ZONE $1
+ 1 | 0 | SET LOCAL TIME ZONE LOCAL
+ 2 | 0 | SET LOCAL enable_seqscan = $1
+ 1 | 0 | SET TIME ZONE $1
+ 1 | 0 | SET TIME ZONE LOCAL
+ 2 | 0 | SET enable_seqscan = $1
+ 1 | 0 | SET enable_seqscan FROM CURRENT
+ 1 | 0 | SET enable_seqscan TO DEFAULT
+ 1 | 0 | SET search_path = $1
+ 2 | 0 | SET search_path = $1, $2
+ 1 | 0 | SET search_path = $1, $2, $3
+ 3 | 0 | SET work_mem = $1
+(18 rows)
SELECT pg_stat_statements_reset();
pg_stat_statements_reset
diff --git a/contrib/pg_stat_statements/expected/wal.out b/contrib/pg_stat_statements/expected/wal.out
index 9896ba2536..ce9df00433 100644
--- a/contrib/pg_stat_statements/expected/wal.out
+++ b/contrib/pg_stat_statements/expected/wal.out
@@ -18,7 +18,7 @@ FROM pg_stat_statements ORDER BY query COLLATE "C";
DELETE FROM pgss_wal_tab WHERE a > $1 | 1 | 1 | t | t | t
INSERT INTO pgss_wal_tab VALUES(generate_series($1, $2), $3) | 1 | 10 | t | t | t
SELECT pg_stat_statements_reset() | 1 | 1 | f | f | f
- SET pg_stat_statements.track_utility = FALSE | 1 | 0 | f | f | t
+ SET pg_stat_statements.track_utility = $1 | 1 | 0 | f | f | t
UPDATE pgss_wal_tab SET b = $1 WHERE a > $2 | 1 | 3 | t | t | t
(5 rows)
diff --git a/contrib/pg_stat_statements/sql/utility.sql b/contrib/pg_stat_statements/sql/utility.sql
index 28fd13dee0..ad4c296a4e 100644
--- a/contrib/pg_stat_statements/sql/utility.sql
+++ b/contrib/pg_stat_statements/sql/utility.sql
@@ -331,12 +331,27 @@ SELECT pg_stat_statements_reset();
-- SET statements.
-- These use two different strings, still they count as one entry.
SET work_mem = '1MB';
-Set work_mem = '1MB';
+SET work_mem = '1MB';
SET work_mem = '2MB';
RESET work_mem;
SET enable_seqscan = off;
SET enable_seqscan = on;
+SET enable_seqscan TO DEFAULT;
+SET enable_seqscan FROM CURRENT;
RESET enable_seqscan;
+BEGIN;
+SET LOCAL enable_seqscan = off;
+SET LOCAL enable_seqscan TO off;
+SET TIME ZONE LOCAL;
+SET TIME ZONE 'PST8PDT';
+SET LOCAL TIME ZONE LOCAL;
+SET LOCAL TIME ZONE 'PST8PDT';
+END;
+SET search_path = 'public';
+SET search_path = 'public', a;
+SET search_path = 'public', 'a';
+SET search_path = 'public', 'a', 'c';
+RESET search_path;
SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
SELECT pg_stat_statements_reset();
diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c
index 281907a4d8..afc2aa44c1 100644
--- a/src/backend/nodes/queryjumblefuncs.c
+++ b/src/backend/nodes/queryjumblefuncs.c
@@ -52,6 +52,7 @@ static void _jumbleNode(JumbleState *jstate, Node *node);
static void _jumbleA_Const(JumbleState *jstate, Node *node);
static void _jumbleList(JumbleState *jstate, Node *node);
static void _jumbleRangeTblEntry(JumbleState *jstate, Node *node);
+static void _jumbleVariableSetStmt(JumbleState *jstate, Node *node);
/*
* Given a possibly multi-statement source string, confine our attention to the
@@ -395,3 +396,31 @@ _jumbleRangeTblEntry(JumbleState *jstate, Node *node)
break;
}
}
+
+static void
+_jumbleVariableSetStmt(JumbleState *jstate, Node *node)
+{
+ VariableSetStmt *expr = (VariableSetStmt *) node;
+ ListCell *l;
+
+ /* We don't need to normalize SET TRANSACTION */
+ if (expr->kind == VAR_SET_MULTI)
+ JUMBLE_NODE(args);
+ else
+ {
+ int num_args = list_length(expr->args);
+
+ JUMBLE_FIELD(kind);
+ JUMBLE_STRING(name);
+ JUMBLE_FIELD(is_local);
+ JUMBLE_FIELD_SINGLE(num_args);
+
+ foreach (l, expr->args)
+ {
+ A_Const *ac = (A_Const *) lfirst(l);
+
+ if(ac->type != T_String)
+ RecordConstLocation(jstate, ac->location);
+ }
+ }
+}
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 993b545739..7787bd2439 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2478,6 +2478,8 @@ typedef enum VariableSetKind
typedef struct VariableSetStmt
{
+ pg_node_attr(custom_query_jumble)
+
NodeTag type;
VariableSetKind kind;
char *name; /* variable to be set */
--
2.40.1
0001-v2-Jumble-the-CALL-command-in-pg_stat_statements.patchapplication/octet-stream; name=0001-v2-Jumble-the-CALL-command-in-pg_stat_statements.patchDownload
From 47bcce494e741922803581a24306c0748419d891 Mon Sep 17 00:00:00 2001
From: EC2 Default User <ec2-user@ip-172-31-44-253.ec2.internal>
Date: Wed, 13 Sep 2023 00:18:34 +0000
Subject: [PATCH 1/1] Jumble the CALL command in pg_stat_statements
3db72ebcbe introduced gen_node_support.pl for
An earlier proposal to jumble CALL and SET commands [1]
was rejected in lieu of a more comprehensive solution.
3db72ebcbe allowed gen_node_support.pl to support query jumbling.
This change takes advantage of the infrastructure introduced
in the aforementioned commit and jumbles the CALL command.
[1] https://www.postgresql.org/message-id/flat/36e5bffe-e989-194f-85c8-06e7bc88e6f7%40amazon.com
Author: Sami Imseih
Reviewed-by: Michael Pacquier
Discussion: https://www.postgresql.org/message-id/flat/B44FA29D-EBD0-4DD9-ABC2-16F1CB087074%40amazon.com
---
.../pg_stat_statements/expected/utility.out | 44 ++++++++++++++++---
contrib/pg_stat_statements/sql/utility.sql | 23 ++++++++++
src/include/nodes/parsenodes.h | 6 +--
3 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/contrib/pg_stat_statements/expected/utility.out b/contrib/pg_stat_statements/expected/utility.out
index f331044f3e..f3bc9fcb7c 100644
--- a/contrib/pg_stat_statements/expected/utility.out
+++ b/contrib/pg_stat_statements/expected/utility.out
@@ -300,6 +300,25 @@ DECLARE
BEGIN
SELECT (i + j)::int INTO r;
END; $$ LANGUAGE plpgsql;
+CREATE OR REPLACE PROCEDURE overload(i int) AS $$
+DECLARE
+ r int;
+BEGIN
+ SELECT (i + i)::int INTO r;
+END; $$ LANGUAGE plpgsql;
+CREATE OR REPLACE PROCEDURE overload(i text) AS $$
+DECLARE
+ r text;
+BEGIN
+ SELECT i::text INTO r;
+END; $$ LANGUAGE plpgsql;
+CREATE OR REPLACE PROCEDURE in_out(i int, i2 OUT int, i3 INOUT int) AS $$
+DECLARE
+ r int;
+BEGIN
+ i2 := i;
+ i3 := i3 + i;
+END; $$ LANGUAGE plpgsql;
SELECT pg_stat_statements_reset();
pg_stat_statements_reset
--------------------------
@@ -310,15 +329,30 @@ CALL sum_one(3);
CALL sum_one(199);
CALL sum_two(1,1);
CALL sum_two(1,2);
+CALL overload(1);
+CALL overload('A');
+CALL in_out(1, NULL, 1);
+ i2 | i3
+----+----
+ 1 | 2
+(1 row)
+
+CALL in_out(2, 1, 2);
+ i2 | i3
+----+----
+ 2 | 4
+(1 row)
+
SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
calls | rows | query
-------+------+-----------------------------------
- 1 | 0 | CALL sum_one(199)
- 1 | 0 | CALL sum_one(3)
- 1 | 0 | CALL sum_two(1,1)
- 1 | 0 | CALL sum_two(1,2)
+ 2 | 0 | CALL in_out($1, $2, $3)
+ 1 | 0 | CALL overload($1)
+ 1 | 0 | CALL overload($1)
+ 2 | 0 | CALL sum_one($1)
+ 2 | 0 | CALL sum_two($1,$2)
1 | 1 | SELECT pg_stat_statements_reset()
-(5 rows)
+(6 rows)
-- COPY
CREATE TABLE copy_stats (a int, b int);
diff --git a/contrib/pg_stat_statements/sql/utility.sql b/contrib/pg_stat_statements/sql/utility.sql
index 5f7d4a467f..28fd13dee0 100644
--- a/contrib/pg_stat_statements/sql/utility.sql
+++ b/contrib/pg_stat_statements/sql/utility.sql
@@ -164,11 +164,34 @@ DECLARE
BEGIN
SELECT (i + j)::int INTO r;
END; $$ LANGUAGE plpgsql;
+CREATE OR REPLACE PROCEDURE overload(i int) AS $$
+DECLARE
+ r int;
+BEGIN
+ SELECT (i + i)::int INTO r;
+END; $$ LANGUAGE plpgsql;
+CREATE OR REPLACE PROCEDURE overload(i text) AS $$
+DECLARE
+ r text;
+BEGIN
+ SELECT i::text INTO r;
+END; $$ LANGUAGE plpgsql;
+CREATE OR REPLACE PROCEDURE in_out(i int, i2 OUT int, i3 INOUT int) AS $$
+DECLARE
+ r int;
+BEGIN
+ i2 := i;
+ i3 := i3 + i;
+END; $$ LANGUAGE plpgsql;
SELECT pg_stat_statements_reset();
CALL sum_one(3);
CALL sum_one(199);
CALL sum_two(1,1);
CALL sum_two(1,2);
+CALL overload(1);
+CALL overload('A');
+CALL in_out(1, NULL, 1);
+CALL in_out(2, 1, 2);
SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
-- COPY
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index fef4c714b8..993b545739 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3376,11 +3376,11 @@ typedef struct InlineCodeBlock
typedef struct CallStmt
{
NodeTag type;
- FuncCall *funccall; /* from the parser */
+ FuncCall *funccall pg_node_attr(query_jumble_ignore); /* from the parser */
/* transformed call, with only input args */
- FuncExpr *funcexpr pg_node_attr(query_jumble_ignore);
+ FuncExpr *funcexpr;
/* transformed output-argument expressions */
- List *outargs pg_node_attr(query_jumble_ignore);
+ List *outargs;
} CallStmt;
typedef struct CallContext
--
2.40.1
On Wed, Sep 13, 2023 at 11:09:19PM +0000, Imseih (AWS), Sami wrote:
I do have a patch for this with test cases, 0001-v1-Jumble-the-SET-command.patch
If you feel this needs a separate discussion I can start one.
Agreed tha tthis should have its own thread with a proper subject.
In the patch, the custom _jumbleVariableSetStmt jumbles
the kind, name, is_local and number of arguments ( in case of a list )
and tracks the locations for normalization.
There is much more going on here, like FunctionSetResetClause, or
AlterSystemStmt with its generic_reset.
+ foreach (l, expr->args)
+ {
+ A_Const *ac = (A_Const *) lfirst(l);
+
+ if(ac->type != T_String)
+ RecordConstLocation(jstate, ac->location);
+ }Even this part, I am not sure if it is always correct. Couldn't we
have cases where String's A_Const had better be recorded as const?
CALL with OUT or INOUT args is a bit strange, because
as the doc [1] mentions "Arguments must be supplied for all procedure parameters
that lack defaults, including OUT parameters. However, arguments
matching OUT parameters are not evaluated, so it's customary
to just write NULL for them."so for pgss, passing a NULL or some other value into OUT/INOUT args should
be normalized like IN args.
I've been studying this one, and I can see why you're right here.
This feels much more natural to include. The INOUT parameters get
registered twice at the same position, and the duplicates are
discarded by pg_stat_statements, which is OK. The patch is straight
for the CALL part, so I have applied it.
--
Michael