> > I'd just add a comment mentioning that any padding bytes should be avoided.
>
> Agreed on this part.
>
> I am wondering also about the addition of a static assertion as well,
> as it seems that this thread and the opinions on it point to such an
> addition having value, and requiring valgrind to detect that is
> annoying, especially if people propose more patches in the future that
> may affect the way we pass the hash key values in this area of the
> code. An idea is attached.
hmm, so if we decide to add a member that has a type that requires
padding, the assert will fail? I am not sure I like this very much, since
we lose flexibility on the struct member types that can be used. See
the examples below on top of pgstat-hashkey-padding.patch
The following will fail the assert since padding is needed for the
new Oid member.
```
index d5869299fa8..4cdd41fda3a 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -53,6 +53,7 @@ typedef struct PgStat_HashKey
{
PgStat_Kind kind; /* statistics entry kind */
Oid dboid; /* database
ID. InvalidOid for shared objects. */
+ Oid field1;
uint64 objid; /* object ID (table,
function, etc.), or
*
identifier. */
} PgStat_HashKey;
@@ -62,7 +63,7 @@ typedef struct PgStat_HashKey
* size matches with the sum of each field is a check simple enough to
* enforce this policy.
*/
-StaticAssertDecl((sizeof(PgStat_Kind) + sizeof(uint64) + sizeof(Oid)) ==
+StaticAssertDecl((sizeof(PgStat_Kind) + sizeof(uint64) + sizeof(Oid)
+ sizeof(Oid)) ==
sizeof(PgStat_HashKey),
"PgStat_HashKey should have no padding");
```
This will not fail the assert, since no padding is needed for the new member.
```
diff --git a/src/include/utils/pgstat_internal.h
b/src/include/utils/pgstat_internal.h
index d5869299fa8..5732c89219d 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -53,6 +53,7 @@ typedef struct PgStat_HashKey
{
PgStat_Kind kind; /* statistics entry kind */
Oid dboid; /* database
ID. InvalidOid for shared objects. */
+ uint64 field1;
uint64 objid; /* object ID (table,
function, etc.), or
*
identifier. */
} PgStat_HashKey;
@@ -62,7 +63,7 @@ typedef struct PgStat_HashKey
* size matches with the sum of each field is a check simple enough to
* enforce this policy.
*/
-StaticAssertDecl((sizeof(PgStat_Kind) + sizeof(uint64) + sizeof(Oid)) ==
+StaticAssertDecl((sizeof(PgStat_Kind) + sizeof(uint64) + sizeof(Oid)
+ sizeof(uint64)) ==
sizeof(PgStat_HashKey),
"PgStat_HashKey should have no padding");
```
--
Sami
