On Mon, Sep 08, 2025 at 09:36:52PM -0500, Sami Imseih wrote: > But my concern is the flexibility of this approach. If someone is to add an > OID field next, they will not be able to as that will be introducing > padding. On the other hand, passing the key by reference and > documenting the reason in pgstat_shmem.c will not lose this > flexibility.
I don't mind discarding the static assertion idea, but at the end what counts for me here is that I don't want to sacrifice future changes in the pgstats code that would always require passing around the hash key by reference. So I would just do like attached, documenting at the top of PgStat_HashKey that we should not have padding in it, removing three memset(0) calls that expected it. -- Michael
From 104ab93d761746366d6e71e84472f3a3264e7a5b Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 11 Sep 2025 11:50:18 +0900
Subject: [PATCH] Document no-padding rule for PgStat_HashKey
This removes a couple of memset(0) calls, as we don't want padding in
it.
---
src/include/utils/pgstat_internal.h | 6 +++++-
src/backend/utils/activity/pgstat.c | 5 +----
src/backend/utils/activity/pgstat_shmem.c | 10 ++--------
3 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 6cf00008f633..b62747a3ba05 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -48,7 +48,11 @@
* PgStatShared_Common as the first element.
*/
-/* struct for shared statistics hash entry key. */
+/*
+ * Struct for shared statistics hash entry key.
+ *
+ * NB: We assume that this struct contains no padding.
+ */
typedef struct PgStat_HashKey
{
PgStat_Kind kind; /* statistics entry kind */
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index f8e91484e36b..73c2ced3f4e2 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -932,7 +932,7 @@ pgstat_clear_snapshot(void)
void *
pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, uint64 objid)
{
- PgStat_HashKey key;
+ PgStat_HashKey key = {0};
PgStat_EntryRef *entry_ref;
void *stats_data;
const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
@@ -943,9 +943,6 @@ pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, uint64 objid)
pgstat_prep_snapshot();
- /* clear padding */
- memset(&key, 0, sizeof(struct PgStat_HashKey));
-
key.kind = kind;
key.dboid = dboid;
key.objid = objid;
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 9dc3212f7dd0..ca36fd247f64 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -456,14 +456,11 @@ PgStat_EntryRef *
pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create,
bool *created_entry)
{
- PgStat_HashKey key;
+ PgStat_HashKey key = {0};
PgStatShared_HashEntry *shhashent;
PgStatShared_Common *shheader = NULL;
PgStat_EntryRef *entry_ref;
- /* clear padding */
- memset(&key, 0, sizeof(struct PgStat_HashKey));
-
key.kind = kind;
key.dboid = dboid;
key.objid = objid;
@@ -988,13 +985,10 @@ pgstat_drop_database_and_contents(Oid dboid)
bool
pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid)
{
- PgStat_HashKey key;
+ PgStat_HashKey key = {0};
PgStatShared_HashEntry *shent;
bool freed = true;
- /* clear padding */
- memset(&key, 0, sizeof(struct PgStat_HashKey));
-
key.kind = kind;
key.dboid = dboid;
key.objid = objid;
--
2.51.0
signature.asc
Description: PGP signature
