On Mon, Jul 2, 2018 at 6:42 PM Sergei Kornilov <s...@zsrv.org> wrote:

> Hello
>

Thanks for the review.


> I found SELECT pg_stat_statements_reset(NULL,NULL,s.queryid) in tests and
> it pass tests, but i wonder how it works. Should not we check the NULL
> through PG_ARGISNULL macro before any PG_GETARG_*? According
> src/include/fmgr.h
> > * If function is not marked "proisstrict" in pg_proc, it must check for
> > * null arguments using this macro.  Do not try to GETARG a null argument!
>

Thanks for checking, Added.


> > pg_stat_statements_reset(userid Oid, dbid Oid, queryid bigint) returns
> void
> And you forgot to change return type in docs (and description of return
> value)
>

Corrected and also added details of the returns value.

Update patch attached.

Regards,
Haribabu Kommi
Fujitsu Australia
From f3d4c1a319ceefafabd45039b4b3447fb126e91c Mon Sep 17 00:00:00 2001
From: Hari Babu <kommi.haribabu@gmail.com>
Date: Fri, 29 Jun 2018 01:17:41 +1000
Subject: [PATCH] pg_stat_statements_reset to reset specific query/user/db
 statistics

Now the pg_stat_statements_reset() can accept input parameters of
userid, dbid and queryid. Based on the specified input values,
specific statistics entries are released. The new behaviour is useful
to get the fresh new statistics of a specific query/user/database
to observation without resetting all the existing statistics.
If no parameters are passed, it will release all entries as per the
old behaviour.

Backward Compatible change, Now pg_stat_statements_reset() function
returns the number of statement entries that are reset.
---
 contrib/pg_stat_statements/Makefile                |   8 +-
 .../expected/pg_stat_statements.out                | 134 ++++++++++++++++++++-
 .../pg_stat_statements--1.5--1.6.sql               |  24 ++++
 contrib/pg_stat_statements/pg_stat_statements.c    | 120 +++++++++++++-----
 .../pg_stat_statements/pg_stat_statements.control  |   2 +-
 .../pg_stat_statements/sql/pg_stat_statements.sql  |  45 +++++++
 doc/src/sgml/pgstatstatements.sgml                 |  16 ++-
 7 files changed, 303 insertions(+), 46 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.5--1.6.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 39b368b70e..f0942f829e 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -4,10 +4,10 @@ MODULE_big = pg_stat_statements
 OBJS = pg_stat_statements.o $(WIN32RES)
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.4--1.5.sql \
-	pg_stat_statements--1.3--1.4.sql pg_stat_statements--1.2--1.3.sql \
-	pg_stat_statements--1.1--1.2.sql pg_stat_statements--1.0--1.1.sql \
-	pg_stat_statements--unpackaged--1.0.sql
+DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.5--1.6.sql \
+	pg_stat_statements--1.4--1.5.sql pg_stat_statements--1.3--1.4.sql \
+	pg_stat_statements--1.2--1.3.sql pg_stat_statements--1.1--1.2.sql \
+	pg_stat_statements--1.0--1.1.sql pg_stat_statements--unpackaged--1.0.sql
 PGFILEDESC = "pg_stat_statements - execution statistics of SQL statements"
 
 LDFLAGS_SL += $(filter -lm, $(LIBS))
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index 5318c3550c..4e2ef7719a 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -6,7 +6,7 @@ SET pg_stat_statements.track_utility = FALSE;
 SELECT pg_stat_statements_reset();
  pg_stat_statements_reset 
 --------------------------
- 
+                       10
 (1 row)
 
 SELECT 1 AS "int";
@@ -122,7 +122,7 @@ SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
 SELECT pg_stat_statements_reset();
  pg_stat_statements_reset 
 --------------------------
- 
+                       12
 (1 row)
 
 -- utility "create table" should not be shown
@@ -216,7 +216,7 @@ SET pg_stat_statements.track = 'none';
 SELECT pg_stat_statements_reset();
  pg_stat_statements_reset 
 --------------------------
- 
+                       10
 (1 row)
 
 SELECT 1 AS "one";
@@ -243,7 +243,7 @@ SET pg_stat_statements.track = 'top';
 SELECT pg_stat_statements_reset();
  pg_stat_statements_reset 
 --------------------------
- 
+                        2
 (1 row)
 
 DO LANGUAGE plpgsql $$
@@ -303,7 +303,7 @@ SET pg_stat_statements.track = 'all';
 SELECT pg_stat_statements_reset();
  pg_stat_statements_reset 
 --------------------------
- 
+                        7
 (1 row)
 
 -- we drop and recreate the functions to avoid any caching funnies
@@ -361,7 +361,7 @@ SET pg_stat_statements.track_utility = TRUE;
 SELECT pg_stat_statements_reset();
  pg_stat_statements_reset 
 --------------------------
- 
+                        6
 (1 row)
 
 SELECT 1;
@@ -395,4 +395,126 @@ SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
  SELECT pg_stat_statements_reset()         |     1 |    1
 (8 rows)
 
+--
+-- Track user activity and reset them
+--
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--------------------------
+                        9
+(1 row)
+
+CREATE ROLE stats_regress_user1;
+CREATE ROLE stats_regress_user2;
+SET ROLE stats_regress_user1;
+SELECT 1 AS "ONE";
+ ONE 
+-----
+   1
+(1 row)
+
+SELECT 1+1 AS "TWO";
+ TWO 
+-----
+   2
+(1 row)
+
+RESET ROLE;
+SET ROLE stats_regress_user2;
+SELECT 1 AS "ONE";
+ ONE 
+-----
+   1
+(1 row)
+
+SELECT 1+1 AS "TWO";
+ TWO 
+-----
+   2
+(1 row)
+
+RESET ROLE;
+SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
+               query               | calls | rows 
+-----------------------------------+-------+------
+ CREATE ROLE stats_regress_user1   |     1 |    0
+ CREATE ROLE stats_regress_user2   |     1 |    0
+ RESET ROLE                        |     2 |    0
+ SELECT $1 AS "ONE"                |     1 |    1
+ SELECT $1 AS "ONE"                |     1 |    1
+ SELECT $1+$2 AS "TWO"             |     1 |    1
+ SELECT $1+$2 AS "TWO"             |     1 |    1
+ SELECT pg_stat_statements_reset() |     1 |    1
+ SET ROLE stats_regress_user1      |     1 |    0
+ SET ROLE stats_regress_user2      |     1 |    0
+(10 rows)
+
+--
+-- remove query ('SELECT $1 AS "ONE"') executed by two users
+--
+SELECT pg_stat_statements_reset(NULL,NULL,s.queryid) FROM pg_stat_statements AS s WHERE s.query = 'SELECT $1 AS "ONE"';
+ pg_stat_statements_reset 
+--------------------------
+                        2
+                        0
+(2 rows)
+
+SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
+                                              query                                               | calls | rows 
+--------------------------------------------------------------------------------------------------+-------+------
+ CREATE ROLE stats_regress_user1                                                                  |     1 |    0
+ CREATE ROLE stats_regress_user2                                                                  |     1 |    0
+ RESET ROLE                                                                                       |     2 |    0
+ SELECT $1+$2 AS "TWO"                                                                            |     1 |    1
+ SELECT $1+$2 AS "TWO"                                                                            |     1 |    1
+ SELECT pg_stat_statements_reset($1,$2,s.queryid) FROM pg_stat_statements AS s WHERE s.query = $3 |     1 |    2
+ SELECT pg_stat_statements_reset()                                                                |     1 |    1
+ SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C"                     |     1 |   10
+ SET ROLE stats_regress_user1                                                                     |     1 |    0
+ SET ROLE stats_regress_user2                                                                     |     1 |    0
+(10 rows)
+
+--
+-- remove query of a user (stats_regress_user1)
+--
+SELECT pg_stat_statements_reset(r.oid) FROM pg_roles AS r WHERE r.rolname = 'stats_regress_user1';
+ pg_stat_statements_reset 
+--------------------------
+                        2
+(1 row)
+
+SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
+                                              query                                               | calls | rows 
+--------------------------------------------------------------------------------------------------+-------+------
+ CREATE ROLE stats_regress_user1                                                                  |     1 |    0
+ CREATE ROLE stats_regress_user2                                                                  |     1 |    0
+ RESET ROLE                                                                                       |     2 |    0
+ SELECT $1+$2 AS "TWO"                                                                            |     1 |    1
+ SELECT pg_stat_statements_reset($1,$2,s.queryid) FROM pg_stat_statements AS s WHERE s.query = $3 |     1 |    2
+ SELECT pg_stat_statements_reset()                                                                |     1 |    1
+ SELECT pg_stat_statements_reset(r.oid) FROM pg_roles AS r WHERE r.rolname = $1                   |     1 |    1
+ SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C"                     |     2 |   20
+ SET ROLE stats_regress_user2                                                                     |     1 |    0
+(9 rows)
+
+--
+-- reset all
+--
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--------------------------
+                        9
+(1 row)
+
+SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
+               query               | calls | rows 
+-----------------------------------+-------+------
+ SELECT pg_stat_statements_reset() |     1 |    1
+(1 row)
+
+--
+-- cleanup
+--
+DROP ROLE stats_regress_user1;
+DROP ROLE stats_regress_user2;
 DROP EXTENSION pg_stat_statements;
diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.5--1.6.sql b/contrib/pg_stat_statements/pg_stat_statements--1.5--1.6.sql
new file mode 100644
index 0000000000..81ac627f37
--- /dev/null
+++ b/contrib/pg_stat_statements/pg_stat_statements--1.5--1.6.sql
@@ -0,0 +1,24 @@
+/* contrib/pg_stat_statements/pg_stat_statements--1.5--1.6.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pg_stat_statements UPDATE TO '1.6'" to load this file. \quit
+
+/* First we have to remove them from the extension */
+ALTER EXTENSION pg_stat_statements DROP FUNCTION pg_stat_statements_reset();
+
+/* Then we can drop them */
+DROP FUNCTION pg_stat_statements_reset();
+
+/* Now redefine */
+CREATE FUNCTION pg_stat_statements_reset(IN userid Oid DEFAULT NULL,
+	IN dbid Oid DEFAULT NULL,
+	IN queryid bigint DEFAULT NULL
+)
+RETURNS bigint
+AS 'MODULE_PATHNAME', 'pg_stat_statements_reset_1_6'
+LANGUAGE C PARALLEL SAFE;
+
+-- Don't want this to be available to non-superusers.
+REVOKE ALL ON FUNCTION pg_stat_statements_reset(Oid, Oid, bigint) FROM PUBLIC;
+GRANT EXECUTE ON FUNCTION pg_stat_statements_reset(Oid, Oid, bigint) TO pg_read_all_stats;
+
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index cc9efab243..df3b1ce867 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -290,6 +290,7 @@ void		_PG_init(void);
 void		_PG_fini(void);
 
 PG_FUNCTION_INFO_V1(pg_stat_statements_reset);
+PG_FUNCTION_INFO_V1(pg_stat_statements_reset_1_6);
 PG_FUNCTION_INFO_V1(pg_stat_statements_1_2);
 PG_FUNCTION_INFO_V1(pg_stat_statements_1_3);
 PG_FUNCTION_INFO_V1(pg_stat_statements);
@@ -327,7 +328,7 @@ static char *qtext_fetch(Size query_offset, int query_len,
 			char *buffer, Size buffer_size);
 static bool need_gc_qtexts(void);
 static void gc_qtexts(void);
-static void entry_reset(void);
+static long entry_reset(Oid userid, Oid dbid, uint64 queryid);
 static void AppendJumble(pgssJumbleState *jstate,
 			 const unsigned char *item, Size size);
 static void JumbleQuery(pgssJumbleState *jstate, Query *query);
@@ -1293,7 +1294,32 @@ done:
 }
 
 /*
- * Reset all statement statistics.
+ * Reset statement statistics according to userid, dbid, queryid.
+ */
+Datum
+pg_stat_statements_reset_1_6(PG_FUNCTION_ARGS)
+{
+	Oid			userid;
+	Oid			dbid;
+	uint64		queryid;
+	int64		num_remove;
+
+	if (!pgss || !pgss_hash)
+		ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				 errmsg("pg_stat_statements must be loaded via shared_preload_libraries")));
+
+	userid = PG_ARGISNULL(0) ? 0 : PG_GETARG_OID(0);
+	dbid = PG_ARGISNULL(1) ? 0 : PG_GETARG_OID(1);
+	queryid = (uint64) (PG_ARGISNULL(2) ? 0 : PG_GETARG_INT64(2));
+
+	num_remove = entry_reset(userid, dbid, queryid);
+
+	PG_RETURN_INT64(num_remove);
+}
+
+/*
+ * Reset statement statistics.
  */
 Datum
 pg_stat_statements_reset(PG_FUNCTION_ARGS)
@@ -1302,7 +1328,9 @@ pg_stat_statements_reset(PG_FUNCTION_ARGS)
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("pg_stat_statements must be loaded via shared_preload_libraries")));
-	entry_reset();
+
+	entry_reset(0, 0, 0);
+
 	PG_RETURN_VOID();
 }
 
@@ -2229,52 +2257,82 @@ gc_fail:
 }
 
 /*
- * Release all entries.
+ * Release entries according to parameters.
  */
-static void
-entry_reset(void)
+static long
+entry_reset(Oid userid, Oid dbid, uint64 queryid)
 {
 	HASH_SEQ_STATUS hash_seq;
 	pgssEntry  *entry;
 	FILE	   *qfile;
+	long		num_entries;
+	long		num_remove = 0;
+	pgssHashKey key;
 
 	LWLockAcquire(pgss->lock, LW_EXCLUSIVE);
+	num_entries = hash_get_num_entries(pgss_hash);
 
-	hash_seq_init(&hash_seq, pgss_hash);
-	while ((entry = hash_seq_search(&hash_seq)) != NULL)
+	/* If all parameter available, use the fast path */
+	if (userid != 0 && dbid != 0 && queryid != 0)
 	{
-		hash_search(pgss_hash, &entry->key, HASH_REMOVE, NULL);
+		key.userid = userid;
+		key.dbid = dbid;
+		key.queryid = queryid;
+
+		/* Remove the key out of hash if available */
+		entry = (pgssEntry *) hash_search(pgss_hash, &key, HASH_REMOVE, NULL);
+		if (entry)				/* found */
+			num_remove++;
 	}
-
-	/*
-	 * Write new empty query file, perhaps even creating a new one to recover
-	 * if the file was missing.
-	 */
-	qfile = AllocateFile(PGSS_TEXT_FILE, PG_BINARY_W);
-	if (qfile == NULL)
+	else
 	{
-		ereport(LOG,
-				(errcode_for_file_access(),
-				 errmsg("could not create pg_stat_statement file \"%s\": %m",
-						PGSS_TEXT_FILE)));
-		goto done;
+		hash_seq_init(&hash_seq, pgss_hash);
+		while ((entry = hash_seq_search(&hash_seq)) != NULL)
+		{
+			if ((!userid || (userid && entry->key.userid == userid)) &&
+				(!dbid || (dbid && entry->key.dbid == dbid)) &&
+				(!queryid || (queryid && entry->key.queryid == queryid)))
+			{
+				hash_search(pgss_hash, &entry->key, HASH_REMOVE, NULL);
+				num_remove++;
+			}
+		}
 	}
 
-	/* If ftruncate fails, log it, but it's not a fatal problem */
-	if (ftruncate(fileno(qfile), 0) != 0)
-		ereport(LOG,
-				(errcode_for_file_access(),
-				 errmsg("could not truncate pg_stat_statement file \"%s\": %m",
-						PGSS_TEXT_FILE)));
+	if (num_entries == num_remove)
+	{
+		/*
+		 * Write new empty query file, perhaps even creating a new one to
+		 * recover if the file was missing.
+		 */
+		qfile = AllocateFile(PGSS_TEXT_FILE, PG_BINARY_W);
+		if (qfile == NULL)
+		{
+			ereport(LOG,
+					(errcode_for_file_access(),
+					 errmsg("could not create pg_stat_statement file \"%s\": %m",
+							PGSS_TEXT_FILE)));
+			goto done;
+		}
 
-	FreeFile(qfile);
+		/* If ftruncate fails, log it, but it's not a fatal problem */
+		if (ftruncate(fileno(qfile), 0) != 0)
+			ereport(LOG,
+					(errcode_for_file_access(),
+					 errmsg("could not truncate pg_stat_statement file \"%s\": %m",
+							PGSS_TEXT_FILE)));
+
+		FreeFile(qfile);
 
 done:
-	pgss->extent = 0;
-	/* This counts as a query text garbage collection for our purposes */
-	record_gc_qtexts();
+		pgss->extent = 0;
+		/* This counts as a query text garbage collection for our purposes */
+		record_gc_qtexts();
+	}
 
 	LWLockRelease(pgss->lock);
+
+	return num_remove;
 }
 
 /*
diff --git a/contrib/pg_stat_statements/pg_stat_statements.control b/contrib/pg_stat_statements/pg_stat_statements.control
index 193fcdfafa..617038b4c0 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.control
+++ b/contrib/pg_stat_statements/pg_stat_statements.control
@@ -1,5 +1,5 @@
 # pg_stat_statements extension
 comment = 'track execution statistics of all SQL statements executed'
-default_version = '1.5'
+default_version = '1.6'
 module_pathname = '$libdir/pg_stat_statements'
 relocatable = true
diff --git a/contrib/pg_stat_statements/sql/pg_stat_statements.sql b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
index a8361fd1bf..c027b6c540 100644
--- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql
+++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
@@ -195,4 +195,49 @@ DROP FUNCTION PLUS_TWO(INTEGER);
 
 SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
 
+--
+-- Track user activity and reset them
+--
+SELECT pg_stat_statements_reset();
+CREATE ROLE stats_regress_user1;
+CREATE ROLE stats_regress_user2;
+
+SET ROLE stats_regress_user1;
+
+SELECT 1 AS "ONE";
+SELECT 1+1 AS "TWO";
+
+RESET ROLE;
+SET ROLE stats_regress_user2;
+
+SELECT 1 AS "ONE";
+SELECT 1+1 AS "TWO";
+
+RESET ROLE;
+SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+--
+-- remove query ('SELECT $1 AS "ONE"') executed by two users
+--
+SELECT pg_stat_statements_reset(NULL,NULL,s.queryid) FROM pg_stat_statements AS s WHERE s.query = 'SELECT $1 AS "ONE"';
+SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+--
+-- remove query of a user (stats_regress_user1)
+--
+SELECT pg_stat_statements_reset(r.oid) FROM pg_roles AS r WHERE r.rolname = 'stats_regress_user1';
+SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+--
+-- reset all
+--
+SELECT pg_stat_statements_reset();
+SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+--
+-- cleanup
+--
+DROP ROLE stats_regress_user1;
+DROP ROLE stats_regress_user2;
+
 DROP EXTENSION pg_stat_statements;
diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml
index c0217ed485..8432e4bb84 100644
--- a/doc/src/sgml/pgstatstatements.sgml
+++ b/doc/src/sgml/pgstatstatements.sgml
@@ -336,7 +336,7 @@
   <variablelist>
    <varlistentry>
     <term>
-     <function>pg_stat_statements_reset() returns void</function>
+     <function>pg_stat_statements_reset(userid Oid, dbid Oid, queryid bigint) returns bigint</function>
      <indexterm>
       <primary>pg_stat_statements_reset</primary>
      </indexterm>
@@ -344,9 +344,17 @@
 
     <listitem>
      <para>
-      <function>pg_stat_statements_reset</function> discards all statistics
-      gathered so far by <filename>pg_stat_statements</filename>.
-      By default, this function can only be executed by superusers.
+      <function>pg_stat_statements_reset</function> discards statistics
+      gathered so far by <filename>pg_stat_statements</filename> according to 
+      the specified userid, dbid and queryid. Returns the total number of
+      statement statistics that are reset based on the specified input.
+      If any of the parameter is not specified, the default value NULL(invalid)
+      is used and the statistics that match with other parameter(s) will be reset.
+      If no parameter is specified or specify everything as NULL(invalid), it will
+      discard all statistics.
+      By default, this function can only be executed by superusers and members
+      of the <literal>pg_read_all_stats</literal> role. Access may be granted
+      to others using <command>GRANT</command>.
      </para>
     </listitem>
    </varlistentry>
-- 
2.16.1.windows.4

Reply via email to