Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

Hi Alena,

On Wed, 2023-10-25 at 16:25 +0300, Alena Rybakina wrote:
>  Hi! Thank you for your work on the subject.
> 1. I didn't understand why we first try to find pgssEntry with a
> false top_level value, and later find it with a true top_level value.

In case of pg_stat_statements the top_level field is the bool field of
the pgssHashKey struct used as the key for pgss_hash hashtable. When we
are performing a reset only userid, dbid and queryid values are
provided. We need to reset both top-level and non-top level entries. We
have only one way to get them all from a hashtable - search for entries
having top_level=true and with top_level=false in their keys.

> 2. And honestly, I think you should change 
>  "Remove the entry if it exists, starting with the non-top-level
> entry." on 
>  "Remove the entry or reset min/max time statistic information and
> the timestamp if it exists, starting with the non-top-level entry."
> And the same with the comment bellow:
> "Also reset the top-level entry if it exists."
>  "Also remove the entry or reset min/max time statistic information
> and the timestamp if it exists."

There are four such places actually - every time when the
SINGLE_ENTRY_RESET macro is used. The logic of reset performed is
described a bit in this macro definition. It seems quite redundant to
repeat this description four times. But I've noted that "remove" terms
should be replaced by "reset".

I've attached v24 with commits updated.

regards, Andrei Zubkov
Postgres Professional