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

Reply via email to