From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Fix comments related to pending statistics |
Date: | 2024-12-11 05:56:08 |
Message-ID: | Z1kpeNIFHgrE2UAD@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Dec 10, 2024 at 02:16:14PM +0000, Bertrand Drouvot wrote:
> 1. One linked to PgStat_TableCounts pending stats, mentioning the use of
> memcmp() against zeroes to detect whether there are any stats updates to apply.
>
> This is not true anymore as of 07e9e28b56.
Oops, you're right. The comment has missed the memo about this
mention to memcpy().
>
> 2. One linked to PgStat_FunctionCounts pending stats, mentioning the use of
> memcmp() against zeroes to detect whether there are any pending stats: I think
> it has never been the case.
* PgStat_FunctionCounts The actual per-function counts kept by a backend
*
- * This struct should contain only actual event counters, because we memcmp
- * it against zeroes to detect whether there are any pending stats.
+ * This struct should contain only actual event counters, because pending stats
+ * always has non-zero content.
pgstat_function_flush_cb() in pgstat_function.c says since
5891c7a8ed8f, so that seems like a remnant from the original patch
that has introduced this inconsistency across its reviews:
"/* localent always has non-zero content */"
Your suggestion does not look completely right to me. There is
nothing preventing us from using something else than event counters
since we don't use memcpy() and there is no comparison work, no? It
seems to me that we could remove the entire sentence instead.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2024-12-11 06:03:34 | Re: Make use of pg_memory_is_all_zeros() in more places |
Previous Message | Shubham Khanna | 2024-12-11 05:53:19 | Re: Adding a '--two-phase' option to 'pg_createsubscriber' utility. |