On 2024/07/23 15:02, kuroda.keis...@nttcom.co.jp wrote:
Hi Fujii-san,
Thank you for your comment!
attach v5 fixed patch.
Using "postgres" as the default superuser name can cause instability.
This is why the Patch Tester reports now test failures again.
You should create and use a different superuser, such as
"regress_stats_superuser."
I understand.
Add "regress_stats_superuser" for test by superuser.
Thanks for updating the patch!
I've slightly modified the comments in the regression tests for clarity.
Attached is the v6 patch. If there are no objections,
I will push this version.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From d2dfc611750bce0b06ba3a3f82b136014d399987 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fu...@postgresql.org>
Date: Wed, 24 Jul 2024 02:50:35 +0900
Subject: [PATCH v6] pg_stat_statements: Add regression test for privilege
handling.
This commit adds a regression test to verify that pg_stat_statements
correctly handles privileges, improving its test coverage.
Author: Keisuke Kuroda
Reviewed-by: Michael Paquier, Fujii Masao
Discussion: https://postgr.es/m/2224ccf2e12c41ccb81702ef3303d...@nttcom.co.jp
---
contrib/pg_stat_statements/Makefile | 3 +-
.../expected/privileges.out | 97 +++++++++++++++++++
contrib/pg_stat_statements/meson.build | 1 +
contrib/pg_stat_statements/sql/privileges.sql | 60 ++++++++++++
4 files changed, 160 insertions(+), 1 deletion(-)
create mode 100644 contrib/pg_stat_statements/expected/privileges.out
create mode 100644 contrib/pg_stat_statements/sql/privileges.sql
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..daaaa0bb83
--- /dev/null
+++ b/contrib/pg_stat_statements/expected/privileges.out
@@ -0,0 +1,97 @@
+--
+-- Only superusers and roles with privileges of the pg_read_all_stats role
+-- are allowed to see the SQL text and queryid of queries executed by
+-- other users. Other users can see the statistics.
+--
+SET pg_stat_statements.track_utility = FALSE;
+CREATE ROLE regress_stats_superuser SUPERUSER;
+CREATE ROLE regress_stats_user1;
+CREATE ROLE regress_stats_user2;
+GRANT pg_read_all_stats TO regress_stats_user2;
+SET ROLE regress_stats_superuser;
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t
+---
+ t
+(1 row)
+
+SELECT 1 AS "ONE";
+ ONE
+-----
+ 1
+(1 row)
+
+SET ROLE regress_stats_user1;
+SELECT 1+1 AS "TWO";
+ TWO
+-----
+ 2
+(1 row)
+
+--
+-- A superuser can read all columns of queries executed by others,
+-- including query text and queryid.
+--
+SET ROLE regress_stats_superuser;
+SELECT r.rolname, ss.queryid <> 0 AS queryid_bool, ss.query, ss.calls, ss.rows
+ FROM pg_stat_statements ss JOIN pg_roles r ON ss.userid = r.oid
+ ORDER BY r.rolname, ss.query COLLATE "C", ss.calls, ss.rows;
+ rolname | queryid_bool | query
| calls | rows
+-------------------------+--------------+----------------------------------------------------+-------+------
+ regress_stats_superuser | t | SELECT $1 AS "ONE"
| 1 | 1
+ regress_stats_superuser | t | SELECT pg_stat_statements_reset() IS
NOT NULL AS t | 1 | 1
+ regress_stats_user1 | t | SELECT $1+$2 AS "TWO"
| 1 | 1
+(3 rows)
+
+--
+-- regress_stats_user1 has no privileges to read the query text or
+-- queryid of queries executed by others but can see statistics
+-- like calls and rows.
+--
+SET ROLE regress_stats_user1;
+SELECT r.rolname, ss.queryid <> 0 AS queryid_bool, ss.query, ss.calls, ss.rows
+ FROM pg_stat_statements ss JOIN pg_roles r ON ss.userid = r.oid
+ ORDER BY r.rolname, ss.query COLLATE "C", ss.calls, ss.rows;
+ rolname | queryid_bool | query | calls |
rows
+-------------------------+--------------+--------------------------+-------+------
+ regress_stats_superuser | | <insufficient privilege> | 1 |
1
+ regress_stats_superuser | | <insufficient privilege> | 1 |
1
+ regress_stats_superuser | | <insufficient privilege> | 1 |
3
+ regress_stats_user1 | t | SELECT $1+$2 AS "TWO" | 1 |
1
+(4 rows)
+
+--
+-- regress_stats_user2, with pg_read_all_stats role privileges, can
+-- read all columns, including query text and queryid, of queries
+-- executed by others.
+--
+SET ROLE regress_stats_user2;
+SELECT r.rolname, ss.queryid <> 0 AS queryid_bool, ss.query, ss.calls, ss.rows
+ FROM pg_stat_statements ss JOIN pg_roles r ON ss.userid = r.oid
+ ORDER BY r.rolname, ss.query COLLATE "C", ss.calls, ss.rows;
+ rolname | queryid_bool |
query | calls | rows
+-------------------------+--------------+---------------------------------------------------------------------------------+-------+------
+ regress_stats_superuser | t | SELECT $1 AS "ONE"
| 1 | 1
+ regress_stats_superuser | t | SELECT pg_stat_statements_reset() IS
NOT NULL AS t | 1 | 1
+ regress_stats_superuser | t | SELECT r.rolname, ss.queryid <> $1
AS queryid_bool, ss.query, ss.calls, ss.rows+| 1 | 3
+ | | FROM pg_stat_statements ss JOIN
pg_roles r ON ss.userid = r.oid +| |
+ | | ORDER BY r.rolname, ss.query
COLLATE "C", ss.calls, ss.rows | |
+ regress_stats_user1 | t | SELECT $1+$2 AS "TWO"
| 1 | 1
+ regress_stats_user1 | t | SELECT r.rolname, ss.queryid <> $1
AS queryid_bool, ss.query, ss.calls, ss.rows+| 1 | 4
+ | | FROM pg_stat_statements ss JOIN
pg_roles r ON ss.userid = r.oid +| |
+ | | ORDER BY r.rolname, ss.query
COLLATE "C", ss.calls, ss.rows | |
+(5 rows)
+
+--
+-- cleanup
+--
+RESET ROLE;
+DROP ROLE regress_stats_superuser;
+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..75b1489a47
--- /dev/null
+++ b/contrib/pg_stat_statements/sql/privileges.sql
@@ -0,0 +1,60 @@
+--
+-- Only superusers and roles with privileges of the pg_read_all_stats role
+-- are allowed to see the SQL text and queryid of queries executed by
+-- other users. Other users can see the statistics.
+--
+
+SET pg_stat_statements.track_utility = FALSE;
+CREATE ROLE regress_stats_superuser SUPERUSER;
+CREATE ROLE regress_stats_user1;
+CREATE ROLE regress_stats_user2;
+GRANT pg_read_all_stats TO regress_stats_user2;
+
+SET ROLE regress_stats_superuser;
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+SELECT 1 AS "ONE";
+
+SET ROLE regress_stats_user1;
+SELECT 1+1 AS "TWO";
+
+--
+-- A superuser can read all columns of queries executed by others,
+-- including query text and queryid.
+--
+
+SET ROLE regress_stats_superuser;
+SELECT r.rolname, ss.queryid <> 0 AS queryid_bool, ss.query, ss.calls, ss.rows
+ FROM pg_stat_statements ss JOIN pg_roles r ON ss.userid = r.oid
+ ORDER BY r.rolname, ss.query COLLATE "C", ss.calls, ss.rows;
+
+--
+-- regress_stats_user1 has no privileges to read the query text or
+-- queryid of queries executed by others but can see statistics
+-- like calls and rows.
+--
+
+SET ROLE regress_stats_user1;
+SELECT r.rolname, ss.queryid <> 0 AS queryid_bool, ss.query, ss.calls, ss.rows
+ FROM pg_stat_statements ss JOIN pg_roles r ON ss.userid = r.oid
+ ORDER BY r.rolname, ss.query COLLATE "C", ss.calls, ss.rows;
+
+--
+-- regress_stats_user2, with pg_read_all_stats role privileges, can
+-- read all columns, including query text and queryid, of queries
+-- executed by others.
+--
+
+SET ROLE regress_stats_user2;
+SELECT r.rolname, ss.queryid <> 0 AS queryid_bool, ss.query, ss.calls, ss.rows
+ FROM pg_stat_statements ss JOIN pg_roles r ON ss.userid = r.oid
+ ORDER BY r.rolname, ss.query COLLATE "C", ss.calls, ss.rows;
+
+--
+-- cleanup
+--
+
+RESET ROLE;
+DROP ROLE regress_stats_superuser;
+DROP ROLE regress_stats_user1;
+DROP ROLE regress_stats_user2;
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
--
2.45.2
