Re: Add connection active, idle time to pg_stat_activity

[Andrei Zubkov]

Hi Sergei,

> I still would like to maintaint the focus on committing the "idle in
transactions" part of pg_stat_session first.

Agreed.

I've done a review of version 0004. This version is applied successful
over ce571434ae7, installcheck passed. The behavior of pg_stat_session
view and corresponding function looks correct. I've didn't found any
issues in the code.

Notes about the current state of a patch:

Naming
the view and function names 'pg_stat_session' seems correct for this
particular scope of a patch. However possible future resource
consumption statistics are valid for all backends (vacuum for example).
Right now it is not clear for me if we can get resource statistics from
those backends while those are listed in the pg_stat_activity view but
renaming to something like 'pg_stat_backend' seems reasonable to me.

Docs
1. session states referenced in monitoring.sgml is not uniform with
those of the pg_stat_activity view.
monitoring.sgml:4635
monitoring.sgml:4644
+   Time in milliseconds this backend spent in the
running or fastpath state.
I think those states should be referenced uniformly with
pg_stat_activity.

2. Description of the 'pg_stat_get_session()' function might be as
follows:

  Returns a row, showing statistics about the client backend with the
  specified process ID, or one row per client backend if
  NULL is specified. The fields returned are the
  same as those of pg_stat_session view.

The main thought here is to get rid of 'each active backend' because
'active backend' looks like backend in the 'active' state.

Tests
Call to a non-existing function is depend on non-existence of a
function, which can't be guaranteed absolutely. How about to do some
kind of obvious error here? Couple examples follows:

SELECT 0/0;

- or -

DO $$
BEGIN
RAISE 'test error';
END;
$$ LANGUAGE plpgsql;

My personal choice would be the last one.

-- 
regards, Andrei Zubkov
Postgres Professional

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Alexander!

On Sat, 2023-11-25 at 02:45 +0200, Alexander Korotkov wrote:

> I've reviewed this patch.

Thank you very much for your review.

> I think the design was well-discussed in this thread.  Implementation
> also looks good to me.  I've just slightly revised the commit
> messages.

I've noted a strange space in a commit message of 0001 patch: 
"I t is needed for upcoming patch..." 
It looks like a typo.

-- 
regards, Andrei Zubkov 
Postgres Professional

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

A little fix in "level_tracking" tests after merge.

-- 
regards, Andrei Zubkov
Postgres Professional


From ed7531ba471061346922bbcb00d92738f6515a3f Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Fri, 17 Nov 2023 11:27:20 +0300
Subject: [PATCH 1/2] pg_stat_statements tests: Add NOT NULL checking of
 pg_stat_statements_reset

This is preliminary patch. It adds NOT NULL checking for the result of
pg_stat_statements_reset() function. It is needed for upcoming patch
"Track statement entry timestamp" that will change the result type of
this function to the timestamp of a reset performed.

Author: Andrei Zubkov (zubkov)

Reviewed by: Julien Rouhaud (rjuju), Hayato Kuroda (ha-kun),
  Yuki Seino (seinoyu), Chengxi Sun (martin-sun),
  Anton Melnikov (antonmel), Darren Rush (darrenr),
  Michael Paquier (michael-kun), Sergei Kornilov (melkij),
  Alena Rybakina (a.rybakina), Andrei Lepikhov (lepikhov)

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 .../pg_stat_statements/expected/cursors.out   |  28 +--
 contrib/pg_stat_statements/expected/dml.out   |  28 +--
 .../expected/level_tracking.out   | 112 +--
 .../pg_stat_statements/expected/planning.out  |  18 +-
 .../pg_stat_statements/expected/select.out|  44 ++---
 .../expected/user_activity.out| 120 ++--
 .../pg_stat_statements/expected/utility.out   | 178 +-
 contrib/pg_stat_statements/expected/wal.out   |  10 +-
 contrib/pg_stat_statements/sql/cursors.sql|   6 +-
 contrib/pg_stat_statements/sql/dml.sql|   6 +-
 .../pg_stat_statements/sql/level_tracking.sql |  16 +-
 contrib/pg_stat_statements/sql/planning.sql   |   4 +-
 contrib/pg_stat_statements/sql/select.sql |  10 +-
 .../pg_stat_statements/sql/user_activity.sql  |  15 +-
 contrib/pg_stat_statements/sql/utility.sql|  32 ++--
 contrib/pg_stat_statements/sql/wal.sql|   2 +-
 16 files changed, 317 insertions(+), 312 deletions(-)

diff --git a/contrib/pg_stat_statements/expected/cursors.out b/contrib/pg_stat_statements/expected/cursors.out
index 46375ea9051..0fc4b2c098d 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -3,10 +3,10 @@
 --
 -- These tests require track_utility to be enabled.
 SET pg_stat_statements.track_utility = TRUE;
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- DECLARE
@@ -20,13 +20,13 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 ---+--+---
  2 |0 | CLOSE cursor_stats_1
  2 |0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (3 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- FETCH
@@ -59,12 +59,12 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |0 | DECLARE cursor_stats_2 CURSOR WITH HOLD FOR SELECT $1
  1 |1 | FETCH 1 IN cursor_stats_1
  1 |1 | FETCH 1 IN cursor_stats_2
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (9 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
diff --git a/contrib/pg_stat_statements/expected/dml.out b/contrib/pg_stat_statements/expected/dml.out
index ede47a71acc..f6ac8da5ca2 100644
--- a/contrib/pg_stat_statements/expected/dml.out
+++ b/contrib/pg_stat_statements/expected/dml.out
@@ -81,16 +81,16 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |   12 | SELECT * FROM pgss_dml_tab ORDER BY a
  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 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
  1 |0 | SET pg_stat_statements.track_utility = FALSE
  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)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- MERGE
@@ -136,16 +136,16 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
|  |  WHEN NOT MATCHED THEN INSERT (a,

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi hackers,

Patch rebased to the current master
-- 
regards, Andrei Zubkov
Postgres Professional
From ff2ff96352a843d32a1213a1e953503fd1525b7b Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Fri, 17 Nov 2023 00:27:24 +0300
Subject: [PATCH 1/2] pg_stat_statements tests: Add NOT NULL checking of
 pg_stat_statements_reset

This is preliminary patch. It adds NOT NULL checking for the result of
pg_stat_statements_reset() function. It is needed for upcoming patch
"Track statement entry timestamp" that will change the result type of
this function to the timestamp of a reset performed.

Author: Andrei Zubkov (zubkov)

Reviewed by: Julien Rouhaud (rjuju), Hayato Kuroda (ha-kun),
  Yuki Seino (seinoyu), Chengxi Sun (martin-sun),
  Anton Melnikov (antonmel), Darren Rush (darrenr),
  Michael Paquier (michael-kun), Sergei Kornilov (melkij),
  Alena Rybakina (a.rybakina), Andrei Lepikhov (lepikhov)

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 .../pg_stat_statements/expected/cursors.out   |  28 +--
 contrib/pg_stat_statements/expected/dml.out   |  28 +--
 .../expected/level_tracking.out   |  80 
 .../pg_stat_statements/expected/planning.out  |  18 +-
 .../pg_stat_statements/expected/select.out|  44 ++---
 .../expected/user_activity.out| 120 ++--
 .../pg_stat_statements/expected/utility.out   | 178 +-
 contrib/pg_stat_statements/expected/wal.out   |  10 +-
 contrib/pg_stat_statements/sql/cursors.sql|   6 +-
 contrib/pg_stat_statements/sql/dml.sql|   6 +-
 .../pg_stat_statements/sql/level_tracking.sql |  12 +-
 contrib/pg_stat_statements/sql/planning.sql   |   4 +-
 contrib/pg_stat_statements/sql/select.sql |  10 +-
 .../pg_stat_statements/sql/user_activity.sql  |  15 +-
 contrib/pg_stat_statements/sql/utility.sql|  32 ++--
 contrib/pg_stat_statements/sql/wal.sql|   2 +-
 16 files changed, 299 insertions(+), 294 deletions(-)

diff --git a/contrib/pg_stat_statements/expected/cursors.out b/contrib/pg_stat_statements/expected/cursors.out
index 46375ea9051..0fc4b2c098d 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -3,10 +3,10 @@
 --
 -- These tests require track_utility to be enabled.
 SET pg_stat_statements.track_utility = TRUE;
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- DECLARE
@@ -20,13 +20,13 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 ---+--+---
  2 |0 | CLOSE cursor_stats_1
  2 |0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (3 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- FETCH
@@ -59,12 +59,12 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |0 | DECLARE cursor_stats_2 CURSOR WITH HOLD FOR SELECT $1
  1 |1 | FETCH 1 IN cursor_stats_1
  1 |1 | FETCH 1 IN cursor_stats_2
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (9 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
diff --git a/contrib/pg_stat_statements/expected/dml.out b/contrib/pg_stat_statements/expected/dml.out
index ede47a71acc..f6ac8da5ca2 100644
--- a/contrib/pg_stat_statements/expected/dml.out
+++ b/contrib/pg_stat_statements/expected/dml.out
@@ -81,16 +81,16 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |   12 | SELECT * FROM pgss_dml_tab ORDER BY a
  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 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
  1 |0 | SET pg_stat_statements.track_utility = FALSE
  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)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- MERGE
@@ -136,16 +136,16 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
|  |  WHEN NOT MATCHED THEN INSERT (a, b) VALUES ($1, $2)
  

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

On Thu, 2023-10-26 at 15:49 +0700, Andrei Lepikhov wrote:
> It is the gist of my question. If needed, You can remove the record
> by 
> (userid, dbOid, queryId). As I understand, this extension is usually 
> used by an administrator. Who can reset these parameters except you
> and 
> the DBMS?
This extension is used by administrator but indirectly through some
kind of sampling solution providing information about statistics change
over time. The only kind of statistics unavailable to sampling
solutions without a periodic reset is a min/max statistics. This patch
provides a way for resetting those statistics without entry eviction.
Suppose the DBA will use several sampling solutions. Every such
solution can perform its own resets of min/max statistics. Other
sampling solutions need a way to detect such resets to avoid undetected
interference. Timestamping of min/max reset can be used for that
purpose.

-- 
regards, Andrei Zubkov
Postgres Professional

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Alena,

On Wed, 2023-10-25 at 16:25 +0300, Alena Rybakina wrote:
>  Hi! Thank you for your work on the subject.
> 1. I didn't understand why we first try to find pgssEntry with a
> false top_level value, and later find it with a true top_level value.

In case of pg_stat_statements the top_level field is the bool field of
the pgssHashKey struct used as the key for pgss_hash hashtable. When we
are performing a reset only userid, dbid and queryid values are
provided. We need to reset both top-level and non-top level entries. We
have only one way to get them all from a hashtable - search for entries
having top_level=true and with top_level=false in their keys.

> 2. And honestly, I think you should change 
>  "Remove the entry if it exists, starting with the non-top-level
> entry." on 
>  "Remove the entry or reset min/max time statistic information and
> the timestamp if it exists, starting with the non-top-level entry."
> And the same with the comment bellow:
> "Also reset the top-level entry if it exists."
>  "Also remove the entry or reset min/max time statistic information
> and the timestamp if it exists."

There are four such places actually - every time when the
SINGLE_ENTRY_RESET macro is used. The logic of reset performed is
described a bit in this macro definition. It seems quite redundant to
repeat this description four times. But I've noted that "remove" terms
should be replaced by "reset".

I've attached v24 with commits updated.

regards, Andrei Zubkov
Postgres Professional

From 4fadc88d5e6c1afe7558393ba99c28d070ac7244 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Wed, 25 Oct 2023 17:58:57 +0300
Subject: [PATCH 1/2] pg_stat_statements tests: Add NOT NULL checking of
 pg_stat_statements_reset

This is preliminary patch. It adds NOT NULL checking for the result of
pg_stat_statements_reset() function. It is needed for upcoming patch
"Track statement entry timestamp" that will change the result type of
this function to the timestamp of a reset performed.

Author: Andrei Zubkov (zubkov)

Reviewed by: Julien Rouhaud (rjuju), Hayato Kuroda (ha-kun),
  Yuki Seino (seinoyu), Chengxi Sun (martin-sun),
  Anton Melnikov (antonmel), Darren Rush (darrenr),
  Michael Paquier (michael-kun), Sergei Kornilov (melkij),
  Alena Rybakina (a.rybakina), Andrei Lepikhov (lepikhov)

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 .../pg_stat_statements/expected/cursors.out   |  28 +--
 contrib/pg_stat_statements/expected/dml.out   |  28 +--
 .../expected/level_tracking.out   |  80 
 .../pg_stat_statements/expected/planning.out  |  18 +-
 .../pg_stat_statements/expected/select.out|  44 ++--
 .../expected/user_activity.out| 120 +--
 .../pg_stat_statements/expected/utility.out   | 192 +-
 contrib/pg_stat_statements/expected/wal.out   |  10 +-
 contrib/pg_stat_statements/sql/cursors.sql|   6 +-
 contrib/pg_stat_statements/sql/dml.sql|   6 +-
 .../pg_stat_statements/sql/level_tracking.sql |  12 +-
 contrib/pg_stat_statements/sql/planning.sql   |   4 +-
 contrib/pg_stat_statements/sql/select.sql |  10 +-
 .../pg_stat_statements/sql/user_activity.sql  |  15 +-
 contrib/pg_stat_statements/sql/utility.sql|  34 ++--
 contrib/pg_stat_statements/sql/wal.sql|   2 +-
 16 files changed, 307 insertions(+), 302 deletions(-)

diff --git a/contrib/pg_stat_statements/expected/cursors.out b/contrib/pg_stat_statements/expected/cursors.out
index 46375ea9051..0fc4b2c098d 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -3,10 +3,10 @@
 --
 -- These tests require track_utility to be enabled.
 SET pg_stat_statements.track_utility = TRUE;
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- DECLARE
@@ -20,13 +20,13 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 ---+--+---
  2 |0 | CLOSE cursor_stats_1
  2 |0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (3 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- FETCH
@@ -59,12 +59,12 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |0 | DECLARE cursor_stats_2 CURSOR WITH HOLD FOR SELECT $1
  1 |1 | FETCH 1 IN cursor_stats_1
  1 |1 | FETCH 1 IN cursor_stats_2
- 1 |   

Re: Add connection active, idle time to pg_stat_activity

[Andrei Zubkov]

Hi Aleksander,

On Wed, 2023-10-25 at 16:17 +0300, Aleksander Alekseev wrote:
> On top of that not sure if I see the patch on the November commitfest
> [1]. Please make sure it's there so that cfbot will check the patch.

Yes, this patch is listed on the November commitfest. cfbot says rebase
needed since 2023-08-21.

regards, Andrei Zubkov

Re: Add connection active, idle time to pg_stat_activity

[Andrei Zubkov]

Hi Sergey,

I've done a review of this patch. I found the patch idea very useful,
thank you for the patch. I've noted something observing this patch:
1. Patch can't be applied on the current master. My review is based on
   application of this patch over ac68323a878
2. Being applied over ac68323a878 patch works as expected.
3. Field names seems quite long to me (and they should be uniformly
   named with the same statistics in other views. For example
   "running" term is called "active" in pg_stat_database)
4. Meaningless spaces at the end of line:
   - backend_status.c:586
   - monitoring.sgml:5857
5. Patch adds

 usecs_diff = secs * 100 + usecs;

   at backend_status.c:pgstat_report_activity() to optimize
   calculations. But

 pgstat_count_conn_active_time((PgStat_Counter) secs * 100 +
usecs);
   and  
 pgstat_count_conn_txn_idle_time((PgStat_Counter) secs * 100 +
usecs);

   are left in place after that.
6. I'm not sure that I can understand the comment
 /* Keep statistics for pg_stat_database intact */
   at backend_status.c:600 correctly. Can you please explain it a
   little?
7. Tests seems incomplete. It looks like we can check increments in
   all fields playing with transactions in tests.

Also, I have a thought about other possible improvements fitting to
this patch.

The view pg_stat_session is really needed in Postgres but I think it
should have much more statistics. I mean all resource statistics
related to sessions. Every backend has instrumentation that tracks
resource consumption. Data of this instrumentation goes to the
cumulative statistics system and is used in monitoring extensions
(like pg_stat_statements). I think pg_stat_session view is able to add
one more dimension of monitoring - a dimension of sessions. In my
opinion this view should provide resource consumption statistics of
current sessions in two cumulative sets of statistics - since backend
start and since transaction start.  Such view will be really useful in
monitoring of long running sessions and transactions providing
resource consumption information besides timing statistics.

regards, Andrei Zubkov
Postgres Professional

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Andrei,

On Wed, 2023-10-25 at 13:59 +0700, Andrei Lepikhov wrote:
> But minmax_stats_since and changes in the UI of the reset routine
> look like syntactic sugar here.
> I can't convince myself that it is really needed. Do you have some
> set of cases that can enforce the changes proposed?

Yes, it looks strange, but it is needed in some way.
The main purpose of this patch is to provide means for sampling
solutions for collecting statistics data in series of samples. The
first goal here - is to be sure that the statement was not evicted and
come back between samples (especially between rare samples). This is
the target of the stats_since field. The second goal - is the ability
of getting all statistic increments for the interval between samples.
All statistics provided by pg_stat_statements with except of min/max
values can be calculated for the interval since the last sample knowing
the values in the last and current samples. We need a way to get
min/max values too. This is achieved by min/max reset performed by the
sampling solution. The minmax_stats_since field is here for the same
purpose - the sampling solution is need to be sure that no one have
done a min/max reset between samples. And if such reset was performed
at least we know when it was performed.

regards,
Andrei Zubkov
Postgres Professional

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi,

During last moving to the current commitfest this patch have lost its
reviewers list. With respect to reviewers contribution in this patch, I
think reviewers list should be fixed.

regards,

Andrei Zubkov
Postgres Professional
The Russian Postgres Company

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi hackers,

New version 23 attached. It contains rebase to the current master.
Noted that v1.11 adds new fields to the pg_stat_sstatements view, but
leaves the PGSS_FILE_HEADER constant unchanged. It this correct?

-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From 1bc4ccaed7cd3922d0b0bb83afd07f386792c8dc Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Thu, 19 Oct 2023 15:04:42 +0300
Subject: [PATCH 1/2] pg_stat_statements tests: Add NOT NULL checking of
 pg_stat_statements_reset

This is preliminary patch. It adds NOT NULL checking for the result of
pg_stat_statements_reset() function. It is needed for upcoming patch
"Track statement entry timestamp" that will change the result type of
this function to the timestamp of a reset performed.

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 .../pg_stat_statements/expected/cursors.out   |  28 +--
 contrib/pg_stat_statements/expected/dml.out   |  28 +--
 .../expected/level_tracking.out   |  80 
 .../pg_stat_statements/expected/planning.out  |  18 +-
 .../pg_stat_statements/expected/select.out|  44 ++--
 .../expected/user_activity.out| 120 +--
 .../pg_stat_statements/expected/utility.out   | 192 +-
 contrib/pg_stat_statements/expected/wal.out   |  10 +-
 contrib/pg_stat_statements/sql/cursors.sql|   6 +-
 contrib/pg_stat_statements/sql/dml.sql|   6 +-
 .../pg_stat_statements/sql/level_tracking.sql |  12 +-
 contrib/pg_stat_statements/sql/planning.sql   |   4 +-
 contrib/pg_stat_statements/sql/select.sql |  10 +-
 .../pg_stat_statements/sql/user_activity.sql  |  15 +-
 contrib/pg_stat_statements/sql/utility.sql|  34 ++--
 contrib/pg_stat_statements/sql/wal.sql|   2 +-
 16 files changed, 307 insertions(+), 302 deletions(-)

diff --git a/contrib/pg_stat_statements/expected/cursors.out b/contrib/pg_stat_statements/expected/cursors.out
index 46375ea9051..0fc4b2c098d 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -3,10 +3,10 @@
 --
 -- These tests require track_utility to be enabled.
 SET pg_stat_statements.track_utility = TRUE;
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- DECLARE
@@ -20,13 +20,13 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 ---+--+---
  2 |0 | CLOSE cursor_stats_1
  2 |0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (3 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- FETCH
@@ -59,12 +59,12 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |0 | DECLARE cursor_stats_2 CURSOR WITH HOLD FOR SELECT $1
  1 |1 | FETCH 1 IN cursor_stats_1
  1 |1 | FETCH 1 IN cursor_stats_2
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (9 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
diff --git a/contrib/pg_stat_statements/expected/dml.out b/contrib/pg_stat_statements/expected/dml.out
index ede47a71acc..f6ac8da5ca2 100644
--- a/contrib/pg_stat_statements/expected/dml.out
+++ b/contrib/pg_stat_statements/expected/dml.out
@@ -81,16 +81,16 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |   12 | SELECT * FROM pgss_dml_tab ORDER BY a
  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 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
  1 |0 | SET pg_stat_statements.track_utility = FALSE
  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)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- MERGE
@@ -136,16 +136,16 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
|  |  WHEN NOT MATCHED THEN INSERT (a, b) VALUES ($1, $2)
  1 |0 | MERGE INTO pgss_dml_tab USING pgss_dml_tab st ON (st.a = pgss_dml_tab.a)  

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Sergei!

Thank you for your review.

On Tue, 2023-03-21 at 23:18 +0300, Sergei Kornilov wrote:
> -- Don't want this to be available to non-superusers.
> REVOKE ALL ON FUNCTION pg_stat_statements_reset(Oid, Oid, bigint,
> boolean) FROM PUBLIC;
> 
> should be added to the upgrade script

Indeed.

> Also, shouldn't we first do:
> 
> /* First we have to remove them from the extension */
> ALTER EXTENSION pg_stat_statements DROP VIEW ..
> ALTER EXTENSION pg_stat_statements DROP FUNCTION ..
> 
> like in previous upgrade scripts?

It was discussed few messages earlier in this thread. We've decided
that those are unnecessary in upgrade script.

> > +   Time at which min/max statistics gathering started for this
> > +   statement
> 
> I think it would be better to explicitly mention in the documentation
> all 4 fields for which minmax_stats_since displays the time.

Agreed.

New version is attached.

regards, Andrei
From 187aeb26df54daea4f38eb9a8605ee09985b55ae Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Wed, 22 Mar 2023 10:09:58 +0300
Subject: [PATCH 1/2] pg_stat_statements tests: Add NOT NULL checking of
 pg_stat_statements_reset

This is preliminary patch. It adds NOT NULL checking for the result of
pg_stat_statements_reset() function. It is needed for upcoming patch
"Track statement entry timestamp" that will change the result type of
this function to the timestamp of a reset performed.

Reviewed by: Julien Rouhaud, Hayato Kuroda, Yuki Seino, Chengxi Sun,
Anton Melnikov, Darren Rush, Michael Paquier, Sergei Kornilov

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 .../pg_stat_statements/expected/cursors.out   |  28 ++--
 contrib/pg_stat_statements/expected/dml.out   |  20 +--
 .../expected/level_tracking.out   |  80 +-
 .../pg_stat_statements/expected/planning.out  |  18 +--
 .../pg_stat_statements/expected/select.out|  44 ++---
 .../expected/user_activity.out| 120 +++---
 .../pg_stat_statements/expected/utility.out   | 150 +-
 contrib/pg_stat_statements/expected/wal.out   |  10 +-
 contrib/pg_stat_statements/sql/cursors.sql|   6 +-
 contrib/pg_stat_statements/sql/dml.sql|   4 +-
 .../pg_stat_statements/sql/level_tracking.sql |  12 +-
 contrib/pg_stat_statements/sql/planning.sql   |   4 +-
 contrib/pg_stat_statements/sql/select.sql |  10 +-
 .../pg_stat_statements/sql/user_activity.sql  |  15 +-
 contrib/pg_stat_statements/sql/utility.sql|  28 ++--
 contrib/pg_stat_statements/sql/wal.sql|   2 +-
 16 files changed, 278 insertions(+), 273 deletions(-)

diff --git a/contrib/pg_stat_statements/expected/cursors.out b/contrib/pg_stat_statements/expected/cursors.out
index 46375ea9051..0fc4b2c098d 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -3,10 +3,10 @@
 --
 -- These tests require track_utility to be enabled.
 SET pg_stat_statements.track_utility = TRUE;
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- DECLARE
@@ -20,13 +20,13 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 ---+--+---
  2 |0 | CLOSE cursor_stats_1
  2 |0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (3 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- FETCH
@@ -59,12 +59,12 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |0 | DECLARE cursor_stats_2 CURSOR WITH HOLD FOR SELECT $1
  1 |1 | FETCH 1 IN cursor_stats_1
  1 |1 | FETCH 1 IN cursor_stats_2
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (9 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
diff --git a/contrib/pg_stat_statements/expected/dml.out b/contrib/pg_stat_statements/expected/dml.out
index 7b9c8f979ee..4c723a038b9 100644
--- a/contrib/pg_stat_statements/expected/dml.out
+++ b/contrib/pg_stat_statements/expected/dml.out
@@ -81,16 +81,16 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |   12 | SELECT * FROM pgss_dml_tab ORDER BY a
  2 |4 | SELECT * FROM pgss_dml_tab WHERE a > $1 ORDER BY a
  1 |8 | SELECT * FROM pgss_dml_t

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

A little comment fix in update script of a patch
-- 
Andrei Zubkov
From 52e75fa05f5dea5700d96aea81ea81d91492b018 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Thu, 16 Mar 2023 13:18:59 +0300
Subject: [PATCH 1/2] pg_stat_statements tests: Add NOT NULL checking of
 pg_stat_statements_reset

This is preliminary patch. It adds NOT NULL checking for the result of
pg_stat_statements_reset() function. It is needed for upcoming patch
"Track statement entry timestamp" that will change the result type of
this function to the timestamp of a reset performed.

Reviewed by: Julien Rouhaud, Hayato Kuroda, Yuki Seino, Chengxi Sun,
Anton Melnikov, Darren Rush

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 .../pg_stat_statements/expected/cursors.out   |  28 ++--
 contrib/pg_stat_statements/expected/dml.out   |  20 +--
 .../expected/level_tracking.out   |  80 +-
 .../pg_stat_statements/expected/planning.out  |  18 +--
 .../pg_stat_statements/expected/select.out|  44 ++---
 .../expected/user_activity.out| 120 +++---
 .../pg_stat_statements/expected/utility.out   | 150 +-
 contrib/pg_stat_statements/expected/wal.out   |  10 +-
 contrib/pg_stat_statements/sql/cursors.sql|   6 +-
 contrib/pg_stat_statements/sql/dml.sql|   4 +-
 .../pg_stat_statements/sql/level_tracking.sql |  12 +-
 contrib/pg_stat_statements/sql/planning.sql   |   4 +-
 contrib/pg_stat_statements/sql/select.sql |  10 +-
 .../pg_stat_statements/sql/user_activity.sql  |  15 +-
 contrib/pg_stat_statements/sql/utility.sql|  28 ++--
 contrib/pg_stat_statements/sql/wal.sql|   2 +-
 16 files changed, 278 insertions(+), 273 deletions(-)

diff --git a/contrib/pg_stat_statements/expected/cursors.out b/contrib/pg_stat_statements/expected/cursors.out
index 46375ea9051..0fc4b2c098d 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -3,10 +3,10 @@
 --
 -- These tests require track_utility to be enabled.
 SET pg_stat_statements.track_utility = TRUE;
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- DECLARE
@@ -20,13 +20,13 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 ---+--+---
  2 |0 | CLOSE cursor_stats_1
  2 |0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (3 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- FETCH
@@ -59,12 +59,12 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |0 | DECLARE cursor_stats_2 CURSOR WITH HOLD FOR SELECT $1
  1 |1 | FETCH 1 IN cursor_stats_1
  1 |1 | FETCH 1 IN cursor_stats_2
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (9 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
diff --git a/contrib/pg_stat_statements/expected/dml.out b/contrib/pg_stat_statements/expected/dml.out
index 7b9c8f979ee..4c723a038b9 100644
--- a/contrib/pg_stat_statements/expected/dml.out
+++ b/contrib/pg_stat_statements/expected/dml.out
@@ -81,16 +81,16 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |   12 | SELECT * FROM pgss_dml_tab ORDER BY a
  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 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
  1 |0 | SET pg_stat_statements.track_utility = FALSE
  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)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- MERGE
@@ -136,12 +136,12 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
|  |  WHEN NOT MATCHED THEN INSERT (a, b) VALUES ($1, $2)
  1 |0 | MERGE INTO pgss_dml_tab USING pgss_dml_tab st ON (st.a = pgss_dml_tab.a)   +
|  |  WHEN NOT MATCHED THEN INSERT (b, a) VALUES ($1, $2)
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 |

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Michael,

Thank you for your attention.

On Thu, 2023-03-16 at 16:13 +0900, Michael Paquier wrote:
> +/* First we have to remove them from the extension */
> +ALTER EXTENSION pg_stat_statements DROP VIEW pg_stat_statements;
> +ALTER EXTENSION pg_stat_statements DROP FUNCTION
> pg_stat_statements(boolean);
> +ALTER EXTENSION pg_stat_statements DROP FUNCTION
> +  pg_stat_statements_reset(Oid, Oid, bigint);
> 
> The upgrade script of an extension is launched by the backend in the
> context of an extension, so these three queries should not be needed,
> as far as I recall.

Agreed. I've done it as it was in previous versions. But I'm sure those
are unnecessary.

> Wouldn't it be better to do this kind of refactoring in its own patch
> to make the follow-up changes more readable?  This function is
> changed
> to return a timestamp rather than void, but IS NOT NULL applied on
> the
> existing queries would also return true.  This would clean up quite a
> few diffs in the main patch..
Splitting this commit seems reasonable to me.

New version is attached.
From 52e75fa05f5dea5700d96aea81ea81d91492b018 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Thu, 16 Mar 2023 13:18:59 +0300
Subject: [PATCH 1/2] pg_stat_statements tests: Add NOT NULL checking of
 pg_stat_statements_reset

This is preliminary patch. It adds NOT NULL checking for the result of
pg_stat_statements_reset() function. It is needed for upcoming patch
"Track statement entry timestamp" that will change the result type of
this function to the timestamp of a reset performed.

Reviewed by: Julien Rouhaud, Hayato Kuroda, Yuki Seino, Chengxi Sun,
Anton Melnikov, Darren Rush

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 .../pg_stat_statements/expected/cursors.out   |  28 ++--
 contrib/pg_stat_statements/expected/dml.out   |  20 +--
 .../expected/level_tracking.out   |  80 +-
 .../pg_stat_statements/expected/planning.out  |  18 +--
 .../pg_stat_statements/expected/select.out|  44 ++---
 .../expected/user_activity.out| 120 +++---
 .../pg_stat_statements/expected/utility.out   | 150 +-
 contrib/pg_stat_statements/expected/wal.out   |  10 +-
 contrib/pg_stat_statements/sql/cursors.sql|   6 +-
 contrib/pg_stat_statements/sql/dml.sql|   4 +-
 .../pg_stat_statements/sql/level_tracking.sql |  12 +-
 contrib/pg_stat_statements/sql/planning.sql   |   4 +-
 contrib/pg_stat_statements/sql/select.sql |  10 +-
 .../pg_stat_statements/sql/user_activity.sql  |  15 +-
 contrib/pg_stat_statements/sql/utility.sql|  28 ++--
 contrib/pg_stat_statements/sql/wal.sql|   2 +-
 16 files changed, 278 insertions(+), 273 deletions(-)

diff --git a/contrib/pg_stat_statements/expected/cursors.out b/contrib/pg_stat_statements/expected/cursors.out
index 46375ea9051..0fc4b2c098d 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -3,10 +3,10 @@
 --
 -- These tests require track_utility to be enabled.
 SET pg_stat_statements.track_utility = TRUE;
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- DECLARE
@@ -20,13 +20,13 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 ---+--+---
  2 |0 | CLOSE cursor_stats_1
  2 |0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (3 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- FETCH
@@ -59,12 +59,12 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |0 | DECLARE cursor_stats_2 CURSOR WITH HOLD FOR SELECT $1
  1 |1 | FETCH 1 IN cursor_stats_1
  1 |1 | FETCH 1 IN cursor_stats_2
- 1 |1 | SELECT pg_stat_statements_reset()
+ 1 |1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (9 rows)
 
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
diff --git a/contrib/pg_stat_statements/expected/dml.out b/contrib/pg_stat_statements/expected/dml.out
index 7b9c8f979ee..4c723a038b9 100644
--- a/contrib/pg_stat_statements/expected/dml.out
+++ b/contrib/pg_stat_statements/expected/dml.out
@@ -81,16 +81,16 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  1 |   12 | SELECT * FROM pgss_dml_tab ORDER BY a
  2 |4 | SEL

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi,

I've done a rebase of a patch to the current master.

-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

From e43e9eab39bbd377665a7cad3b7fe7162f8f6578 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Sat, 11 Mar 2023 09:53:10 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since and minmax_stats_since columns to the
pg_stat_statements view and pg_stat_statements() function. The new min/max reset
mode for the pg_stat_stetments_reset() function is controlled by the
parameter minmax_only.
stat_since column is populated with the current timestamp when a new statement
is added to the pg_stat_statements hashtable. It provides clean information
about statistics collection time interval for each statement. Besides it can be
used by sampling solutions to detect situations when a statement was evicted and
stored again between samples.
Such sampling solution could derive any pg_stat_statements statistic values for
an interval between two samples with the exception of all min/max statistics. To
address this issue this patch adds the ability to reset min/max statistics
independently of the statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function. Timestamp of such reset is stored in the
minmax_stats_since field for each statement.
pg_stat_statements_reset() function now returns the timestamp of a reset as a
result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   4 +-
 .../pg_stat_statements/expected/cursors.out   |  28 +--
 contrib/pg_stat_statements/expected/dml.out   |  20 +--
 .../expected/entry_timestamp.out  | 159 ++
 .../expected/level_tracking.out   |  80 -
 .../expected/oldextversions.out   |  70 
 .../pg_stat_statements/expected/planning.out  |  18 +-
 .../pg_stat_statements/expected/select.out|  44 ++---
 .../expected/user_activity.out| 120 ++---
 .../pg_stat_statements/expected/utility.out   | 150 -
 contrib/pg_stat_statements/expected/wal.out   |  10 +-
 contrib/pg_stat_statements/meson.build|   2 +
 .../pg_stat_statements--1.10--1.11.sql|  81 +
 .../pg_stat_statements/pg_stat_statements.c   | 139 +++
 .../pg_stat_statements.control|   2 +-
 contrib/pg_stat_statements/sql/cursors.sql|   6 +-
 contrib/pg_stat_statements/sql/dml.sql|   4 +-
 .../sql/entry_timestamp.sql   | 114 +
 .../pg_stat_statements/sql/level_tracking.sql |  12 +-
 .../pg_stat_statements/sql/oldextversions.sql |   7 +
 contrib/pg_stat_statements/sql/planning.sql   |   4 +-
 contrib/pg_stat_statements/sql/select.sql |  10 +-
 .../pg_stat_statements/sql/user_activity.sql  |  15 +-
 contrib/pg_stat_statements/sql/utility.sql|  28 +--
 contrib/pg_stat_statements/sql/wal.sql|   2 +-
 doc/src/sgml/pgstatstatements.sgml|  66 +++-
 26 files changed, 881 insertions(+), 314 deletions(-)
 create mode 100644 contrib/pg_stat_statements/expected/entry_timestamp.out
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.10--1.11.sql
 create mode 100644 contrib/pg_stat_statements/sql/entry_timestamp.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 5578a9dd4e3..b2446e7a1cf 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,7 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql \
+DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.10--1.11.sql \
 	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.sql \
 	pg_stat_statements--1.5--1.6.sql pg_stat_statements--1.4--1.5.sql \
@@ -18,7 +18,7 @@ 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 cleanup oldextversions
+	user_activity wal entry_timestamp 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/cursors.out b/contrib/pg_stat_statements/expected/cursors.out
index 46375ea9051..0fc4b2c098d 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -3,10 +3,10 @@
 --
 -- These tests require track_utility to be enabled.
 SET pg_stat_statements.track_utility = TRU

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Gregory,

> patching file contrib/pg_stat_statements/sql/oldextversions.sql
> can't find file to patch at input line 1833
> 
> 
> and
> 
> > --- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql
> > +++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
> --
> No file to patch.  Skipping patch.
> 
Thank you for your attention.

Yes, it is due to parallel work on "Normalization of utility queries in
pg_stat_statements" patch
(https://postgr.es/m/Y/7Y9U/y/keaw...@paquier.xyz)

It seems I've found something strange in new test files - I've
mentioned this in a thread of a patch. Once there will be any solution
I'll do a rebase again as soon as possible.

-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: Normalization of utility queries in pg_stat_statements

[Andrei Zubkov]

Hi Michael!

I'm rebasing a patch "Tracking statements entry timestamp in
pg_stat_statements" for applying after this patch. I've noted that
current tests are not quite independent one from another. There is two
statements in the end of user_activity.sql test:

DROP ROLE regress_stats_user1;
DROP ROLE regress_stats_user2;

Those are done after the last pg_stat_statements_reset call in this
test file and thus, those are included in checks of wal.out file:

query 
| calls | rows | wal_bytes_generated | wal_records_generated |
wal_records_ge_rows 
---
---+---+--+-+---+--
---
 DELETE FROM pgss_wal_tab WHERE a > $1
| 1 |1 | t   | t | t
 DROP ROLE regress_stats_user1
| 1 |0 | t   | t | t
 DROP ROLE regress_stats_user2
| 1 |0 | t   | t | t

Those statements is not related to any WAL tests. It seems a little bit
incorrect to me.

Are we need some changes here?
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Gregory,

On Wed, 2023-03-01 at 14:24 -0500, Gregory Stark (as CFM) wrote:
> The CFBot (http://cfbot.cputube.org/) doesn't seem to like this. It
> looks like all the Meson builds are failing, perhaps there's
> something
> particular about the test environment that is either not set up right
> or is exposing a bug?

Thank you, I've missed it.
> 
> Please check if this is a real failure or a cfbot failure.
> 
It is my failure. Just forgot to update meson.build
I think CFBot should be happy now.

Regards,
-- 
Andrei Zubkov

From 0d97c3d3bc0f00c3a1c2bf00cbe9d1aab7e90a32 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Wed, 1 Mar 2023 23:07:00 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since and minmax_stats_since columns to the
pg_stat_statements view and pg_stat_statements() function. The new min/max reset
mode for the pg_stat_stetments_reset() function is controlled by the
parameter minmax_only.
stat_since column is populated with the current timestamp when a new statement
is added to the pg_stat_statements hashtable. It provides clean information
about statistics collection time interval for each statement. Besides it can be
used by sampling solutions to detect situations when a statement was evicted and
stored again between samples.
Such sampling solution could derive any pg_stat_statements statistic values for
an interval between two samples with the exception of all min/max statistics. To
address this issue this patch adds the ability to reset min/max statistics
independently of the statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function. Timestamp of such reset is stored in the
minmax_stats_since field for each statement.
pg_stat_statements_reset() function now returns the timestamp of a reset as a
result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   4 +-
 .../pg_stat_statements/expected/cursors.out   |  28 +--
 .../expected/entry_timestamp.out  | 159 ++
 .../expected/level_tracking.out   |  72 
 .../expected/oldextversions.out   |  70 
 .../expected/pg_stat_statements.out   | 147 
 .../pg_stat_statements/expected/planning.out  |  18 +-
 .../pg_stat_statements/expected/utility.out   | 142 
 contrib/pg_stat_statements/meson.build|   2 +
 .../pg_stat_statements--1.10--1.11.sql|  81 +
 .../pg_stat_statements/pg_stat_statements.c   | 139 +++
 .../pg_stat_statements.control|   2 +-
 contrib/pg_stat_statements/sql/cursors.sql|   6 +-
 .../sql/entry_timestamp.sql   | 114 +
 .../pg_stat_statements/sql/level_tracking.sql |  10 +-
 .../pg_stat_statements/sql/oldextversions.sql |   7 +
 .../sql/pg_stat_statements.sql|  28 +--
 contrib/pg_stat_statements/sql/planning.sql   |   4 +-
 contrib/pg_stat_statements/sql/utility.sql|  26 +--
 doc/src/sgml/pgstatstatements.sgml|  66 +++-
 20 files changed, 850 insertions(+), 275 deletions(-)
 create mode 100644 contrib/pg_stat_statements/expected/entry_timestamp.out
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.10--1.11.sql
 create mode 100644 contrib/pg_stat_statements/sql/entry_timestamp.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 69fbc6a8580..89b9f2dda85 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,7 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql \
+DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.10--1.11.sql \
 	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.sql \
 	pg_stat_statements--1.5--1.6.sql pg_stat_statements--1.4--1.5.sql \
@@ -18,7 +18,7 @@ LDFLAGS_SL += $(filter -lm, $(LIBS))
 
 REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
 REGRESS = pg_stat_statements cursors utility level_tracking planning \
-	cleanup oldextversions
+	entry_timestamp 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/cursors.out b/contrib/pg_stat_statements/expected/cursors.out
index 5d0dc196f97..b709bff830a 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -3,10 +3,10 @@
 --
 -- These tests require track_utility to be enabled.
 SET pg_stat_statem

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi!

I've attached a new version of a patch - rebase to the current master.
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


From 92816cfb26f0ebd35c00a6ce4f25e45f08d83790 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Wed, 1 Mar 2023 11:37:53 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since and minmax_stats_since columns to the
pg_stat_statements view and pg_stat_statements() function. The new min/max reset
mode for the pg_stat_stetments_reset() function is controlled by the
parameter minmax_only.
stat_since column is populated with the current timestamp when a new statement
is added to the pg_stat_statements hashtable. It provides clean information
about statistics collection time interval for each statement. Besides it can be
used by sampling solutions to detect situations when a statement was evicted and
stored again between samples.
Such sampling solution could derive any pg_stat_statements statistic values for
an interval between two samples with the exception of all min/max statistics. To
address this issue this patch adds the ability to reset min/max statistics
independently of the statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function. Timestamp of such reset is stored in the
minmax_stats_since field for each statement.
pg_stat_statements_reset() function now returns the timestamp of a reset as a
result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   4 +-
 .../pg_stat_statements/expected/cursors.out   |  28 +--
 .../expected/entry_timestamp.out  | 159 ++
 .../expected/level_tracking.out   |  72 
 .../expected/oldextversions.out   |  70 
 .../expected/pg_stat_statements.out   | 147 
 .../pg_stat_statements/expected/planning.out  |  18 +-
 .../pg_stat_statements/expected/utility.out   | 142 
 .../pg_stat_statements--1.10--1.11.sql|  81 +
 .../pg_stat_statements/pg_stat_statements.c   | 139 +++
 .../pg_stat_statements.control|   2 +-
 contrib/pg_stat_statements/sql/cursors.sql|   6 +-
 .../sql/entry_timestamp.sql   | 114 +
 .../pg_stat_statements/sql/level_tracking.sql |  10 +-
 .../pg_stat_statements/sql/oldextversions.sql |   7 +
 .../sql/pg_stat_statements.sql|  28 +--
 contrib/pg_stat_statements/sql/planning.sql   |   4 +-
 contrib/pg_stat_statements/sql/utility.sql|  26 +--
 doc/src/sgml/pgstatstatements.sgml|  66 +++-
 19 files changed, 848 insertions(+), 275 deletions(-)
 create mode 100644 contrib/pg_stat_statements/expected/entry_timestamp.out
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.10--1.11.sql
 create mode 100644 contrib/pg_stat_statements/sql/entry_timestamp.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 69fbc6a8580..89b9f2dda85 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,7 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql \
+DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.10--1.11.sql \
 	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.sql \
 	pg_stat_statements--1.5--1.6.sql pg_stat_statements--1.4--1.5.sql \
@@ -18,7 +18,7 @@ LDFLAGS_SL += $(filter -lm, $(LIBS))
 
 REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
 REGRESS = pg_stat_statements cursors utility level_tracking planning \
-	cleanup oldextversions
+	entry_timestamp 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/cursors.out b/contrib/pg_stat_statements/expected/cursors.out
index 5d0dc196f97..b709bff830a 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -3,10 +3,10 @@
 --
 -- These tests require track_utility to be enabled.
 SET pg_stat_statements.track_utility = TRUE;
-SELECT pg_stat_statements_reset();
- pg_stat_statements_reset 
---
- 
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
 (1 row)
 
 -- DECLARE
@@ -20,13 +20,13 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 ---+--+--
  2 |0 | CLOSE cursor_stats_1

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi,

The final version of this patch should fix meson build and tests.
-- 
Andrei Zubkov

From 94784bccd48a83cba58d6017253d0b8f051e159c Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Thu, 26 Jan 2023 13:18:11 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since and minmax_stats_since columns to the
pg_stat_statements view and pg_stat_statements() function. The new min/max reset
mode for the pg_stat_stetments_reset() function is controlled by the
parameter minmax_only.
stat_since column is populated with the current timestamp when a new statement
is added to the pg_stat_statements hashtable. It provides clean information
about statistics collection time interval for each statement. Besides it can be
used by sampling solutions to detect situations when a statement was evicted and
stored again between samples.
Such sampling solution could derive any pg_stat_statements statistic values for
an interval between two samples with the exception of all min/max statistics. To
address this issue this patch adds the ability to reset min/max statistics
independently of the statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function. Timestamp of such reset is stored in the
minmax_stats_since field for each statement.
pg_stat_statements_reset() function now returns the timestamp of a reset as a
result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   2 +-
 .../expected/oldextversions.out   |  70 
 .../expected/pg_stat_statements.out   | 361 +-
 contrib/pg_stat_statements/meson.build|   1 +
 .../pg_stat_statements--1.10--1.11.sql|  81 
 .../pg_stat_statements/pg_stat_statements.c   | 139 +--
 .../pg_stat_statements.control|   2 +-
 .../pg_stat_statements/sql/oldextversions.sql |   7 +
 .../sql/pg_stat_statements.sql| 149 +++-
 doc/src/sgml/pgstatstatements.sgml|  66 +++-
 10 files changed, 722 insertions(+), 156 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.10--1.11.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index edc40c8bbfb..0afb9060fa1 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,7 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql \
+DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.10--1.11.sql \
 	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.sql \
 	pg_stat_statements--1.5--1.6.sql pg_stat_statements--1.4--1.5.sql \
diff --git a/contrib/pg_stat_statements/expected/oldextversions.out b/contrib/pg_stat_statements/expected/oldextversions.out
index efb2049ecff..1e1cc11a8e2 100644
--- a/contrib/pg_stat_statements/expected/oldextversions.out
+++ b/contrib/pg_stat_statements/expected/oldextversions.out
@@ -250,4 +250,74 @@ SELECT count(*) > 0 AS has_data FROM pg_stat_statements;
  t
 (1 row)
 
+-- New functions and views for pg_stat_statements in 1.11
+AlTER EXTENSION pg_stat_statements UPDATE TO '1.11';
+\d pg_stat_statements
+  View "public.pg_stat_statements"
+ Column |   Type   | Collation | Nullable | Default 
++--+---+--+-
+ userid | oid  |   |  | 
+ dbid   | oid  |   |  | 
+ toplevel   | boolean  |   |  | 
+ queryid| bigint   |   |  | 
+ query  | text |   |  | 
+ plans  | bigint   |   |  | 
+ total_plan_time| double precision |   |  | 
+ min_plan_time  | double precision |   |  | 
+ max_plan_time  | double precision |   |  | 
+ mean_plan_time | double precision |   |  | 
+ stddev_plan_time   | double precision |   |  | 
+ calls  | bigint   |   |  | 
+ total_exec_time| double precision |   |  | 
+ min_exec_time  | double precision |   |  | 
+ max_exec_time  | double precision |   |  | 
+ mean_exec_time | double precision |   |  | 
+ stddev_exec_time   | double precision |   |  

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi,

I've updated this patch for the current master. Also I have some
additional explanations..

On Wed, 2023-01-18 at 17:29 +0100, Tomas Vondra wrote:
> 1) I'm not sure why the patch is adding tests of permissions on the
> pg_stat_statements_reset function?

I've fixed that

> 
> 2) If we want the second timestamp, shouldn't it also cover resets of
> the mean values, not just min/max?

I think that mean values are not a targets for auxiliary resets because
any sampling solution can easily calculate the mean values between
samples without a reset.

> 
> 3) I don't understand why the patch is adding "IS NOT NULL AS t" to
> various places in the regression tests.

This change is necessary in the current version because the
pg_stat_statements_reset() function will return a timestamp of a reset,
needed for sampling solutions to detect resets, perfermed by someone
else.


Regards
-- 
Andrei Zubkov

From 8214db3676e686993bcf73963f78c96baeb04c4e Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Wed, 25 Jan 2023 18:13:14 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since and minmax_stats_since columns to the
pg_stat_statements view and pg_stat_statements() function. The new min/max reset
mode for the pg_stat_stetments_reset() function is controlled by the
parameter minmax_only.
stat_since column is populated with the current timestamp when a new statement
is added to the pg_stat_statements hashtable. It provides clean information
about statistics collection time interval for each statement. Besides it can be
used by sampling solutions to detect situations when a statement was evicted and
stored again between samples.
Such sampling solution could derive any pg_stat_statements statistic values for
an interval between two samples with the exception of all min/max statistics. To
address this issue this patch adds the ability to reset min/max statistics
independently of the statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function. Timestamp of such reset is stored in the
minmax_stats_since field for each statement.
pg_stat_statements_reset() function now returns the timestamp of a reset as a
result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   2 +-
 .../expected/oldextversions.out   |  70 
 .../expected/pg_stat_statements.out   | 361 +-
 .../pg_stat_statements--1.10--1.11.sql|  81 
 .../pg_stat_statements/pg_stat_statements.c   | 139 +--
 .../pg_stat_statements.control|   2 +-
 .../pg_stat_statements/sql/oldextversions.sql |   7 +
 .../sql/pg_stat_statements.sql| 149 +++-
 doc/src/sgml/pgstatstatements.sgml|  66 +++-
 9 files changed, 721 insertions(+), 156 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.10--1.11.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index edc40c8bbfb..0afb9060fa1 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,7 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql \
+DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.10--1.11.sql \
 	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.sql \
 	pg_stat_statements--1.5--1.6.sql pg_stat_statements--1.4--1.5.sql \
diff --git a/contrib/pg_stat_statements/expected/oldextversions.out b/contrib/pg_stat_statements/expected/oldextversions.out
index efb2049ecff..1e1cc11a8e2 100644
--- a/contrib/pg_stat_statements/expected/oldextversions.out
+++ b/contrib/pg_stat_statements/expected/oldextversions.out
@@ -250,4 +250,74 @@ SELECT count(*) > 0 AS has_data FROM pg_stat_statements;
  t
 (1 row)
 
+-- New functions and views for pg_stat_statements in 1.11
+AlTER EXTENSION pg_stat_statements UPDATE TO '1.11';
+\d pg_stat_statements
+  View "public.pg_stat_statements"
+ Column |   Type   | Collation | Nullable | Default 
++--+---+--+-
+ userid | oid  |   |  | 
+ dbid   | oid  |   |  | 
+ toplevel   | boolean  |   |  | 
+ queryid| bigint   |   |  | 
+ query  | text |   |  | 
+ plans  | bigint   |   |  | 
+ total_plan_time| double precision   

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Tomas,

On Wed, 2023-01-18 at 17:29 +0100, Tomas Vondra wrote:
> I took a quick look at this patch, to see if there's something we
> want/can get into v16. The last version was submitted about 9 months
> ago, and it doesn't apply cleanly anymore, but the bitrot is fairly
> minor. Not sure there's still interest, though.

Thank you for your attention to this patch!

I'm still very interest in this patch. And I think I'll try to rebase
this patch during a week or two if it seems possible to get it in 16..
> 
> I'd probably call it "created_at" or something like
> that, but that's a minor detail. Or maybe stats_reset, which is what
> we
> use in pgstat?

Yes there is some naming issue. My thought was the following:
 - "stats_reset" is not quite correct here, because the statement entry
moment if definitely not a reset. The field named just as it means -
this is time of the moment from which statistics is collected for this
particular entry.
 - "created_at" perfectly matches the purpose of the field, but seems
not such self-explaining to me.

> 
> But is the second timestamp for the min/max fields really useful?
> AFAIK
> to perform analysis, people take regular pg_stat_statements
> snapshots,
> which works fine for counters (calculating deltas) but not for gauges
> (which need a reset, to track fresh values). But people analyzing
> this
> are already resetting the whole entry, and so the snapshots already
> are
> tracking deltas.
>
> So I'm not convinced actually need the second timestamp.

The main purpose of the patch is to provide means to collecting
solutions to avoid the reset of pgss at all. Just like it happens for
the pg_stat_ views. The only really need of reset is that we can't be
sure that observing statement was not evicted and come back since last
sample. Right now we only can do a whole reset on each sample and see
how many entries will be in pgss hashtable on the next sample - how
close this value to the max. If there is a plenty space in hashtable we
can hope that there was not evictions since last sample. However there
could be reset performed by someone else and we are know nothing about
this.
Having a timestamp in stats_since field we are sure about how long this
statement statistics is tracked. That said sampling solution can
totally avoid pgss resets. Avoiding such resets means avoiding
interference between monitoring solutions.
But if no more resets is done we can't track min/max values, because
they still needs a reset and we can do nothing with such resets - they
are necessary. However I still want to know when min/max reset was
performed. This will help to detect possible interference on such
resets.
> 
> 
> A couple more comments:
> 
> 1) I'm not sure why the patch is adding tests of permissions on the
> pg_stat_statements_reset function?
> 
> 2) If we want the second timestamp, shouldn't it also cover resets of
> the mean values, not just min/max?

I think that mean values shouldn't be target for a partial reset
because the value for mean values can be easily reconstructed by the
sampling solution without a reset.

> 
> 3) I don't understand why the patch is adding "IS NOT NULL AS t" to
> various places in the regression tests.

The most of tests was copied from the previous version. I'll recheck
them.

> 
> 4) I rather dislike the "minmax" naming, because that's often used in
> other contexts (for BRIN indexes), and as I mentioned maybe it should
> also cover the "mean" fields.

Agreed, but I couldn't make it better. Other versions seemed worse to
me...
> 
> 
Regards, Andrei Zubkov

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi,

I've rebased this patch so that it can be applied after 57d6aea00fc.

v14 attached
--
regards, Andrei
From 6c541f3001d952e72e5d865fde09de3fb4f36d10 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Fri, 8 Apr 2022 23:12:55 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since and minmax_stats_since columns to the
pg_stat_statements view and pg_stat_statements() function. The new min/max reset
mode for the pg_stat_stetments_reset() function is controlled by the
parameter minmax_only.
stat_since column is populated with the current timestamp when a new statement
is added to the pg_stat_statements hashtable. It provides clean information
about statistics collection time interval for each statement. Besides it can be
used by sampling solutions to detect situations when a statement was evicted and
stored again between samples.
Such sampling solution could derive any pg_stat_statements statistic values for
an interval between two samples with the exception of all min/max statistics. To
address this issue this patch adds the ability to reset min/max statistics
independently of the statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function. Timestamp of such reset is stored in the
minmax_stats_since field for each statement.
pg_stat_statements_reset() function now returns the timestamp of a reset as a
result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 .../expected/oldextversions.out   | 118 +++---
 .../expected/pg_stat_statements.out   | 361 +-
 .../pg_stat_statements--1.9--1.10.sql |  16 +-
 .../pg_stat_statements/pg_stat_statements.c   | 128 +--
 .../pg_stat_statements/sql/oldextversions.sql |   7 +-
 .../sql/pg_stat_statements.sql| 149 +++-
 doc/src/sgml/pgstatstatements.sgml|  66 +++-
 7 files changed, 637 insertions(+), 208 deletions(-)

diff --git a/contrib/pg_stat_statements/expected/oldextversions.out b/contrib/pg_stat_statements/expected/oldextversions.out
index efb2049ecff..0634d73bc03 100644
--- a/contrib/pg_stat_statements/expected/oldextversions.out
+++ b/contrib/pg_stat_statements/expected/oldextversions.out
@@ -138,7 +138,7 @@ SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
 
 -- New function pg_stat_statement_info, and new function
 -- and view for pg_stat_statements introduced in 1.9
-AlTER EXTENSION pg_stat_statements UPDATE TO '1.9';
+ALTER EXTENSION pg_stat_statements UPDATE TO '1.9';
 SELECT pg_get_functiondef('pg_stat_statements_info'::regproc);
pg_get_functiondef
 -
@@ -194,55 +194,79 @@ SELECT count(*) > 0 AS has_data FROM pg_stat_statements;
  t
 (1 row)
 
+\d pg_stat_statements_info
+  View "public.pg_stat_statements_info"
+   Column|   Type   | Collation | Nullable | Default 
+-+--+---+--+-
+ dealloc | bigint   |   |  | 
+ stats_reset | timestamp with time zone |   |  | 
+
+SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
+   pg_get_functiondef   
+
+ CREATE OR REPLACE FUNCTION public.pg_stat_statements_reset(userid oid DEFAULT 0, dbid oid DEFAULT 0, queryid bigint DEFAULT 0)+
+  RETURNS void +
+  LANGUAGE c   +
+  PARALLEL SAFE STRICT +
+ AS '$libdir/pg_stat_statements', $function$pg_stat_statements_reset_1_7$function$ +
+ 
+(1 row)
+
+SET SESSION AUTHORIZATION pg_read_all_stats;
+SELECT pg_stat_statements_reset();
+ERROR:  permission denied for function pg_stat_statements_reset
+RESET SESSION AUTHORIZATION;
 -- New functions and views for pg_stat_statements in 1.10
 AlTER EXTENSION pg_stat_statements UPDATE TO '1.10';
 \d pg_stat_statements
-  View "public.pg_stat_statements"
- Column |   Type   | Collation | Nullable | Default 
-+--+---+--+-
- useri

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Julien,

Thank you very much for your work on this patch!

On Mon, 2022-04-04 at 10:31 +0800, Julien Rouhaud wrote:
> - the commit message:
> 
> It should probably mention the mimnax_stats_since at the beginning. 
> Also, both
> the view and the function contain those new field.
> 
> Minor rephrasing:
> 
> s/evicted and returned back/evicted and stored again/?
> s/with except of all/with the exception of all/
> s/is now returns/now returns/

Agreed, commit message updated.

> - code:
> 
> +#define SINGLE_ENTRY_RESET() \
> +if (entry) { \
> [...]
> 
> It's not great to rely on caller context too much.

Yes, I was thinking about it. But using 4 parameters seemed strange to
me.

>   I think it would be better
> to pass at least the entry as a parameter (maybe e?) to the macro for
> more
> clarity.  I'm fine with keeping minmax_only, stats_reset and
> num_remove as is.

Using an entry as a macro parameter looks good, I'm fine with "e". 

> Apart from that I think this is ready!

v13 attached
--
regards, Andrei
From b6b1eab21f5b873ec9a1bb13b82cca7d6bcaab32 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Mon, 4 Apr 2022 09:46:31 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since and minmax_stats_since columns to the
pg_stat_statements view and pg_stat_statements() function. The new min/max reset
mode for the pg_stat_stetments_reset() function is controlled by the
parameter minmax_only.
stat_since column is populated with the current timestamp when a new statement
is added to the pg_stat_statements hashtable. It provides clean information
about statistics collection time interval for each statement. Besides it can be
used by sampling solutions to detect situations when a statement was evicted and
stored again between samples.
Such sampling solution could derive any pg_stat_statements statistic values for
an interval between two samples with the exception of all min/max statistics. To
address this issue this patch adds the ability to reset min/max statistics
independently of the statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function. Timestamp of such reset is stored in the
minmax_stats_since field for each statement.
pg_stat_statements_reset() function now returns the timestamp of a reset as a
result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   3 +-
 .../expected/oldextversions.out   |  61 +++
 .../expected/pg_stat_statements.out   | 361 +-
 .../pg_stat_statements--1.9--1.10.sql | 108 ++
 .../pg_stat_statements/pg_stat_statements.c   | 151 ++--
 .../pg_stat_statements.control|   2 +-
 .../pg_stat_statements/sql/oldextversions.sql |   8 +
 .../sql/pg_stat_statements.sql| 149 +++-
 doc/src/sgml/pgstatstatements.sgml|  66 +++-
 9 files changed, 747 insertions(+), 162 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38d..edc40c8bbfb 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
+DATA = pg_stat_statements--1.4.sql \
+	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.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 \
diff --git a/contrib/pg_stat_statements/expected/oldextversions.out b/contrib/pg_stat_statements/expected/oldextversions.out
index f18c08838f5..70877948491 100644
--- a/contrib/pg_stat_statements/expected/oldextversions.out
+++ b/contrib/pg_stat_statements/expected/oldextversions.out
@@ -136,4 +136,65 @@ SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
  
 (1 row)
 
+ALTER EXTENSION pg_stat_statements UPDATE TO '1.9';
+\d pg_stat_statements
+View "public.pg_stat_statements"
+   Column|   Type   | Collation | Nullable | Default 
+-+--+---+--+-
+ userid  | oid  |   |  | 
+ dbid| oid  |   |  | 
+ toplevel| boolean  |   |  | 
+ queryid | bigint   |   |  | 
+ query   | text |   |  | 
+ plans

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Julien,

On Sun, 2022-04-03 at 17:56 +0800, Julien Rouhaud wrote:
> Just another minor nitpicking after a quick look:
> 
> + This field will be zero if ...
> [...]
> + this field will contain zero until this statement ...
> 
> The wording should be consistent, so either "will be zero" or "will
> contain
> zero" everywhere.  I'm personally fine with any, but maybe a native
> English
> will think one is better.
Agreed.

Searching the docs I've fond out that "will contain" usually used with
the description of contained structure rather then a simple value. So
I'll use a "will be zero" in the next version after your review.
--
regards, Andrei

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

I've attached v12 of a patch. The only unsolved issue now is the
following:

On Sun, 2022-04-03 at 15:07 +0800, Julien Rouhaud wrote:
> +ALTER EXTENSION pg_stat_statements UPDATE TO '1.9';
> +\d pg_stat_statements
> +\d pg_stat_statements_info
> +SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
> 
> I don't think this bring any useful coverage.

It is a little bit unclear to me what is the best solution here.
--
regards, Andrei
From 9359b7dfdadeb7a672c146030995626150acf231 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Sun, 3 Apr 2022 12:21:47 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since column to the pg_stat_statements view. This column
is populated with the current timestamp when a new statement is added to the
pg_stat_statements hashtable. It provides clean information about statistics
collection time interval for each statement. Besides it can be used
by sampling solutions to detect situations when a statement was evicted and
returned back between samples.
Such sampling solution could derive any pg_stat_statements statistic value for
an interval between two samples with except of all min/max statistics. To
address this issue this patch adds the ability to reset min/max
statistics independently of statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function.
Timestamp of such reset is stored in the minmax_stats_since field for
each statement. pg_stat_statements_reset() function is now returns
this timestamp as a result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   3 +-
 .../expected/oldextversions.out   |  61 +++
 .../expected/pg_stat_statements.out   | 361 +-
 .../pg_stat_statements--1.9--1.10.sql | 108 ++
 .../pg_stat_statements/pg_stat_statements.c   | 151 ++--
 .../pg_stat_statements.control|   2 +-
 .../pg_stat_statements/sql/oldextversions.sql |   8 +
 .../sql/pg_stat_statements.sql| 149 +++-
 doc/src/sgml/pgstatstatements.sgml|  66 +++-
 9 files changed, 747 insertions(+), 162 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38d..edc40c8bbfb 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
+DATA = pg_stat_statements--1.4.sql \
+	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.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 \
diff --git a/contrib/pg_stat_statements/expected/oldextversions.out b/contrib/pg_stat_statements/expected/oldextversions.out
index f18c08838f5..70877948491 100644
--- a/contrib/pg_stat_statements/expected/oldextversions.out
+++ b/contrib/pg_stat_statements/expected/oldextversions.out
@@ -136,4 +136,65 @@ SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
  
 (1 row)
 
+ALTER EXTENSION pg_stat_statements UPDATE TO '1.9';
+\d pg_stat_statements
+View "public.pg_stat_statements"
+   Column|   Type   | Collation | Nullable | Default 
+-+--+---+--+-
+ userid  | oid  |   |  | 
+ dbid| oid  |   |  | 
+ toplevel| boolean  |   |  | 
+ queryid | bigint   |   |  | 
+ query   | text |   |  | 
+ plans   | bigint   |   |  | 
+ total_plan_time | double precision |   |  | 
+ min_plan_time   | double precision |   |  | 
+ max_plan_time   | double precision |   |  | 
+ mean_plan_time  | double precision |   |  | 
+ stddev_plan_time| double precision |   |  | 
+ calls   | bigint   |   |  | 
+ total_exec_time | double precision |   |  | 
+ min_exec_time   | double precision |   |  | 
+ max_exec_time   | double precision |   |  | 
+ mean_exec_time  | double precision |   |  | 
+ stddev_exec_time| double precision |   |  | 
+ rows| bigint   |   |  | 
+ 

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Julien,

On Sun, 2022-04-03 at 15:07 +0800, Julien Rouhaud wrote:
> +static TimestampTz
> +entry_reset(Oid userid, Oid dbid, uint64 queryid, bool minmax_only)
>  {
>     HASH_SEQ_STATUS hash_seq;
>     pgssEntry  *entry;
> +   Counters   *entry_counters;
> 
> Do we really need an extra variable?  Why not simply using
> entry->counters.xxx_time[kind]?
> 
> Also, I think it's better to make the macro more like function
> looking, so
> SINGLE_ENTRY_RESET().

Agreed with the both, I'll fix it.

> 
> index f2e822acd3..c2af29866b 100644
> --- a/contrib/pg_stat_statements/sql/oldextversions.sql
> +++ b/contrib/pg_stat_statements/sql/oldextversions.sql
> @@ -36,4 +36,12 @@ AlTER EXTENSION pg_stat_statements UPDATE TO
> '1.8';
>  \d pg_stat_statements
>  SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
> 
> +ALTER EXTENSION pg_stat_statements UPDATE TO '1.9';
> +\d pg_stat_statements
> +\d pg_stat_statements_info
> +SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
> 
> I don't think this bring any useful coverage.

I feel the same, but I've done it like previous tests (versions 1.7 and
1.8). Am I miss something here? Do you think we should remove these
tests completly?

> 
> I think this need some rewording (and s/fist/first).  Maybe:
> 
> Minimum time spent planning the statement, in milliseconds.
> 
> This field will be zero if
> pg_stat_statements.track_planning
> is disabled, or if the counter has been reset using the the
> pg_stat_statements_reset function with the
> minmax_only parameter set to
> true
> and never been planned since.

Thanks a lot!

> 
>    pg_stat_statements_reset
>   
> @@ -589,6 +623,20 @@
>    If all statistics in the
> pg_stat_statements
>    view are discarded, it will also reset the statistics in the
>    pg_stat_statements_info view.
> +  When minmax_only is
> true only the
> +  values of minimun and maximum execution and planning time will
> be reset (i.e.
> 
> Nitpicking: I would say planning and execution time, as the fields
> are in this
> order in the view/function.

Agreed.
--
regards, Andrei

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Greg,

On Sat, 2022-04-02 at 17:38 -0400, Greg Stark wrote:
> The tests for this seem to need adjustments.
> 
> [12:41:09.403] test pg_stat_statements ... FAILED 180 ms
>     query   | reset_ts_match
>  ---+
> - SELECT $1,$2 AS "STMTTS2" | f
>   SELECT $1 AS "STMTTS1"    | t
> + SELECT $1,$2 AS "STMTTS2" | f
>  (2 rows)
> 
>  -- check that minmax reset does not set stats_reset
> 
> 
> Hm. Is this a collation problem?

Of course, thank you! I've forgot to set collation here.

v11 attached
--
regards, Andrei
From c5900f1c689b2a74edbc30b66c9a73e25b85484a Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Sun, 3 Apr 2022 07:28:59 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since column to the pg_stat_statements view. This column
is populated with the current timestamp when a new statement is added to the
pg_stat_statements hashtable. It provides clean information about statistics
collection time interval for each statement. Besides it can be used
by sampling solutions to detect situations when a statement was evicted and
returned back between samples.
Such sampling solution could derive any pg_stat_statements statistic value for
an interval between two samples with except of all min/max statistics. To
address this issue this patch adds the ability to reset min/max
statistics independently of statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function.
Timestamp of such reset is stored in the minmax_stats_since field for
each statement. pg_stat_statements_reset() function is now returns
this timestamp as a result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   3 +-
 .../expected/oldextversions.out   |  61 +++
 .../expected/pg_stat_statements.out   | 361 +-
 .../pg_stat_statements--1.9--1.10.sql | 108 ++
 .../pg_stat_statements/pg_stat_statements.c   | 153 ++--
 .../pg_stat_statements.control|   2 +-
 .../pg_stat_statements/sql/oldextversions.sql |   8 +
 .../sql/pg_stat_statements.sql| 149 +++-
 doc/src/sgml/pgstatstatements.sgml|  58 ++-
 9 files changed, 745 insertions(+), 158 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38d..edc40c8bbfb 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
+DATA = pg_stat_statements--1.4.sql \
+	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.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 \
diff --git a/contrib/pg_stat_statements/expected/oldextversions.out b/contrib/pg_stat_statements/expected/oldextversions.out
index f18c08838f5..70877948491 100644
--- a/contrib/pg_stat_statements/expected/oldextversions.out
+++ b/contrib/pg_stat_statements/expected/oldextversions.out
@@ -136,4 +136,65 @@ SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
  
 (1 row)
 
+ALTER EXTENSION pg_stat_statements UPDATE TO '1.9';
+\d pg_stat_statements
+View "public.pg_stat_statements"
+   Column|   Type   | Collation | Nullable | Default 
+-+--+---+--+-
+ userid  | oid  |   |  | 
+ dbid| oid  |   |  | 
+ toplevel| boolean  |   |  | 
+ queryid | bigint   |   |  | 
+ query   | text |   |  | 
+ plans   | bigint   |   |  | 
+ total_plan_time | double precision |   |  | 
+ min_plan_time   | double precision |   |  | 
+ max_plan_time   | double precision |   |  | 
+ mean_plan_time  | double precision |   |  | 
+ stddev_plan_time| double precision |   |  | 
+ calls   | bigint   |   |  | 
+ total_exec_time | double precision |   |  | 
+ min_exec_time   | double precision |   |  | 
+ max_exec_time   | double precision |   |  | 
+ mean_exec_time  | d

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

On Sat, 2022-04-02 at 14:11 +0300, Andrei Zubkov wrote:
> On Sat, 2022-04-02 at 18:56 +0800, Julien Rouhaud wrote:
> > Maybe a macro would be better here?  I don't know if that's
> > generally
> > ok or
> > just old and not-that-great code, but there are other places
> > relying
> > on macros
> > when a plain function call isn't that convenient (like here
> > returning
> > 0 or 1 as
> > a hack for incrementing num_remove), for instance in hba.c.
> 
> Yes, it is not very convenient and not looks pretty, so I'll try a
> macro here soon.

Implemented SINGLE_ENTRY_RESET as a macro.
v10 attached
--
regards, Andrei
From 83ef072fd7406509d4eb0e54d36900f5aeb16f1e Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Sat, 2 Apr 2022 14:58:04 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since column to the pg_stat_statements view. This column
is populated with the current timestamp when a new statement is added to the
pg_stat_statements hashtable. It provides clean information about statistics
collection time interval for each statement. Besides it can be used
by sampling solutions to detect situations when a statement was evicted and
returned back between samples.
Such sampling solution could derive any pg_stat_statements statistic value for
an interval between two samples with except of all min/max statistics. To
address this issue this patch adds the ability to reset min/max
statistics independently of statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function.
Timestamp of such reset is stored in the minmax_stats_since field for
each statement. pg_stat_statements_reset() function is now returns
this timestamp as a result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   3 +-
 .../expected/oldextversions.out   |  61 +++
 .../expected/pg_stat_statements.out   | 361 +-
 .../pg_stat_statements--1.9--1.10.sql | 108 ++
 .../pg_stat_statements/pg_stat_statements.c   | 153 ++--
 .../pg_stat_statements.control|   2 +-
 .../pg_stat_statements/sql/oldextversions.sql |   8 +
 .../sql/pg_stat_statements.sql| 149 +++-
 doc/src/sgml/pgstatstatements.sgml|  58 ++-
 9 files changed, 745 insertions(+), 158 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38d..edc40c8bbfb 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
+DATA = pg_stat_statements--1.4.sql \
+	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.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 \
diff --git a/contrib/pg_stat_statements/expected/oldextversions.out b/contrib/pg_stat_statements/expected/oldextversions.out
index f18c08838f5..70877948491 100644
--- a/contrib/pg_stat_statements/expected/oldextversions.out
+++ b/contrib/pg_stat_statements/expected/oldextversions.out
@@ -136,4 +136,65 @@ SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
  
 (1 row)
 
+ALTER EXTENSION pg_stat_statements UPDATE TO '1.9';
+\d pg_stat_statements
+View "public.pg_stat_statements"
+   Column|   Type   | Collation | Nullable | Default 
+-+--+---+--+-
+ userid  | oid  |   |  | 
+ dbid| oid  |   |  | 
+ toplevel| boolean  |   |  | 
+ queryid | bigint   |   |  | 
+ query   | text |   |  | 
+ plans   | bigint   |   |  | 
+ total_plan_time | double precision |   |  | 
+ min_plan_time   | double precision |   |  | 
+ max_plan_time   | double precision |   |  | 
+ mean_plan_time  | double precision |   |  | 
+ stddev_plan_time| double precision |   |  | 
+ calls   | bigint   |   |  | 
+ total_exec_time | double precision |   |  | 
+ min_exec_time   | double precision |   |  | 
+ max_exec_time   | d

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

On Sat, 2022-04-02 at 18:56 +0800, Julien Rouhaud wrote:
> Maybe a macro would be better here?  I don't know if that's generally
> ok or
> just old and not-that-great code, but there are other places relying
> on macros
> when a plain function call isn't that convenient (like here returning
> 0 or 1 as
> a hack for incrementing num_remove), for instance in hba.c.

Yes, it is not very convenient and not looks pretty, so I'll try a
macro here soon.

> > I think that if we can do it in accurate way and there is no
> > obvious
> > side effects, why not to try it...
> > Changing of pg_stat_statements_reset function result caused a
> > confiderable tests update. Also, I'm not sure that my description
> > of
> > this feature in the docs is blameless..
> > 
> > After all, I'm a little bit in doubt about this feature, so I'm
> > ready
> > to rollback it.
> 
> Well, I personally don't think that I would need it for powa as it's
> designed
> to have very frequent snapshot.  I know you have a different approach
> in
> pg_profile, but I'm not sure it will be that useful for you either?

Of course I can do some workaround if the accurate reset time will be
unavailable. I just want to do the whole thing if it doesn't hurt. If
we have a plenty of timestamps saved now, I think it is a good idea to
have then bound to some milestones. At least it is a pretty equal join
condition between samples.
But if you think we should avoid returning ts here I won't insist on
that.

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi,

On Fri, 2022-04-01 at 13:01 -0700, Andres Freund wrote:
> It seems decidedly not great to have four copies of this code. It was
> already
> not great before, but this patch makes the duplicated section go from
> four
> lines to 20 or so.

Agreed. I've created the single_entry_reset() function wrapping this
code. I wonder if it should be declared as inline to speedup a little.

On Sat, 2022-04-02 at 15:10 +0800, Julien Rouhaud wrote:
> > However
> > we can test at least functionality of stats_reset field like this:
> > 
> > SELECT now() AS ref_ts \gset
> > SELECT dealloc, stats_reset >= :'ref_ts' FROM
> > pg_stat_statements_info;
> > SELECT pg_stat_statements_reset();
> > SELECT dealloc, stats_reset >= :'ref_ts' FROM
> > pg_stat_statements_info;
> > 
> > Does it seems reasonable?
> 
> It looks reasonable, especially if the patch adds a new mode for the
> reset
> function.

I've implemented this test.

> > Checking
> > of only execution stats seems enough to me - in most cases we can't
> > check planning stats with such test anyway.
> > What do you think about it?
> 
> Ah I see.  I guess we could set plan_cache_mode to force_generic_plan
> to make
> sure we go though planning.  But otherwise just adding a comment
> saying that
> the test has to be compatible with different plan caching approach
> would be
> fine with me.

Set plan_cache_mode seems a little bit excess to me. And maybe in the
future some another plan caching strategies will be implementd with
coresponding settings.. So I've just left a comment there.

On Sat, 2022-04-02 at 15:21 +0800, Julien Rouhaud wrote:
> I'm not sure about returning the ts.  If you need it you could call
> SELECT
> now() FROM pg_stat_statements_reset() (or clock_timestamp()).  It
> won't be
> entirely accurate but since the function will have an exclusive lock
> during the
> whole execution that shouldn't be a problem.  Now you're already
> adding a new
> version of the C function so I guess that it wouldn't require any
> additional
> effort so why not.

I think that if we can do it in accurate way and there is no obvious
side effects, why not to try it...
Changing of pg_stat_statements_reset function result caused a
confiderable tests update. Also, I'm not sure that my description of
this feature in the docs is blameless..

After all, I'm a little bit in doubt about this feature, so I'm ready
to rollback it.

v9 attached
--
regards, Andrei
From 78f3e2e7bda2683b3aeb30b28fbbe60ed781db1d Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Sat, 2 Apr 2022 12:30:11 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since column to the pg_stat_statements view. This column
is populated with the current timestamp when a new statement is added to the
pg_stat_statements hashtable. It provides clean information about statistics
collection time interval for each statement. Besides it can be used
by sampling solutions to detect situations when a statement was evicted and
returned back between samples.
Such sampling solution could derive any pg_stat_statements statistic value for
an interval between two samples with except of all min/max statistics. To
address this issue this patch adds the ability to reset min/max
statistics independently of statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function.
Timestamp of such reset is stored in the minmax_stats_since field for
each statement. pg_stat_statements_reset() function is now returns
this timestamp as a result.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   3 +-
 .../expected/oldextversions.out   |  61 +++
 .../expected/pg_stat_statements.out   | 361 +-
 .../pg_stat_statements--1.9--1.10.sql | 108 ++
 .../pg_stat_statements/pg_stat_statements.c   | 164 ++--
 .../pg_stat_statements.control|   2 +-
 .../pg_stat_statements/sql/oldextversions.sql |   8 +
 .../sql/pg_stat_statements.sql| 149 +++-
 doc/src/sgml/pgstatstatements.sgml|  58 ++-
 9 files changed, 756 insertions(+), 158 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38d..edc40c8bbfb 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
+DATA = pg_stat_statements--1.4.sql \
+	pg_stat_statements--1.9--1

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi,

Thank you, Greg

On Fri, 2022-04-01 at 11:38 -0400, Greg Stark wrote:
> [13:19:51.544] pg_stat_statements.c: In function ‘entry_reset’:
> [13:19:51.544] pg_stat_statements.c:2598:32: error:
> ‘minmax_stats_reset’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
> [13:19:51.544] 2598 | entry->minmax_stats_since = minmax_stats_reset;
> [13:19:51.544] | ~~^~~~
>

I was afraid of such warning can appear..

On Sat, 2022-04-02 at 00:13 +0800, Julien Rouhaud wrote:
> I guess that pg_stat_statement_reset() is so
> expensive that an extra gettimeofday() wouldn't change much. 

Agreed

> Otherwise
> initializing to NULL should be enough.

Julien, I would prefer an extra GetCurrentTimestamp(). So, I've opted
to use the common unconditional

stats_reset = GetCurrentTimestamp();

for an entire entry_reset function due to the following:

1. It will be uniform for stats_reset and minmax_stats_reset
2. As you mentioned, it wouldn't change a much
3. The most common way to use this function is to reset all statements
meaning that GetCurrentTimestamp() will be called anyway to update the
value of stats_reset field in pg_stat_statements_info view
4. Actually I would like that pg_stat_statements_reset function was
able to return the value of stats_reset as its result. This could give
to the sampling solutions the ability to check if the last reset (of
any type) was performed by this solution or any other reset was
performed by someone else. It seems valuable to me, but it changes the
result type of the pg_stat_statements_reset() function, so I don't know
if we can do that right now.

v8 attached
--
regards, Andrei
From a905dcbd8ca891a1aaf9652324650b87cdcae001 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Fri, 1 Apr 2022 22:09:49 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since column to the pg_stat_statements view. This column
is populated with the current timestamp when a new statement is added to the
pg_stat_statements hashtable. It provides clean information about statistics
collection time interval for each statement. Besides it can be used
by sampling solutions to detect situations when a statement was evicted and
returned back between samples.
Such sampling solution could derive any pg_stat_statements statistic value for
an interval between two samples with except of all min/max statistics. To
address this issue this patch adds the ability to reset min/max
statistics independently of statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function.
Timestamp of such reset is stored in the minmax_stats_since field for
each statement.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   3 +-
 .../expected/oldextversions.out   |  61 ++
 .../expected/pg_stat_statements.out   | 140 
 .../pg_stat_statements--1.9--1.10.sql | 108 ++
 .../pg_stat_statements/pg_stat_statements.c   | 199 +++---
 .../pg_stat_statements.control|   2 +-
 .../pg_stat_statements/sql/oldextversions.sql |   8 +
 .../sql/pg_stat_statements.sql|  92 
 doc/src/sgml/pgstatstatements.sgml|  52 -
 9 files changed, 623 insertions(+), 42 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38d..edc40c8bbfb 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
+DATA = pg_stat_statements--1.4.sql \
+	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.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 \
diff --git a/contrib/pg_stat_statements/expected/oldextversions.out b/contrib/pg_stat_statements/expected/oldextversions.out
index f18c08838f5..70877948491 100644
--- a/contrib/pg_stat_statements/expected/oldextversions.out
+++ b/contrib/pg_stat_statements/expected/oldextversions.out
@@ -136,4 +136,65 @@ SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
  
 (1 row)
 
+ALTER EXTENSION pg_stat_statements UPDATE TO '1.9';
+\d pg_stat_statements
+View "public.pg_stat_statements"
+   Column|   Type   | Collation | Nullable | Default 
+-+--+---+--

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Julien!

Thank you for such detailed review!

On Wed, 2022-03-30 at 17:31 +0800, Julien Rouhaud wrote:
> Feature wise, I'm happy with the patch.  I just have a few comments.
> 
> Tests:
> 
> - it's missing some test in sql/oldextversions.sql to validate that the
> code
>   works with the extension in version 1.9

Yes, I've just added some tests there, but it seems they are not quite
suficient. Maybe we should try to do some queries to views and
functions in old versions? A least when new C function version
appears...

During tests developing I've noted that current test of
pg_stat_statements_info view actually tests only view access. However
we can test at least functionality of stats_reset field like this:

SELECT now() AS ref_ts \gset
SELECT dealloc, stats_reset >= :'ref_ts' FROM pg_stat_statements_info;
SELECT pg_stat_statements_reset();
SELECT dealloc, stats_reset >= :'ref_ts' FROM pg_stat_statements_info;

Does it seems reasonable? 

> - the last test removed the minmax_plan_zero field, why?

My thaught was as follows... Reexecution of the same query will
definitely cause execution. However, most likely it wouldn't be
planned, but if it would (maybe this is possible, or maybe it will be
possible in the future in some cases) the test shouldn't fail. Checking
of only execution stats seems enough to me - in most cases we can't
check planning stats with such test anyway.
What do you think about it?

> 
> Code:
> 
> +   TimestampTz stats_since;/* timestamp of entry
> allocation moment */
> 
> I think "timestamp of entry allocation" is enough?

Yes

> 
> +    * Calculate min and max time. min = 0 and max
> = 0
> +    * means that min/max statistics reset was
> happen
> 
> maybe "means that the min/max statistics were reset"

Agreed

> 
> +/*
> + * Reset min/max values of specified entries
> + */
> +static void
> +entry_minmax_reset(Oid userid, Oid dbid, uint64 queryid)
> +{
> [...]
> 
> There's a lot of duplicated logic with entry_reset().
> Would it be possible to merge at least the C reset function to handle
> either
> all-metrics or minmax-only? 

Great point! I've merged minmax reset functionality in the entry_reset
function.

> Also, maybe it would be better to have a single SQL
> reset function, something like:
> 
> pg_stat_statements_reset(IN userid Oid DEFAULT 0,
> IN dbid Oid DEFAULT 0,
> IN queryid bigint DEFAULT 0,
>     IN minmax_only DEFAULT false
> )

Of course!

> 
> Doc:
> 
> +   stats_since timestamp with
> time zone
> +  
> +  
> +   Timestamp of statistics gathering start for the statement
> 
> The description is a bit weird.  Maybe like "Time at which statistics
> gathering
> started for this statement"?  Same for the minmax version.

Agreed.

I've attached 7th patch version with fixes mentioned above.
-- 
Best regards, Andrei Zubkov

From 41e56237b2c4b68ff101590e6f7c4cdc79b53816 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Thu, 31 Mar 2022 12:37:17 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since column to the pg_stat_statements view. This column
is populated with the current timestamp when a new statement is added to the
pg_stat_statements hashtable. It provides clean information about statistics
collection time interval for each statement. Besides it can be used
by sampling solutions to detect situations when a statement was evicted and
returned back between samples.
Such sampling solution could derive any pg_stat_statements statistic value for
an interval between two samples with except of all min/max statistics. To
address this issue this patch adds the ability to reset min/max
statistics independently of statement reset using the new minmax_only parameter
of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint,
minmax_only boolean) function.
Timestamp of such reset is stored in the minmax_stats_since field for
each statement.

Discussion:
https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   3 +-
 .../expected/oldextversions.out   |  61 ++
 .../expected/pg_stat_statements.out   | 140 
 .../pg_stat_statements--1.9--1.10.sql | 108 ++
 .../pg_stat_statements/pg_stat_statements.c   | 200 +++---
 .../pg_stat_statements.control|   2 +-
 .../pg_stat_statements/sql/oldextversions.sql |   8 +
 .../sql/pg_stat_statements.sql|  92 
 doc/src/sgml/pgstatstatements.sgml|  52 -
 9 files changed, 625 insertions(+), 41 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi

On Fri, 2022-03-25 at 00:37 -0400, Greg Stark wrote:
> Fwiw I find the idea of having a separate "aux" table kind of
> awkward.
> It'll seem strange to users not familiar with the history and without
> any clear idea why the fields are split.

Greg, thank you for your attention and for your thought.

I've just completed the 6th version of a patch implementing idea
proposed by Julien Rouhaud, i.e. without auxiliary statistics. 6th
version will reset current min/max fields to zeros until the first plan
or execute. I've decided to use zeros here because planning statistics
is zero in case of disabled tracking. I think sampling solution could
easily handle this.

-- 
Regards, Andrei Zubkov

From 68cd5efee7b3dbdb1b4034ab4c47249a23ca9d04 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Fri, 25 Mar 2022 12:30:03 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since column to the pg_stat_statements view. This column
is populated with the current timestamp when a new statement is added to the
pg_stat_statements hashtable. It provides clean information about statistics
collection time interval for each statement. Besides it can be used
by sampling solutions to detect situations when a statement was evicted and
returned back between samples.
Such sampling solution could derive any pg_stat_statements statistic value for
an interval between two samples with except of all min/max statistics. To
address this issue this patch adds the ability to reset min/max
statistics independently of statement reset using the new function
pg_stat_statements_aux_reset(userid oid, dbid oid, queryid bigint).
Timestamp of such reset is stored in the minmax_stats_since field for
each statement.

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   3 +-
 .../expected/pg_stat_statements.out   | 140 +
 .../pg_stat_statements--1.9--1.10.sql | 104 +
 .../pg_stat_statements/pg_stat_statements.c   | 197 --
 .../pg_stat_statements.control|   2 +-
 .../sql/pg_stat_statements.sql|  92 
 doc/src/sgml/pgstatstatements.sgml|  64 +-
 7 files changed, 580 insertions(+), 22 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38..edc40c8bbf 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
+DATA = pg_stat_statements--1.4.sql \
+	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.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 \
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index e0abe34bb6..d59fcdc403 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -1077,4 +1077,144 @@ SELECT COUNT(*) FROM pg_stat_statements WHERE query LIKE '%SELECT GROUPING%';
  2
 (1 row)
 
+--
+-- statement timestamps
+--
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--
+ 
+(1 row)
+
+SELECT 1 AS "STMTTS1";
+ STMTTS1 
+-
+   1
+(1 row)
+
+SELECT now() AS ref_ts \gset
+SELECT 1,2 AS "STMTTS2";
+ ?column? | STMTTS2 
+--+-
+1 |   2
+(1 row)
+
+SELECT stats_since >= :'ref_ts', count(*) FROM pg_stat_statements
+WHERE query LIKE '%STMTTS%'
+GROUP BY stats_since >= :'ref_ts'
+ORDER BY stats_since >= :'ref_ts';
+ ?column? | count 
+--+---
+ f| 1
+ t| 1
+(2 rows)
+
+SELECT now() AS ref_ts \gset
+SELECT
+  count(*) as total,
+  count(*) FILTER (
+WHERE min_plan_time + max_plan_time = 0
+  ) as minmax_plan_zero,
+  count(*) FILTER (
+WHERE min_exec_time + max_exec_time = 0
+  ) as minmax_exec_zero,
+  count(*) FILTER (
+WHERE minmax_stats_since >= :'ref_ts'
+  ) as minmax_stats_since_after_ref,
+  count(*) FILTER (
+WHERE stats_since >= :'ref_ts'
+  ) as stats_since_after_ref
+FROM pg_stat_statements
+WHERE query LIKE '%STMTTS%';
+ total | minmax_plan_zero | minmax_exec_zero | minmax_stats_since_after_ref | stats_since_after_ref 
+---+--+--+--+---
+ 2 |0 |0 |0 | 

Re: [PATCH] pg_statio_all_tables: several rows per table due to invalid TOAST index

[Andrei Zubkov]

Hi Tom!
On Thu, 2022-03-24 at 16:34 -0400, Tom Lane wrote:
> I wrote:
> > After a bit of further fooling, I found that we could make that
> > work with LEFT JOIN LATERAL.  This formulation has a different
> > problem, which is that if you do want most or all of the output,
> > computing each sub-aggregation separately is probably less
> > efficient than it could be.  But this is probably the better way
> > to go unless someone has an even better idea.
> 
> Hearing no better ideas, pushed.
> 
> regards, tom lane

Thank you for your attention and for the problem resolution. However
I'm worry a little about possible performance issues related to
monitoring solutions performing regular sampling of statistic views to
find out the most intensive objects in a database. They obviously will
query all rows from statistic views and their impact will only depend
on the sampling frequency. Is it seems reasonable to avoid using
pg_statio_all_tables view by such monitoring tools? But it seems that
the only way to do so is using statistic functions directly in a
sampling query. Is it seems reliable? Maybe we should think about a
little bit different statio view for that? For example, a plain view
for all tables (regular and TOASTs)...
-- 
Regards, Andrei Zubkov

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Julien,

On Tue, 2022-01-25 at 20:22 +0800, Julien Rouhaud wrote:
> To be honest I don't see how any monitoring solution could make any
> use of
> those fields as-is.  For that reason in PoWA we unfortunately have to
> entirely
> ignore them.  There was a previous attempt to provide a way to reset
> those
> counters only (see [1]), but it was returned with feedback due to
> lack of TLC
> from the author.

Thank you, I've just seen a thread in [1], it was of course a weak
attempt and I think it could be done better.
> 
> 
> I don't think it's a problem, as once you have a solution on top of
> pg_stat_statements, you get information order of magnitude better
> from that solution instead of pg_stat_statements.

Agreed. I'm worry about having different solutions running
simultaneously. All of them may want to get accurate min/max values,
but if they all will reset min/max statistics, they will interfere each
other. It seems that min/max resetting should be configurable in each
sampler as a partial problem solution. At least, every sampling
solution can make a decision based on reset timestamps provided by
pg_stat_statements now.
> 
> 
> If you're worried about some external table having a NOT NULL
> constraint for
> those fields, how about returning NaN instead?  That's a valid value
> for a
> double precision data type.

I don't know for sure what we can expect to be wrong here. My opinion
is to use NULLs, as they seems more suitable here. But this can be done
only if we are not expecting significant side effects.
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Julien,
 Tue, 2022-01-25 at 18:08 +0800, Julien Rouhaud wrote:
> 
> Are those 4 new counters really worth it?
> 
> The min/max were added to make pg_stat_statements a bit more useful
> if you
> have nothing else using that extension.  But once you setup a tool
> that
> snapshots the metrics regularly, do you really need to know e.g.
> "what was the
> maximum execution time in the last 3 years", which will typically be
> what
> people will retrieve from the non-aux min/max counters, rather than
> simply
> using your additional tool for better and more precise information?

Of course we can replace old min/max metrics with the new "aux" min/max
metrics. It seems reasonable to me because they will have the same
behavior until we touch reset_aux. I think we can assume that min/max
data is saved somewhere if reset_aux was performed, but how about the
backward compatibility?
There may be some monitoring solutions that doesn't expect min/max
stats reset independently of other statement statistics.
It seems highly unlikely to me, because the min/max stats for "the last
3 years" is obvious unusable but maybe someone uses them as a sign of
something?
Are we need to worry about that?

Also, there is one more dramatic consequence of such decision...
What min/max values should be returned after the auxiliary reset but
before the next statement execution?
The NULL values seems reasonable but there was not any NULLs before and
backward compatibility seems broken. Another approach is to return the
old values of min/max stats and the old aux_stats_since value until the
next statement execution but it seems strange when the reset was
performed and it doesn't affected any stats instantly.

regards,
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hello,

On Sun, 2022-01-02 at 13:28 -0800, Andres Freund wrote:
> Hi,
> 
> This fails with an assertion failure:
> https://cirrus-ci.com/task/5567540742062080?logs=cores#L55
> 
> 
Andres, thank you for your test! I've missed it. Fixed in attached
patch v5.

On Wed, 2021-12-22 at 04:25 +0300, Anton A. Melnikov wrote:
> 
> 
> I completely agree that creating a separate view for these new fields
> is
> the most correct thing to do.

Anton,

I've created a new view named pg_stat_statements_aux. But for now both
views are using the same function pg_stat_statements which returns all
fields. It seems reasonable to me - if sampling solution will need all
values it can query the function.

> Also it might be better to use the term 'auxiliary' and use the same 
> approach as for existent similar vars.

Agreed, renamed to auxiliary term.

> So internally it might look something like this:
> 
> double  aux_min_time[PGSS_NUMKIND];
> double  aux_max_time[PGSS_NUMKIND];
> TimestampTz aux_stats_reset;
> 
> And at the view level:
>aux_min_plan_time float8,
>aux_max_plan_time float8,
>aux_min_exec_time float8,
>aux_max_exec_time float8,
>aux_stats_reset timestamp with time zone
> 
> Functions names might be pg_stat_statements_aux() and 
> pg_stat_statements_aux_reset().
> 

But it seems "stats_reset" term is not quite correct in this case. The
"stats_since" field holds the timestamp of hashtable entry, but not the
reset time. The same applies to aux_stats_since - for new statement it
holds its entry time, but in case of reset it will hold the reset time.

"stats_reset" name seems a little bit confusing to me.

Attached patch v5
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From e3140e136bab9d44d7818ed86e1c8ff119936532 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Fri, 14 Jan 2022 18:04:53 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since column to the pg_stat_statements view. This column
is populated with the current timestamp when a new statement is added to the
pg_stat_statements hashtable. It provides clean information about statistics
collection time interval for each statement. Besides it can be used
by sampling solutions to detect situations when a statement was evicted and
returned back between samples.
Such sampling solution could derive any pg_stat_statements statistic value for
an interval between two samples with except of all min/max statistics. To
address this issue this patch adds auxiliary statistics to the pg_stat_statements
function and a new view named pg_stat_statements_aux:

 View "public.pg_stat_statements_aux"
  Column   |   Type   | Collation | Nullable | Default
---+--+---+--+-
 userid| oid  |   |  |
 dbid  | oid  |   |  |
 toplevel  | boolean  |   |  |
 queryid   | bigint   |   |  |
 query | text |   |  |
 aux_min_plan_time | double precision |   |  |
 aux_max_plan_time | double precision |   |  |
 aux_min_exec_time | double precision |   |  |
 aux_max_exec_time | double precision |   |  |
 stats_since   | timestamp with time zone |   |  |
 aux_stats_since   | timestamp with time zone |   |  |

These auxiliary statistics are resettable independently of statement reset by
the new function pg_stat_statements_aux_reset(userid oid, dbid oid, queryid bigint)

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   3 +-
 .../expected/pg_stat_statements.out   |  90 
 .../pg_stat_statements--1.9--1.10.sql | 122 +++
 .../pg_stat_statements/pg_stat_statements.c   | 203 +-
 .../pg_stat_statements.control|   2 +-
 .../sql/pg_stat_statements.sql|  50 +
 doc/src/sgml/pgstatstatements.sgml| 184 +++-
 7 files changed, 640 insertions(+), 14 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38d..edc40c8bbfb 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statem

Re: [PATCH] pg_statio_all_tables: several rows per table due to invalid TOAST index

[Andrei Zubkov]

It seems we need to bump catalog version here.
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

On Fri, 2021-12-03 at 17:03 +0300, Andrei Zubkov wrote:
> I've added the following fields to the pg_stat_statements view:
> 
>     min_plan_time_local float8,
>     max_plan_time_local float8,
>     min_exec_time_local float8,
>     max_exec_time_local float8
> 
> and a function that is able to reset those fields:
> 
> CREATE FUNCTION pg_stat_statements_reset_local(IN userid Oid DEFAULT
> 0,
>     IN dbid Oid DEFAULT 0,
>     IN queryid bigint DEFAULT 0
> )
> 
> It resets the local fields mentioned above and updates the new field
> 
>     local_stats_since timestamp with time zone
> 
> with the current timestamp. All other statement statistics are
> remains
> unchanged. 

After adding new fields to pg_stat_statements view it looks a little
bit overloaded. Furthermore, fields in this view have different
behavior.

What if we'll create a new view for such resettable fields? It will
make description of views and reset functions in the docs much more
clear.
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi, Anton!

Thank you for your review!

On Mon, 2021-10-18 at 22:11 +0300, Anton A. Melnikov wrote:
> So i suppose that additional vars loc_min and loc_max is a right way
> to do it.

I've added the following fields to the pg_stat_statements view:

min_plan_time_local float8,
max_plan_time_local float8,
min_exec_time_local float8,
max_exec_time_local float8

and a function that is able to reset those fields:

CREATE FUNCTION pg_stat_statements_reset_local(IN userid Oid DEFAULT 0,
IN dbid Oid DEFAULT 0,
IN queryid bigint DEFAULT 0
)

It resets the local fields mentioned above and updates the new field

local_stats_since timestamp with time zone

with the current timestamp. All other statement statistics are remains
unchanged. After the reset _local fields will have NULL values till the
next statement execution.

> And one more thing, if there was a reset of stats between two
> samples,
> then i think it is the best to ignore the new values, 
> since they were obtained for an incomplete period.
> This patch, thanks to the saved time stamp, makes possible 
> to determine the presence of reset between samples and
> there should not be a problem to realize such algorithm.
Yes, it seems this is up to the sampling solution. Maybe in some cases
incomplete information will be better than nothing... Anyway we have
all necessary data now.


> The only thing I could suggest to your notice
> is a small cosmetic edit to replace
> the numeric value in #define on line 1429 of pg_stat_statements.c
> by one of the constants defined above. 
Hmm. I've left it just like it was before me. But it seems, you are
right.

I've attached a new version of a patch. The first_seen column was
renamed to stats_since - it seems to be more self-explaining to me. But
I'm not sure in the current naming at all.

The tests is not ready yet, but any thoughts about the patch are
welcome right now.

-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


From bea0ae9fd062ee1810c9ff2d5f1be98a19114d84 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Fri, 3 Dec 2021 16:25:17 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

This patch adds stats_since column to the pg_stat_statements view. This column
is populated with the current timestamp when a new statement is added to the
pg_stat_statements hashtable. This column provides clean information about
statistics collection time interval for each statement. Besides it can be used
by sampling solutions to detect situations when a statement was evicted and
returned back between two samples.
Such sampling solution could derive any pg_stat_statements statistic value for
an interval between two samples with except of all min/max statistics. To
address this issue this patch adds the following columns:

min_plan_time_local float8,
max_plan_time_local float8,
min_exec_time_local float8,
max_exec_time_local float8

And a function to reset those statistics during a sample:

CREATE FUNCTION pg_stat_statements_reset_local(IN userid Oid DEFAULT 0,
IN dbid Oid DEFAULT 0,
IN queryid bigint DEFAULT 0
)

The timestamp of last local statistics reset is stored for every statement in
a column:

local_stats_since timestamp with time zone

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |   3 +-
 .../expected/pg_stat_statements.out   |  29 +++
 .../pg_stat_statements--1.9--1.10.sql |  72 +++
 .../pg_stat_statements/pg_stat_statements.c   | 200 +-
 .../pg_stat_statements.control|   2 +-
 .../sql/pg_stat_statements.sql|   9 +
 doc/src/sgml/pgstatstatements.sgml| 111 +-
 7 files changed, 419 insertions(+), 7 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38..edc40c8bbf 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
+DATA = pg_stat_statements--1.4.sql \
+	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.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 \
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index e0abe34bb6..9388490073 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_

Re: [PATCH] pg_statio_all_tables: several rows per table due to invalid TOAST index

[Andrei Zubkov]

Hi, Michael

Thank you for your attention!

On Tue, 2021-11-30 at 17:29 +0900, Michael Paquier wrote:
> Hmm.  Why should we care about invalid indexes at all, including
> pg_statio_all_indexes?
> 

I think we should care about them at least because they are exists and
can consume resources. For example, invalid index is to be updated by
DML operations.
Of course we can exclude such indexes from a view using isvalid,
isready, islive fields. But in such case we should mention this in the
docs, and more important is that the new such states of indexes can
appear in the future causing change in a view definition. Counting all
indexes regardless of states seems more reasonable to me.

-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

[PATCH] pg_statio_all_tables: several rows per table due to invalid TOAST index

[Andrei Zubkov]

Hi, hackers!

It seems we have a problem in pg_statio_all_tables view defenition.
According to the documentation and identification fields, this view
must have exact one row per a table.
The view definition contains an x.indexrelid as the last field in its
GROUP BY list:

<...>
GROUP BY c.oid, n.nspname, c.relname, t.oid, x.indexrelid

Which is the oid of a TOAST-index.

However it is possible that the TOAST table will have more than one
index. For example, this happens when REINDEX CONCURRENTLY operation
lefts an index in invalid state (indisvalid = false) due to some kind
of a failure. It's often sufficient to interrupt REINDEX CONCURRENTLY
operation right after start.

Such index will cause the second row to appear in a
pg_statio_all_tables view which obvious is unexpected behaviour.

Now we can have several regular indexes and several TOAST-indexes for
the same table. Statistics for the regular and TOAST indexes is to be
calculated the same way so I've decided to use a CTE here.

The proposed view definition follows:

CREATE VIEW pg_statio_all_tables AS
WITH indstat AS (
SELECT
indrelid,
sum(pg_stat_get_blocks_fetched(indexrelid) -
pg_stat_get_blocks_hit(indexrelid))::bigint
AS idx_blks_read,
sum(pg_stat_get_blocks_hit(indexrelid))::bigint
AS idx_blks_hit
FROM
pg_index
GROUP BY indrelid
)
SELECT
C.oid AS relid,
N.nspname AS schemaname,
C.relname AS relname,
pg_stat_get_blocks_fetched(C.oid) -
pg_stat_get_blocks_hit(C.oid) AS heap_blks_read,
pg_stat_get_blocks_hit(C.oid) AS heap_blks_hit,
I.idx_blks_read AS idx_blks_read,
I.idx_blks_hit AS idx_blks_hit,
pg_stat_get_blocks_fetched(T.oid) -
pg_stat_get_blocks_hit(T.oid) AS toast_blks_read,
pg_stat_get_blocks_hit(T.oid) AS toast_blks_hit,
X.idx_blks_read AS tidx_blks_read,
X.idx_blks_read AS tidx_blks_hit
FROM pg_class C LEFT JOIN
indstat I ON C.oid = I.indrelid LEFT JOIN
pg_class T ON C.reltoastrelid = T.oid LEFT JOIN
indstat X ON T.oid = X.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
WHERE C.relkind IN ('r', 't', 'm');

Reported by Sergey Grinko.

Regards.
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

From ffde04cf285de32c7b8521c0aa9d0b36c1e8b7f7 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Mon, 29 Nov 2021 16:33:34 +0300
Subject: [PATCH] pg_statio_all_tables: several rows per table due to invalid
 TOAST index

This patch changes definition of a pg_statio_all_tables view. More than one
index can exist on a TOAST table due to invalid indexes, which can appear due
to REINDEX CONCURRENTLY failure. Previous view definition in such case caused
several rows to appear in a view for a single table.

Reported by Sergey Grinko.
---
 src/backend/catalog/system_views.sql | 29 ++--
 src/test/regress/expected/rules.out  | 24 ++-
 2 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index eb560955cd..84ec8e3989 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -708,6 +708,18 @@ CREATE VIEW pg_stat_xact_user_tables AS
   schemaname !~ '^pg_toast';
 
 CREATE VIEW pg_statio_all_tables AS
+WITH indstat AS (
+SELECT
+indrelid,
+sum(pg_stat_get_blocks_fetched(indexrelid) -
+pg_stat_get_blocks_hit(indexrelid))::bigint
+AS idx_blks_read,
+sum(pg_stat_get_blocks_hit(indexrelid))::bigint
+AS idx_blks_hit
+FROM
+pg_index
+GROUP BY indrelid
+)
 SELECT
 C.oid AS relid,
 N.nspname AS schemaname,
@@ -715,22 +727,19 @@ CREATE VIEW pg_statio_all_tables AS
 pg_stat_get_blocks_fetched(C.oid) -
 pg_stat_get_blocks_hit(C.oid) AS heap_blks_read,
 pg_stat_get_blocks_hit(C.oid) AS heap_blks_hit,
-sum(pg_stat_get_blocks_fetched(I.indexrelid) -
-pg_stat_get_blocks_hit(I.indexrelid))::bigint AS idx_blks_read,
-sum(pg_stat_get_blocks_hit(I.indexrelid))::bigint AS idx_blks_hit,
+I.idx_blks_read AS idx_blks_read,
+I.idx_blks_hit AS idx_blks_hit,
 pg_stat_get_blocks_fetched(T.oid) -
 pg_stat_get_blocks_hit(T.oid) AS toast_blks_read,
 pg_stat_get_blocks_hit(T.oid) AS toast_blks_hit,
-pg_stat_get_blocks_fetched(X.indexrelid) -
-pg_stat_get_blocks_hit(X.indexrelid) AS tidx_blks_read,
-pg_stat_get_blocks_hit(X.indexrelid) AS tidx_bl

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

There is an issue with this patch. It's main purpose is the ability to
calculate values of pg_stat_statements view for a time period between
two samples without resetting pg_stat_statements being absolutely sure
that the statement was not evicted.
Such approach solves the problem for metrics with except of min and max
time values. It seems that partial reset is needed here resetting
min/max values during a sample. But overall min/max values will be lost
in this case. Does addition of resettable min/max metrics to the
pg_stat_statemets view seems reasonable here?

-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi, Anton!

I've corrected the patch and attached a new version.

-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On Wed, 2021-10-06 at 18:13 +0300, Мельников Антон Андреевич wrote:
> Hi, Andrey!
>  
> I’ve tried to apply your patch v2-0001 on current master, but i
> failed.
> There were git apply errors at:
> pg_stat_statements.out:941
> pg_stat_statements.sql:385
>  
> Best Regards,
> 
> Anton Melnikov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>  

From e44b634f21216791c9b5fd9a533532437039dcc1 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Thu, 7 Oct 2021 13:07:36 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

Added first_seen column in pg_stat_statements view. This field is
populated with current timestamp when a new statement is added to
pg_stat_statements hashtable. This field provides clean information
about statistics collection time interval for each statement. Besides
it can be used by sampling solutions to detect situations when a
statement was evicted and returned back between two samples.

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |  3 +-
 .../expected/pg_stat_statements.out   | 29 +
 .../pg_stat_statements--1.9--1.10.sql | 59 +++
 .../pg_stat_statements/pg_stat_statements.c   | 31 +-
 .../pg_stat_statements.control|  2 +-
 .../sql/pg_stat_statements.sql|  9 +++
 doc/src/sgml/pgstatstatements.sgml|  9 +++
 7 files changed, 137 insertions(+), 5 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38..edc40c8bbf 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
+DATA = pg_stat_statements--1.4.sql \
+	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.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 \
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index b52d187722..c5b5d8082f 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -1077,4 +1077,33 @@ SELECT COUNT(*) FROM pg_stat_statements WHERE query LIKE '%SELECT GROUPING%';
  2
 (1 row)
 
+--
+-- statement timestamps
+--
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--
+ 
+(1 row)
+
+SELECT 1 AS "STMTTS1";
+ STMTTS1 
+-
+   1
+(1 row)
+
+SELECT now() AS ref_ts \gset
+SELECT 1,2 AS "STMTTS2";
+ ?column? | STMTTS2 
+--+-
+1 |   2
+(1 row)
+
+SELECT first_seen >= :'ref_ts', count(*) FROM pg_stat_statements WHERE query LIKE '%STMTTS%' GROUP BY first_seen >= :'ref_ts' ORDER BY first_seen >= :'ref_ts';
+ ?column? | count 
+--+---
+ f| 1
+ t| 1
+(2 rows)
+
 DROP EXTENSION pg_stat_statements;
diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql b/contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql
new file mode 100644
index 00..969a30fbdc
--- /dev/null
+++ b/contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql
@@ -0,0 +1,59 @@
+/* contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pg_stat_statements UPDATE TO '1.10'" to load this file. \quit
+
+/* We need to redefine a view and a function */
+/* First we have to remove them from the extension */
+ALTER EXTENSION pg_stat_statements DROP VIEW pg_stat_statements;
+ALTER EXTENSION pg_stat_statements DROP FUNCTION pg_stat_statements(boolean);
+
+/* Then we can drop them */
+DROP VIEW pg_stat_statements;
+DROP FUNCTION pg_stat_statements(boolean);
+
+/* Now redefine */
+CREATE FUNCTION pg_stat_statements(IN showtext boolean,
+OUT userid oid,
+OUT dbid oid,
+OUT toplevel bool,
+OUT queryid bigint,
+OUT query text,
+OUT plans int8,
+OUT total_plan_time float8,
+OUT min_plan_time float8,
+OUT max_plan_time float8,
+OUT mean_plan_time float8,
+OUT stddev_plan_time float8,
+OUT calls int8,
+OUT total_exec_time float8,
+OUT min_exec_time float8,
+OUT max_exec_time float8,
+OUT mean

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi, Martin

On Mon, 2021-04-19 at 11:39 +, Chengxi Sun wrote:
> I tested your patch, and it works well. I also prefer timestamp
> inseatead of dealloc num.
> I think it can provide more useful details about query statements.
> 
Thank you for your review.
Certainly, timestamp is valuable here. Deallocation number is only a
workaround in unlikely case when timestamping will cost a much. It
seems, that it can happen only when significant amount of statements
causes a new entry in pg_stat_statements hashtable. However, in such
case using of pg_stat_statements extension might be qute difficult.
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hello, Kuroda!

On Fri, 2021-04-09 at 00:23 +, kuroda.hay...@fujitsu.com wrote:
> I think you are right. 
> According to [1] we can bump up the version per one PG major version,
> and any features are not committed yet for 15.
> 
> [1]: https://www.postgresql.org/message-id/20201202040516.GA43757@nol

Version 2 of patch attached. 
pg_stat_statements version is now 1.10 and patch is based on 0f61727.

-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

From 321ce82f22894be39297515268c1f3bed74778e2 Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Wed, 14 Apr 2021 16:35:56 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

Added first_seen column in pg_stat_statements view. This field is
populated with current timestamp when a new statement is added to
pg_stat_statements hashtable. This field provides clean information
about statistics collection time interval for each statement. Besides
it can be used by sampling solutions to detect situations when a
statement was evicted and returned back between two samples.

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 contrib/pg_stat_statements/Makefile   |  3 +-
 .../expected/pg_stat_statements.out   | 29 +
 .../pg_stat_statements--1.9--1.10.sql | 59 +++
 .../pg_stat_statements/pg_stat_statements.c   | 31 +-
 .../pg_stat_statements.control|  2 +-
 .../sql/pg_stat_statements.sql|  9 +++
 doc/src/sgml/pgstatstatements.sgml|  9 +++
 7 files changed, 137 insertions(+), 5 deletions(-)
 create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 3ec627b956..7d7b2f4808 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,8 @@ OBJS = \
 	pg_stat_statements.o
 
 EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
+DATA = pg_stat_statements--1.4.sql \
+	pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \
 	pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.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 \
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index 674ed270a8..f2268dfd87 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -941,4 +941,33 @@ SELECT query, toplevel, plans, calls FROM pg_stat_statements WHERE query LIKE '%
  $$ LANGUAGE plpgsql   |  |   | 
 (3 rows)
 
+--
+-- statement timestamps
+--
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--
+ 
+(1 row)
+
+SELECT 1 AS "STMTTS1";
+ STMTTS1 
+-
+   1
+(1 row)
+
+SELECT now() AS ref_ts \gset
+SELECT 1,2 AS "STMTTS2";
+ ?column? | STMTTS2 
+--+-
+1 |   2
+(1 row)
+
+SELECT first_seen >= :'ref_ts', count(*) FROM pg_stat_statements WHERE query LIKE '%STMTTS%' GROUP BY first_seen >= :'ref_ts' ORDER BY first_seen >= :'ref_ts';
+ ?column? | count 
+--+---
+ f| 1
+ t| 1
+(2 rows)
+
 DROP EXTENSION pg_stat_statements;
diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql b/contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql
new file mode 100644
index 00..969a30fbdc
--- /dev/null
+++ b/contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql
@@ -0,0 +1,59 @@
+/* contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pg_stat_statements UPDATE TO '1.10'" to load this file. \quit
+
+/* We need to redefine a view and a function */
+/* First we have to remove them from the extension */
+ALTER EXTENSION pg_stat_statements DROP VIEW pg_stat_statements;
+ALTER EXTENSION pg_stat_statements DROP FUNCTION pg_stat_statements(boolean);
+
+/* Then we can drop them */
+DROP VIEW pg_stat_statements;
+DROP FUNCTION pg_stat_statements(boolean);
+
+/* Now redefine */
+CREATE FUNCTION pg_stat_statements(IN showtext boolean,
+OUT userid oid,
+OUT dbid oid,
+OUT toplevel bool,
+OUT queryid bigint,
+OUT query text,
+OUT plans int8,
+OUT total_plan_time float8,
+OUT min_plan_time float8,
+OUT max_plan_time float8,
+OUT mean_plan_time float8,
+OUT stddev_plan_time float8,
+OUT calls int8,
+OUT total_exec_time float8,
+OUT min_exec_time float8,
+OUT max_exec_time float8,
+OUT mean_exec_time float8,
+OUT

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

On Wed, 2021-04-14 at 17:32 +0800, Julien Rouhaud wrote:
> 
> did you enable compute_query_id new parameter? 

Hi, Julien!
Thank you very much! I've missed it.
> 

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi, Kuroda!

I've intended to change the pg_stat_statements version with rebasing
this patch to the current master branch state. Now this is commit
07e5e66.

But I'm unable to test the patch - it seems that pg_stat_statements is
receiving queryId = 0 for every statements in every hook now and
statements are not tracked at all.

Am I mistaken somewhere? Maybe you know why this is happening?

Thank you!
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


On Fri, 2021-04-09 at 00:23 +, kuroda.hay...@fujitsu.com wrote:
> Dear Andrei,
> 
> > I think, yes, version of pg_stat_statements is need to be updated.
> > Is
> > it will be 1.10 in PG15?
> 
> I think you are right. 
> According to [1] we can bump up the version per one PG major version,
> and any features are not committed yet for 15.
> 
> [1]: https://www.postgresql.org/message-id/20201202040516.GA43757@nol
> 
> 
> Best Regards,
> Hayato Kuroda
> FUJITSU LIMITED
> 

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

On Wed, 2021-04-07 at 17:26 +0900, Seino Yuki wrote:


> Is it necessary to update the version of pg_stat_statements now that
> the 
> release is targeted for PG15?
I think, yes, version of pg_stat_statements is need to be updated. Is
it will be 1.10 in PG15?

Regards,
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Dear Kuroda,

> I don't like the idea because such a column has no meaning for the
> specific row.
> I prefer storing timestamp if GetCurrentTimestamp() is cheap.
I agree. New version attached.
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

From ac68636c8a26223854292ee5860f4a5989cb985a Mon Sep 17 00:00:00 2001
From: Andrei Zubkov 
Date: Tue, 23 Mar 2021 17:03:34 +0300
Subject: [PATCH] pg_stat_statements: Track statement entry timestamp

Added first_seen column in pg_stat_statements view. This field is
populated with current timestamp when a new statement is added to
pg_stat_statements hashtable. This field provides clean information
about statistics collection time interval for each statement. Besides
it can be used by sampling solutions to detect situations when a
statement was evicted and returned back between two samples.

Discussion: https://www.postgresql.org/message-id/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru
---
 .../expected/pg_stat_statements.out   | 22 
 .../pg_stat_statements--1.8--1.9.sql  | 54 +++
 .../pg_stat_statements/pg_stat_statements.c   | 31 +--
 .../sql/pg_stat_statements.sql|  8 +++
 doc/src/sgml/pgstatstatements.sgml|  9 
 5 files changed, 121 insertions(+), 3 deletions(-)

diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index 16158525ca..ffe08dece5 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -876,4 +876,26 @@ SELECT dealloc FROM pg_stat_statements_info;
0
 (1 row)
 
+--
+-- statement timestamps
+--
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--
+ 
+(1 row)
+
+SELECT now() AS start_ts \gset
+SELECT 1 AS "STMTTS";
+ STMTTS 
+
+  1
+(1 row)
+
+SELECT count(*) FROM pg_stat_statements WHERE first_seen >= :'start_ts' AND query LIKE '%STMTTS%';
+ count 
+---
+ 1
+(1 row)
+
 DROP EXTENSION pg_stat_statements;
diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql b/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
index 3504ca7eb1..9842d2da49 100644
--- a/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
+++ b/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
@@ -3,6 +3,60 @@
 -- complain if script is sourced in psql, rather than via ALTER EXTENSION
 \echo Use "ALTER EXTENSION pg_stat_statements UPDATE TO '1.9'" to load this file. \quit
 
+/* We need to redefine a view and a function */
+/* First we have to remove them from the extension */
+ALTER EXTENSION pg_stat_statements DROP VIEW pg_stat_statements;
+ALTER EXTENSION pg_stat_statements DROP FUNCTION pg_stat_statements(boolean);
+
+/* Then we can drop them */
+DROP VIEW pg_stat_statements;
+DROP FUNCTION pg_stat_statements(boolean);
+
+/* Now redefine */
+CREATE FUNCTION pg_stat_statements(IN showtext boolean,
+OUT userid oid,
+OUT dbid oid,
+OUT queryid bigint,
+OUT query text,
+OUT plans int8,
+OUT total_plan_time float8,
+OUT min_plan_time float8,
+OUT max_plan_time float8,
+OUT mean_plan_time float8,
+OUT stddev_plan_time float8,
+OUT calls int8,
+OUT total_exec_time float8,
+OUT min_exec_time float8,
+OUT max_exec_time float8,
+OUT mean_exec_time float8,
+OUT stddev_exec_time float8,
+OUT rows int8,
+OUT shared_blks_hit int8,
+OUT shared_blks_read int8,
+OUT shared_blks_dirtied int8,
+OUT shared_blks_written int8,
+OUT local_blks_hit int8,
+OUT local_blks_read int8,
+OUT local_blks_dirtied int8,
+OUT local_blks_written int8,
+OUT temp_blks_read int8,
+OUT temp_blks_written int8,
+OUT blk_read_time float8,
+OUT blk_write_time float8,
+OUT wal_records int8,
+OUT wal_fpi int8,
+OUT wal_bytes numeric,
+OUT first_seen timestamp with time zone
+)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'pg_stat_statements_1_9'
+LANGUAGE C STRICT VOLATILE PARALLEL SAFE;
+
+CREATE VIEW pg_stat_statements AS
+  SELECT * FROM pg_stat_statements(true);
+
+GRANT SELECT ON pg_stat_statements TO PUBLIC;
+
 --- Define pg_stat_statements_info
 CREATE FUNCTION pg_stat_statements_info(
 OUT dealloc bigint,
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 62cccbfa44..090889e21b 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -99,7 +99,7 @@ PG_MODULE_MAGIC;
 #define PGSS_TEXT_FILE	PG_STAT_TMP_DIR "/pgss_query_texts.stat"
 
 /* Magic number identifying the stats file format */
-static const uint32 PGSS_FILE_HEADER = 0x20201218;
+static const uint32 PGSS_FILE_HEADER = 0x20210322;
 
 /* PostgreSQL major v

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Julien,

On Tue, 2021-03-23 at 15:03 +0800, Julien Rouhaud wrote:
> Note that you could also detect entries for which some counters
> decreased (e.g.
> the execution count), and in that case only use the current values. 

Yes, but checking condition for several counters seems complicated
compared to check only one field.

>  It should
> give the exact same results as what you will get with the first_seen
> column,
> except of course if some entry is almost never used and is suddenly
> used a lot
> after an explicit reset or an eviction and only until you perform
> your
> snapshot.  I'm not sure that it's a very likely scenario though.
But it is possible, and we are guessing here. Storing a timestamp does
not seems too expensive to me, but it totally eliminates guessing, and
provides a clear view about the time interval we watching for this
specific statement.

> FTR that's how powa currently deals with reset/eviction.
PoWA sampling is much more frequent than pg_profile. For PoWA it is, of
cource, very unlikely scenario, but still possible.

-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

Hi Kuroda,

Thank you for your attention!

On Tue, 2021-03-23 at 02:13 +, kuroda.hay...@fujitsu.com wrote:
> But multiple calling of GetCurrentTimestamp() may cause some
> performance issues.
> How about adding a configuration parameter for controlling this
> feature?
> Or we don't not have to worry about that?
Certaily I was thinking about this. And I've taken an advice of Teodor
Sigaev - a much more expirienced developer than me. It seems that
GetCurrentTimestamp() is fast enough for our purpose and we won't call
it too often - only on new statement entry allocation.

By the way right now in my workload tracing tool pg_profile I have to
reset pg_stat_statements on every sample (wich is about 30-60 minutes)
to make sure that all workload between samples is captured. This causes
much more overhead. Introduced first_seen column can eliminate the need
of resets.

However, there is another way - we can store the curent value
of pg_stat_statements_info.dealloc field when allocating a new
statement entry instead of timstamping it. Probably, it would be little
faster, but timestamp seems much more valuable here.
> 
> > +   if (api_version >= PGSS_V1_9)
> > +   {
> > +   values[i++] = TimestampTzGetDatum(first_seen);
> > +   }
> 
> I think {} is not needed here.
Of course, thank you!
-- 
Andrei Zubkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

[PATCH] Tracking statements entry timestamp in pg_stat_statements

[Andrei Zubkov]

This is a proposal for a new feature in pg_stat_statements extension.

As a statistical extension providing counters pg_stat_statements
extension is a target for various sampling solutions. All of them
interested in calculation of statement statistics increments between
two samples. But we face a problem here - observing one statement with
its statistics right now we can't be sure that statistics increment for
this statement is continuous from previous sample. This statement could
be deallocated after previous sample and come back soon. Also it could
happen that statement executions after that increased statistics to
above the values we observed in previous sample making it impossible to
detect deallocation on statement level.
My proposition here is to store statement entry timestamp. In this case
any sampling solution in case of returning statement will easily detect
it by changed timestamp value. And for every statement we will know
exact time interval for its statistics.
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index 16158525ca..ffe08dece5 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -876,4 +876,26 @@ SELECT dealloc FROM pg_stat_statements_info;
0
 (1 row)
 
+--
+-- statement timestamps
+--
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--
+ 
+(1 row)
+
+SELECT now() AS start_ts \gset
+SELECT 1 AS "STMTTS";
+ STMTTS 
+
+  1
+(1 row)
+
+SELECT count(*) FROM pg_stat_statements WHERE first_seen >= :'start_ts' AND query LIKE '%STMTTS%';
+ count 
+---
+ 1
+(1 row)
+
 DROP EXTENSION pg_stat_statements;
diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql b/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
index 3504ca7eb1..9842d2da49 100644
--- a/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
+++ b/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
@@ -3,6 +3,60 @@
 -- complain if script is sourced in psql, rather than via ALTER EXTENSION
 \echo Use "ALTER EXTENSION pg_stat_statements UPDATE TO '1.9'" to load this file. \quit
 
+/* We need to redefine a view and a function */
+/* First we have to remove them from the extension */
+ALTER EXTENSION pg_stat_statements DROP VIEW pg_stat_statements;
+ALTER EXTENSION pg_stat_statements DROP FUNCTION pg_stat_statements(boolean);
+
+/* Then we can drop them */
+DROP VIEW pg_stat_statements;
+DROP FUNCTION pg_stat_statements(boolean);
+
+/* Now redefine */
+CREATE FUNCTION pg_stat_statements(IN showtext boolean,
+OUT userid oid,
+OUT dbid oid,
+OUT queryid bigint,
+OUT query text,
+OUT plans int8,
+OUT total_plan_time float8,
+OUT min_plan_time float8,
+OUT max_plan_time float8,
+OUT mean_plan_time float8,
+OUT stddev_plan_time float8,
+OUT calls int8,
+OUT total_exec_time float8,
+OUT min_exec_time float8,
+OUT max_exec_time float8,
+OUT mean_exec_time float8,
+OUT stddev_exec_time float8,
+OUT rows int8,
+OUT shared_blks_hit int8,
+OUT shared_blks_read int8,
+OUT shared_blks_dirtied int8,
+OUT shared_blks_written int8,
+OUT local_blks_hit int8,
+OUT local_blks_read int8,
+OUT local_blks_dirtied int8,
+OUT local_blks_written int8,
+OUT temp_blks_read int8,
+OUT temp_blks_written int8,
+OUT blk_read_time float8,
+OUT blk_write_time float8,
+OUT wal_records int8,
+OUT wal_fpi int8,
+OUT wal_bytes numeric,
+OUT first_seen timestamp with time zone
+)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'pg_stat_statements_1_9'
+LANGUAGE C STRICT VOLATILE PARALLEL SAFE;
+
+CREATE VIEW pg_stat_statements AS
+  SELECT * FROM pg_stat_statements(true);
+
+GRANT SELECT ON pg_stat_statements TO PUBLIC;
+
 --- Define pg_stat_statements_info
 CREATE FUNCTION pg_stat_statements_info(
 OUT dealloc bigint,
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 62cccbfa44..70b08d36c8 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -99,7 +99,7 @@ PG_MODULE_MAGIC;
 #define PGSS_TEXT_FILE	PG_STAT_TMP_DIR "/pgss_query_texts.stat"
 
 /* Magic number identifying the stats file format */
-static const uint32 PGSS_FILE_HEADER = 0x20201218;
+static const uint32 PGSS_FILE_HEADER = 0x20210322;
 
 /* PostgreSQL major version number, changes in which invalidate all entries */
 static const uint32 PGSS_PG_MAJOR_VERSION = PG_VERSION_NUM / 100;
@@ -125,7 +125,8 @@ typedef enum pgssVersion
 	PGSS_V1_1,
 	PGSS_V1_2,
 	PGSS_V1_3,
-	PGSS_V1_8
+	PGSS_V1_8,
+	PGSS_V1_9
 } pgssVersion;
 
 typedef enum pgssStoreKind
@@ -217,6 +218,7 @@ typedef struct pgssEntry
 	Size		query_offset;	/* query text offset in external file */
 	int			query_len;		/* # 

Re: [PATCH] pg_stat_statements dealloc field ignores manual deallocation

[Andrei Zubkov]

On Fri, 2021-03-19 at 22:15 +0800, Julien Rouhaud wrote:
> I disagree.  The point of that field is to help users configuring
> pg_stat_statements.max, as evictions have a huge overhead in many
> workloads.
> 
> If users remove entries for some reasons, we don't have to give the
> impression
> that pg_stat_statements.max is too low and that it should be
> increased,
> especially since it requires a restart.
> 
Ok.
But when we are collecting aggregated statistics on pg_stat_statememts
periodically it would be great to know about every deallocation
happened. Maybe we need to add another counter for manual deallocations
tracking?