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 which returns all > >fields. It seems reasonable to me - if sampling solution will need all > >values it can query the function. > > Agreed, it might be useful in some cases. > > >But it seems "stats_reset" term is not quite correct in this case. The > >"stats_since" field holds the timestamp of hashtable entry, but not the > >reset time. The same applies to aux_stats_since - for new statement it > >holds its entry time, but in case of reset it will hold the reset time. > > Thanks for the clarification. Indeed if we mean the word 'reset' as the > removal of all the hashtable entries during pg_stat_statements_reset() then > we shouldn't use it for the first occurrence timestamp in the struct > pgssEntry. > So with the stats_since field everything is clear. > On the other hand aux_stats_since field can be updated for two reasons: > 1) The same as for stats_since due to first occurrence of entry in the > hashtable. And it will be equal to stats_since timestamp in that case. > 2) Due to an external reset from an upper level sampler. > I think it's not very important how to name this field, but it would be > better to mention both these reasons in the comment.
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 regularly, do you really need to know e.g. "what was the maximum execution time in the last 3 years", which will typically be what people will retrieve from the non-aux min/max counters, rather than simply using your additional tool for better and more precise information?
