Re: PgStat_HashKey padding issue when passed by reference - Mailing list pgsql-hackers

> > 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