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 > <varname>pg_stat_statements.track_planning</varname> > is disabled, or if the counter has been reset using the the > <function>pg_stat_statements_reset</function> function with the > <structfield>minmax_only</structfield> parameter set to > <literal>true</literal> > and never been planned since. Thanks a lot! > > <primary>pg_stat_statements_reset</primary> > </indexterm> > @@ -589,6 +623,20 @@ > If all statistics in the > <filename>pg_stat_statements</filename> > view are discarded, it will also reset the statistics in the > <structname>pg_stat_statements_info</structname> view. > + When <structfield>minmax_only</structfield> is > <literal>true</literal> 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