Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

From: Julien Rouhaud <rjuju123(at)gmail(dot)com>
To: Andrei Zubkov <zubkov(at)moonset(dot)ru>
Cc: Greg Stark <stark(at)mit(dot)edu>, Andres Freund <andres(at)anarazel(dot)de>, "Anton A(dot) Melnikov" <aamelnikov(at)inbox(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements
Date: 2022-04-04 02:31:45
Message-ID: 20220404023145.hftzskw425h76y6g@jrouhaud
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


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 ...
> >
> > 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.


So last round of review.

- 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/

- code:

+if (entry) { \

It's not great to rely on caller context too much. 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.

Apart from that I think this is ready!

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2022-04-04 02:46:05 Re: JSON constructors and window functions
Previous Message Noah Misch 2022-04-04 02:31:28 Re: Skipping logical replication transactions on subscriber side