Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-11-28 Thread Alexander Lakhin
Hello Alexander, 25.11.2023 23:46, Alexander Korotkov wrote: On Sat, Nov 25, 2023 at 10:45 PM Andrei Zubkov wrote: 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. Thank you for catching it. I'll fix this before

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-11-25 Thread Alexander Korotkov
On Sat, Nov 25, 2023 at 10:45 PM Andrei Zubkov wrote: > 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.

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-11-25 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-11-24 Thread Julien Rouhaud
Hi, On Sat, Nov 25, 2023 at 02:45:07AM +0200, Alexander Korotkov wrote: > > I've reviewed this patch. I think this is the feature of high demand. > New columns (stats_since and minmax_stats_since) to the > pg_stat_statements view, enhancing the granularity and precision of > performance

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-11-24 Thread Alexander Korotkov
Hi! On Fri, Nov 17, 2023 at 10:40 AM Andrei Zubkov wrote: > > A little fix in "level_tracking" tests after merge. I've reviewed this patch. I think this is the feature of high demand. New columns (stats_since and minmax_stats_since) to the pg_stat_statements view, enhancing the granularity and

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-11-17 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-11-16 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-26 Thread Alena Rybakina
On 25.10.2023 18:35, Andrei Zubkov wrote: 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-26 Thread 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?

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-26 Thread Andrei Lepikhov
On 25/10/2023 20:00, Andrei Zubkov wrote: 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-25 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-25 Thread Alena Rybakina
On 19.10.2023 15:40, Andrei Zubkov wrote: 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? Hi! Thank you for your work on the

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-25 Thread 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?

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-25 Thread Andrei Lepikhov
On 19/10/2023 19:40, Andrei Zubkov wrote: Hi hackers, New version 23 attached. It contains rebase to the current master. I discovered the patch and parameters you've proposed. In my opinion, the stats_since parameter adds valuable information and should definitely be included in the stats

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-24 Thread Peter Eisentraut
On 24.10.23 14:40, Julien Rouhaud wrote: On Tue, Oct 24, 2023 at 6:57 PM Peter Eisentraut wrote: On 24.10.23 09:58, Andrei Zubkov wrote: During last moving to the current commitfest this patch have lost its reviewers list. With respect to reviewers contribution in this patch, I think

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-24 Thread Julien Rouhaud
On Tue, Oct 24, 2023 at 6:57 PM Peter Eisentraut wrote: > > On 24.10.23 09:58, Andrei Zubkov wrote: > > 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. > > I

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-24 Thread Peter Eisentraut
On 24.10.23 09:58, Andrei Zubkov wrote: 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. I don't think it's the purpose of the commitfest app to track how *has*

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-10-24 Thread 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

2023-10-19 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-08-01 Thread Daniel Gustafsson
> On 22 Mar 2023, at 09:17, Andrei Zubkov wrote: > New version is attached. This patch is marked RfC but didn't get reviewed/committed during this CF so I'm moving it to the next, the patch no longer applies though so please submit an updated version. -- Daniel Gustafsson

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-22 Thread 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.

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-21 Thread Sergei Kornilov
Hello! The documentation still describes the function pg_stat_statements_reset like this > By default, this function can only be executed by superusers. But unfortunately, this part was lost somewhere. -- Don't want this to be available to non-superusers. REVOKE ALL ON FUNCTION

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-16 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-16 Thread 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 >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-16 Thread Michael Paquier
On Sat, Mar 11, 2023 at 02:49:50PM +0300, Andrei Zubkov wrote: > Hi, > > I've done a rebase of a patch to the current master. +/* 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-11 Thread 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:

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-06 Thread 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 > -- >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-06 Thread Gregory Stark (as CFM)
I'm sorry, It seems this is failing again? It's Makefile and meson.build again :( Though there are also 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 |+++

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-01 Thread 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 >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-01 Thread Gregory Stark (as CFM)
On Wed, 1 Mar 2023 at 04:04, Andrei Zubkov wrote: > > Hi! > > I've attached a new version of a patch - rebase to the current master. 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-03-01 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-01-26 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-01-25 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-01-18 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-01-18 Thread Tomas Vondra
Hi, 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. As for the patch, I wonder if it's

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-08 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-04 Thread Julien Rouhaud
Hi, On Mon, Apr 04, 2022 at 09:59:04AM +0300, Andrei Zubkov wrote: > > 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: > > > >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-04 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-03 Thread Julien Rouhaud
Hi, On Sun, Apr 03, 2022 at 01:24:40PM +0300, Andrei Zubkov wrote: > > 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 ... > >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-03 Thread 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 >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-03 Thread Julien Rouhaud
Hi, On Sun, Apr 03, 2022 at 12:29:43PM +0300, Andrei Zubkov wrote: > 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 > >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-03 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-03 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-03 Thread Julien Rouhaud
On Sun, Apr 03, 2022 at 07:32:47AM +0300, Andrei Zubkov wrote: > v11 attached + /* When requested reset only min/max statistics of an entry */ \ + entry_counters = >counters; \ + for (int kind = 0; kind < PGSS_NUMKIND; kind++) \ + { \ +

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-02 Thread 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" |

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-02 Thread Tom Lane
Greg Stark writes: > The tests for this seem to need adjustments. > [12:41:09.403] test pg_stat_statements ... FAILED 180 ms > diff -U3 > /tmp/cirrus-ci-build/contrib/pg_stat_statements/expected/pg_stat_statements.out >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-02 Thread Greg Stark
The tests for this seem to need adjustments. [12:41:09.403] test pg_stat_statements ... FAILED 180 ms diff -U3 /tmp/cirrus-ci-build/contrib/pg_stat_statements/expected/pg_stat_statements.out /tmp/cirrus-ci-build/contrib/pg_stat_statements/results/pg_stat_statements.out ---

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-02 Thread 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 > >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-02 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-02 Thread Julien Rouhaud
On Sat, Apr 02, 2022 at 01:12:54PM +0300, Andrei Zubkov wrote: > 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 > >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-02 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-02 Thread Julien Rouhaud
On Fri, Apr 01, 2022 at 01:01:53PM -0700, Andres Freund wrote: > Hi, > > On 2022-04-01 22:47:02 +0300, Andrei Zubkov wrote: > > + entry = (pgssEntry *) hash_search(pgss_hash, , HASH_FIND, > > NULL); > > + > > + if (entry) { > > + /* Found */ > > +

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-02 Thread Julien Rouhaud
On Fri, Apr 01, 2022 at 10:47:02PM +0300, Andrei Zubkov wrote: > > 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-02 Thread Julien Rouhaud
On Thu, Mar 31, 2022 at 01:06:10PM +0300, Andrei Zubkov wrote: > > 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 > >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-01 Thread Andres Freund
Hi, On 2022-04-01 22:47:02 +0300, Andrei Zubkov wrote: > + entry = (pgssEntry *) hash_search(pgss_hash, , HASH_FIND, > NULL); > + > + if (entry) { > + /* Found */ > + if (minmax_only) { > + /* When

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-01 Thread 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] >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-01 Thread Julien Rouhaud
Hi, On Fri, Apr 01, 2022 at 11:38:52AM -0400, Greg Stark wrote: > FYI this has a compiler warning showing up on the cfbot: > > [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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-04-01 Thread Greg Stark
FYI this has a compiler warning showing up on the cfbot: [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 |

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-03-31 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-03-30 Thread Julien Rouhaud
Hi, On Fri, Mar 25, 2022 at 01:25:23PM +0300, Andrei Zubkov wrote: > 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-03-25 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-03-24 Thread Greg Stark
This patch seems to have gotten some feedback but development has stalled. It's marked "waiting on author" but I'm not clear exactly what is expected from the authors here. It seems there isn't really consensus on the design at the moment. There's been no emails in over a month. Fwiw I find the

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-02-07 Thread Anton A. Melnikov
Hello! On 26.01.2022 16:43, Andrei Zubkov wrote: >> >> 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-01-26 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-01-25 Thread Julien Rouhaud
Hi Andrei, On Tue, Jan 25, 2022 at 02:58:17PM +0300, Andrei Zubkov wrote: > > 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 >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-01-25 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-01-25 Thread Julien Rouhaud
Hi, On Mon, Jan 24, 2022 at 08:16:06PM +0300, Anton A. Melnikov wrote: > Hi, Andrey! > > I've checked the 5th version of the patch and there are some remarks. > > >I've created a new view named pg_stat_statements_aux. But for now both > >views are using the same function pg_stat_statements

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-01-24 Thread Anton A. Melnikov
Hi, Andrey! I've checked the 5th version of the patch and there are some remarks. >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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-01-14 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2022-01-02 Thread Andres Freund
Hi, On 2021-12-03 17:03:46 +0300, Andrei Zubkov wrote: > I've attached a new version of a patch. This fails with an assertion failure: https://cirrus-ci.com/task/5567540742062080?logs=cores#L55 Greetings, Andres Freund

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-12-21 Thread Anton A. Melnikov
On 03.12.2021 19:55, Andrei Zubkov wrote: On Fri, 2021-12-03 at 17:03 +0300, Andrei Zubkov wrote: ... 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. Hi, Andrey! I completely agree that

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-12-03 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-12-03 Thread 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,

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-10-18 Thread Anton A. Melnikov
On 07.10.2021 15:31, Andrei Zubkov wrote: > There is an issue with this patch. It's main purpose is the ability to > calculate values of pg_stat_statements view >  [...] > Does addition of resettable min/max metrics to the > pg_stat_statemets view seems reasonable here? Hello, Andrey! I think

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-10-07 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-10-07 Thread 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

Re[2]: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-10-06 Thread Мельников Антон Андреевич
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  

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-04-19 Thread 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.

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-04-19 Thread Chengxi Sun
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation:not tested Hi, Andrei I tested your patch, and it works well. I also prefer

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-04-14 Thread 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]:

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-04-14 Thread 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

2021-04-14 Thread Julien Rouhaud
Le mer. 14 avr. 2021 à 17:22, Andrei Zubkov a écrit : > > 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-04-14 Thread 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

RE: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-04-08 Thread kuroda.hay...@fujitsu.com
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]:

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-04-07 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-04-07 Thread Seino Yuki
2021-03-23 23:08 に 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. Thanks for posting the patch. I agree with this content. Is

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-03-23 Thread 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

RE: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-03-23 Thread kuroda.hay...@fujitsu.com
Dear Andrei, > 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. OK. >

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-03-23 Thread 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-03-23 Thread Julien Rouhaud
On Tue, Mar 23, 2021 at 09:50:16AM +0300, Andrei Zubkov wrote: > > 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

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-03-23 Thread 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

RE: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-03-22 Thread kuroda.hay...@fujitsu.com
Dear Andrei, I think the idea is good because the pg_stat_statements_info view cannot distinguish whether the specific statement is deallocated or not. But multiple calling of GetCurrentTimestamp() may cause some performance issues. How about adding a configuration parameter for controlling this

[PATCH] Tracking statements entry timestamp in pg_stat_statements

2021-03-22 Thread 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