Hi Fujii-san,
Thank you for your reply and comment!
attach v3 fixed patch.
Shouldn't we also include calls and rows in the ORDER BY clause?
Without this, if there are multiple records with the same query
but different calls or rows, the query result might be unstable.
I believe this is causing the test failure reported by
he PostgreSQL Patch Tester.
I was thinking of adding "queryid <> 0" in the SELECT clause
instead of the WHERE clause. This way, we can verify if
the query results are as expected regardless of the queryid value,
including both queryid <> 0 and queryid = 0.
It's exactly as you said.
* Add calls and rows in the ORDER BY caluse.
* Modify "queryid <> 0" in the SELECT clause.
Modify test SQL belows, and the regress_stats_user1 check SQL
only needs to be done once.
+SELECT queryid <> 0 AS queryid_bool, query, calls, rows
+ FROM pg_stat_statements
+ ORDER BY query COLLATE "C", calls, rows;
Best Regards,
Keisuke Kuroda
NTT Comware
diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 414a30856e..c19ccad77e 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -19,7 +19,8 @@ LDFLAGS_SL += $(filter -lm, $(LIBS))
REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
REGRESS = select dml cursors utility level_tracking planning \
- user_activity wal entry_timestamp cleanup oldextversions
+ user_activity wal entry_timestamp privileges cleanup \
+ oldextversions
# Disabled because these tests require "shared_preload_libraries=pg_stat_statements",
# which typical installcheck users do not have (e.g. buildfarm clients).
NO_INSTALLCHECK = 1
diff --git a/contrib/pg_stat_statements/expected/privileges.out b/contrib/pg_stat_statements/expected/privileges.out
new file mode 100644
index 0000000000..55687f0256
--- /dev/null
+++ b/contrib/pg_stat_statements/expected/privileges.out
@@ -0,0 +1,87 @@
+--
+-- Superusers or roles with the privileges of pg_read_all_stats members
+-- can read query text and queryid
+--
+SET pg_stat_statements.track_utility = FALSE;
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t
+---
+ t
+(1 row)
+
+CREATE ROLE regress_stats_user1;
+CREATE ROLE regress_stats_user2;
+GRANT pg_read_all_stats TO regress_stats_user2;
+SELECT 1 AS "ONE";
+ ONE
+-----
+ 1
+(1 row)
+
+SET ROLE regress_stats_user1;
+SELECT 1+1 AS "TWO";
+ TWO
+-----
+ 2
+(1 row)
+
+-- Superuser can read query text and queryid
+RESET ROLE;
+SELECT queryid <> 0 AS queryid_bool, query, calls, rows
+ FROM pg_stat_statements
+ ORDER BY query COLLATE "C", calls, rows;
+ queryid_bool | query | calls | rows
+--------------+----------------------------------------------------+-------+------
+ t | SELECT $1 AS "ONE" | 1 | 1
+ t | SELECT $1+$2 AS "TWO" | 1 | 1
+ t | SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 | 1
+(3 rows)
+
+--
+-- regress_stats_user1 can not read query text and queryid
+-- executed by other users,
+-- but can read calls and rows
+--
+SET ROLE regress_stats_user1;
+SELECT queryid <> 0 AS queryid_bool, query, calls, rows
+ FROM pg_stat_statements
+ ORDER BY query COLLATE "C", calls, rows;
+ queryid_bool | query | calls | rows
+--------------+--------------------------+-------+------
+ | <insufficient privilege> | 1 | 1
+ | <insufficient privilege> | 1 | 1
+ | <insufficient privilege> | 1 | 3
+ t | SELECT $1+$2 AS "TWO" | 1 | 1
+(4 rows)
+
+RESET ROLE;
+-- regress_stats_user2 can read query text and queryid
+SET ROLE regress_stats_user2;
+SELECT queryid <> 0 AS queryid_bool, query, calls, rows
+ FROM pg_stat_statements
+ ORDER BY query COLLATE "C", calls, rows;
+ queryid_bool | query | calls | rows
+--------------+----------------------------------------------------------+-------+------
+ t | SELECT $1 AS "ONE" | 1 | 1
+ t | SELECT $1+$2 AS "TWO" | 1 | 1
+ t | SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 | 1
+ t | SELECT queryid <> $1 AS queryid_bool, query, calls, rows+| 1 | 3
+ | FROM pg_stat_statements +| |
+ | ORDER BY query COLLATE "C", calls, rows | |
+ t | SELECT queryid <> $1 AS queryid_bool, query, calls, rows+| 1 | 4
+ | FROM pg_stat_statements +| |
+ | ORDER BY query COLLATE "C", calls, rows | |
+(5 rows)
+
+--
+-- cleanup
+--
+RESET ROLE;
+DROP ROLE regress_stats_user1;
+DROP ROLE regress_stats_user2;
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t
+---
+ t
+(1 row)
+
diff --git a/contrib/pg_stat_statements/meson.build b/contrib/pg_stat_statements/meson.build
index 9bfc9657e1..5cf926d1f8 100644
--- a/contrib/pg_stat_statements/meson.build
+++ b/contrib/pg_stat_statements/meson.build
@@ -50,6 +50,7 @@ tests += {
'user_activity',
'wal',
'entry_timestamp',
+ 'privileges',
'cleanup',
'oldextversions',
],
diff --git a/contrib/pg_stat_statements/sql/privileges.sql b/contrib/pg_stat_statements/sql/privileges.sql
new file mode 100644
index 0000000000..74555530e9
--- /dev/null
+++ b/contrib/pg_stat_statements/sql/privileges.sql
@@ -0,0 +1,49 @@
+--
+-- Superusers or roles with the privileges of pg_read_all_stats members
+-- can read query text and queryid
+--
+
+SET pg_stat_statements.track_utility = FALSE;
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+CREATE ROLE regress_stats_user1;
+CREATE ROLE regress_stats_user2;
+GRANT pg_read_all_stats TO regress_stats_user2;
+SELECT 1 AS "ONE";
+
+SET ROLE regress_stats_user1;
+SELECT 1+1 AS "TWO";
+
+-- Superuser can read query text and queryid
+
+RESET ROLE;
+SELECT queryid <> 0 AS queryid_bool, query, calls, rows
+ FROM pg_stat_statements
+ ORDER BY query COLLATE "C", calls, rows;
+
+--
+-- regress_stats_user1 can not read query text and queryid
+-- executed by other users,
+-- but can read calls and rows
+--
+
+SET ROLE regress_stats_user1;
+SELECT queryid <> 0 AS queryid_bool, query, calls, rows
+ FROM pg_stat_statements
+ ORDER BY query COLLATE "C", calls, rows;
+RESET ROLE;
+
+-- regress_stats_user2 can read query text and queryid
+
+SET ROLE regress_stats_user2;
+SELECT queryid <> 0 AS queryid_bool, query, calls, rows
+ FROM pg_stat_statements
+ ORDER BY query COLLATE "C", calls, rows;
+
+--
+-- cleanup
+--
+
+RESET ROLE;
+DROP ROLE regress_stats_user1;
+DROP ROLE regress_stats_user2;
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
