Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

On 25.10.2023 18:35, Andrei Zubkov wrote:
> 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.
Thank you for explanation, I got it.
>> 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.
Yes, I agree with the changes.

--
Regards,
Alena Rybakina