Re: pg_stat_statements oddity with track = all

2021-04-08 Thread Julien Rouhaud
On Thu, Apr 08, 2021 at 03:18:09PM +0200, Magnus Hagander wrote: > On Thu, Apr 8, 2021 at 2:04 PM Julien Rouhaud wrote: > > > > If yes, PFA a patch to merge 1.10 in 1.9. > > I actually thought I looked at that, but clearly I was confused one > way or another. > > I think you're right, it's

Re: pg_stat_statements oddity with track = all

2021-04-08 Thread Magnus Hagander
On Thu, Apr 8, 2021 at 2:04 PM Julien Rouhaud wrote: > > On Thu, Apr 08, 2021 at 10:30:53AM +0200, Magnus Hagander wrote: > > > > I agree. If those numbers are indeed representable, it seems like > > better to pay that overhead than to pay the overhead of trying to > > de-dupe it. > > > > Let's

Re: pg_stat_statements oddity with track = all

2021-04-08 Thread Julien Rouhaud
On Thu, Apr 08, 2021 at 10:30:53AM +0200, Magnus Hagander wrote: > > I agree. If those numbers are indeed representable, it seems like > better to pay that overhead than to pay the overhead of trying to > de-dupe it. > > Let's hope they are :) :) > Looking through ti again my feeling said the

Re: pg_stat_statements oddity with track = all

2021-04-08 Thread Magnus Hagander
On Tue, Mar 16, 2021 at 4:34 PM Julien Rouhaud wrote: > > On Tue, Mar 16, 2021 at 12:55:45PM +0100, Magnus Hagander wrote: > > On Tue, Mar 9, 2021 at 3:39 AM Julien Rouhaud wrote: > > > > > > I think that we might be able to handle that without a flag. The only > > > thing > > > that would

Re: pg_stat_statements oddity with track = all

2021-03-16 Thread Julien Rouhaud
On Tue, Mar 16, 2021 at 12:55:45PM +0100, Magnus Hagander wrote: > On Tue, Mar 9, 2021 at 3:39 AM Julien Rouhaud wrote: > > > > I think that we might be able to handle that without a flag. The only thing > > that would need to be done is when creating an entry, look for an existing > > entry

Re: pg_stat_statements oddity with track = all

2021-03-16 Thread Magnus Hagander
On Tue, Mar 9, 2021 at 3:39 AM Julien Rouhaud wrote: > > On Mon, Mar 08, 2021 at 06:03:59PM +0100, Magnus Hagander wrote: > > On Sun, Mar 7, 2021 at 8:39 AM Julien Rouhaud wrote: > > > > > > Yes I was a bit worried about the possible extra text entry. I kept > > > things > > > simple until now

Re: pg_stat_statements oddity with track = all

2021-03-08 Thread Julien Rouhaud
On Mon, Mar 08, 2021 at 06:03:59PM +0100, Magnus Hagander wrote: > On Sun, Mar 7, 2021 at 8:39 AM Julien Rouhaud wrote: > > > > Yes I was a bit worried about the possible extra text entry. I kept things > > simple until now as the general opinion was that entries existing for both > > top > >

Re: pg_stat_statements oddity with track = all

2021-03-08 Thread Magnus Hagander
On Sun, Mar 7, 2021 at 8:39 AM Julien Rouhaud wrote: > > On Sat, Mar 06, 2021 at 06:56:49PM +0100, Magnus Hagander wrote: > > On Sun, Dec 27, 2020 at 9:39 AM Julien Rouhaud wrote: > > > > > - * > > - * Right now, this structure contains no padding. If you add any, make > > sure > > - * to

Re: pg_stat_statements oddity with track = all

2021-03-06 Thread Julien Rouhaud
On Sat, Mar 06, 2021 at 06:56:49PM +0100, Magnus Hagander wrote: > On Sun, Dec 27, 2020 at 9:39 AM Julien Rouhaud wrote: > > > - * > - * Right now, this structure contains no padding. If you add any, make sure > - * to teach pgss_store() to zero the padding bytes. Otherwise, things will > - *

Re: pg_stat_statements oddity with track = all

2021-03-06 Thread Julien Rouhaud
On Thu, Jan 21, 2021 at 12:43:22AM +0900, Masahiko Sawada wrote: > On Wed, Jan 20, 2021 at 6:15 PM Julien Rouhaud wrote: > > > > I agree, thanks for the change! > > I've changed the topic accordingly. Thanks Sawada-san! I thought that I took care of that but I somehow missed it.

Re: pg_stat_statements oddity with track = all

2021-03-06 Thread Magnus Hagander
On Sun, Dec 27, 2020 at 9:39 AM Julien Rouhaud wrote: > > On Fri, Dec 04, 2020 at 06:09:13PM +0800, Julien Rouhaud wrote: > > On Fri, Dec 04, 2020 at 12:06:10PM +0300, Sergei Kornilov wrote: > > > Hello > > > > > > Seems we need also change PGSS_FILE_HEADER. > > > > Indeed, thanks! v2 attached.

Re: pg_stat_statements oddity with track = all

2021-01-20 Thread Masahiko Sawada
On Wed, Jan 20, 2021 at 6:15 PM Julien Rouhaud wrote: > > Hi, > > On Tue, Jan 19, 2021 at 4:55 PM Masahiro Ikeda > wrote: > > > > Thanks for making the patch to add "toplevel" column in > > pg_stat_statements. > > This is a review comment. > > Thanks a lot for the thorough review! > > > I

Re: pg_stat_statements oddity with track = all

2021-01-20 Thread Masahiro Ikeda
On 2021-01-20 18:14, Julien Rouhaud wrote: On Tue, Jan 19, 2021 at 4:55 PM Masahiro Ikeda wrote: I tested the "update" command can work. postgres=# ALTER EXTENSION pg_stat_statements UPDATE TO '1.10'; Although the "toplevel" column of all queries which already stored is 'false', we have to

Re: pg_stat_statements oddity with track = all

2021-01-20 Thread Julien Rouhaud
Hi, On Tue, Jan 19, 2021 at 4:55 PM Masahiro Ikeda wrote: > > Thanks for making the patch to add "toplevel" column in > pg_stat_statements. > This is a review comment. Thanks a lot for the thorough review! > I tested the "update" command can work. > postgres=# ALTER EXTENSION

Re: pg_stat_statements oddity with track = all

2021-01-19 Thread Masahiro Ikeda
Hi, Thanks for making the patch to add "toplevel" column in pg_stat_statements. This is a review comment. There hasn't been any discussion, at least that I've been able to find. So, +1 to change the status to "Ready for Committer". 1. submission/feature review =

Re: pg_stat_statements oddity with track = all

2020-12-27 Thread Julien Rouhaud
On Fri, Dec 04, 2020 at 06:09:13PM +0800, Julien Rouhaud wrote: > On Fri, Dec 04, 2020 at 12:06:10PM +0300, Sergei Kornilov wrote: > > Hello > > > > Seems we need also change PGSS_FILE_HEADER. > > Indeed, thanks! v2 attached. There was a conflict on PGSS_FILE_HEADER since some recent commit,

Re: pg_stat_statements oddity with track = all

2020-12-04 Thread Julien Rouhaud
On Fri, Dec 04, 2020 at 12:06:10PM +0300, Sergei Kornilov wrote: > Hello > > Seems we need also change PGSS_FILE_HEADER. Indeed, thanks! v2 attached. >From 1da24926d9645ee997aabd2907482a29332e3729 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Fri, 4 Dec 2020 13:33:51 +0800 Subject:

Re: pg_stat_statements oddity with track = all

2020-12-04 Thread Sergei Kornilov
Hello Seems we need also change PGSS_FILE_HEADER. regards, Sergei

Re: pg_stat_statements oddity with track = all

2020-12-04 Thread Julien Rouhaud
On Thu, Dec 03, 2020 at 04:53:59PM +0800, Julien Rouhaud wrote: > On Thu, Dec 03, 2020 at 11:40:22AM +0300, Sergei Kornilov wrote: > > Hello > > > > > To get an increase in the number of records that means that the same > > > statement > > > would appear at top level AND nested level. This seems

Re: pg_stat_statements oddity with track = all

2020-12-03 Thread Julien Rouhaud
On Thu, Dec 03, 2020 at 11:40:22AM +0300, Sergei Kornilov wrote: > Hello > > > To get an increase in the number of records that means that the same > > statement > > would appear at top level AND nested level. This seems a corner case with > > very low > > (neglectible) occurence rate. > > +1 >

Re: pg_stat_statements oddity with track = all

2020-12-03 Thread Julien Rouhaud
On Wed, Dec 02, 2020 at 05:13:56PM +0300, Sergei Kornilov wrote: > Hello > > > - add a parent_statement_id column that would be NULL for top level queries > > Will generate too much entries... Every FK for each different delete/insert, > for example. > But very useful for databases with a lot

Re: pg_stat_statements oddity with track = all

2020-12-03 Thread Sergei Kornilov
Hello > To get an increase in the number of records that means that the same > statement > would appear at top level AND nested level. This seems a corner case with > very low > (neglectible) occurence rate. +1 I think splitting fields into plans_toplevel / plans_nested will be less convenient.

Re: pg_stat_statements oddity with track = all

2020-12-03 Thread legrand legrand
Hi Julien, > The extra field I've proposed would increase the number of records, as it > needs to be a part of the key. To get an increase in the number of records that means that the same statement would appear at top level AND nested level. This seems a corner case with very low

Re: pg_stat_statements oddity with track = all

2020-12-02 Thread Julien Rouhaud
On Wed, Dec 02, 2020 at 06:23:54AM -0800, Nikolay Samokhvalov wrote: > On Tue, Dec 1, 2020 at 10:32 PM Julien Rouhaud wrote: > > > On Tue, Dec 01, 2020 at 10:08:06PM -0800, Nikolay Samokhvalov wrote: > > > If all top-level records in pg_stat_statements have "true" in the new > > > column

Re: pg_stat_statements oddity with track = all

2020-12-02 Thread Nikolay Samokhvalov
On Tue, Dec 1, 2020 at 10:32 PM Julien Rouhaud wrote: > On Tue, Dec 01, 2020 at 10:08:06PM -0800, Nikolay Samokhvalov wrote: > > If all top-level records in pg_stat_statements have "true" in the new > > column (is_toplevel), how would this lead to the need to increase > > pg_stat_statements.max?

Re: pg_stat_statements oddity with track = all

2020-12-02 Thread Sergei Kornilov
Hello > - add a parent_statement_id column that would be NULL for top level queries Will generate too much entries... Every FK for each different delete/insert, for example. But very useful for databases with a lot of stored procedures to find where this query is called. May be new mode track

Re: pg_stat_statements oddity with track = all

2020-12-02 Thread legrand legrand
Hi, a crazy idea: - add a parent_statement_id column that would be NULL for top level queries - build statement_id for nested queries based on the merge of: a/ current_statement_id and parent one or b/ current_statement_id and nested level. this would offer the ability to track counters at

Re: pg_stat_statements oddity with track = all

2020-12-02 Thread Julien Rouhaud
On Wed, Dec 02, 2020 at 03:52:37PM +0900, Fujii Masao wrote: > > On 2020/12/02 15:32, Julien Rouhaud wrote: > > On Tue, Dec 01, 2020 at 10:08:06PM -0800, Nikolay Samokhvalov wrote: > > > On Tue, Dec 1, 2020 at 8:05 PM Julien Rouhaud wrote: > > > > > > > Someone raised an interested point

Re: pg_stat_statements oddity with track = all

2020-12-01 Thread Fujii Masao
On 2020/12/02 15:32, Julien Rouhaud wrote: On Tue, Dec 01, 2020 at 10:08:06PM -0800, Nikolay Samokhvalov wrote: On Tue, Dec 1, 2020 at 8:05 PM Julien Rouhaud wrote: Someone raised an interested point recently on pg_stat_kcache extension for handling nested statements, which also applies

Re: pg_stat_statements oddity with track = all

2020-12-01 Thread Julien Rouhaud
On Tue, Dec 01, 2020 at 10:08:06PM -0800, Nikolay Samokhvalov wrote: > On Tue, Dec 1, 2020 at 8:05 PM Julien Rouhaud wrote: > > > Someone raised an interested point recently on pg_stat_kcache extension for > > handling nested statements, which also applies to pg_stat_statements. > > > ... > > >

Re: pg_stat_statements oddity with track = all

2020-12-01 Thread Nikolay Samokhvalov
On Tue, Dec 1, 2020 at 8:05 PM Julien Rouhaud wrote: > Hi, > > Someone raised an interested point recently on pg_stat_kcache extension for > handling nested statements, which also applies to pg_stat_statements. > ... > The only idea I have for that is to add a new field to entry key, for >

pg_stat_statements oddity with track = all

2020-12-01 Thread Julien Rouhaud
Hi, Someone raised an interested point recently on pg_stat_kcache extension for handling nested statements, which also applies to pg_stat_statements. The root issue is that when pg_stat_statements tracks nested statements, there's no way to really make sense of the counters, as top level