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 |
Thread: | |
Lists: | pgsql-hackers |
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 ...
> >
> > 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.
Ok!
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:
+#define SINGLE_ENTRY_RESET() \
+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!
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 |