Re: fix_PGSTAT_NUM_TABENTRIES_macro patch

I still don't understand why this case in src/include/pgstat.h
is different from cases elsewhere in the code.  Taken from
src/include/access/heapam_xlog.h:

typedef struct xl_heap_header
{
    uint16      t_infomask2;
    uint16      t_infomask;
    uint8       t_hoff;
} xl_heap_header;

#define SizeOfHeapHeader    (offsetof(xl_heap_header, t_hoff) + sizeof(uint8))

Now, if somebody changed t_hoff to be a uint16, that SizeOfHeapHeader
macro would be wrong.  Should we put a static assert in the code for that?
I have no objection, but it seems you like the static assert in one place
and not the other, and (perhaps due to some incredible ignorance on my
part) I cannot see why.  I tried looking for an assert of this kind already in
the code.  The use of this macro is in src/backend/access/heap/heapam.c,
but I didn't see any asserts for it, though there are lots of asserts for other
stuff.  Maybe I just didn't recognize it?

mark

On Thursday, January 2, 2014 2:05 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

I wrote:
> It occurs to me that, rather than trying to improve the struct definition
> methodology, maybe we should just add static asserts to catch any
> inconsistency here.  It wouldn't be that hard:

> #define PGSTAT_MAX_MSG_SIZE    1000
> #define PGSTAT_MSG_PAYLOAD    (PGSTAT_MAX_MSG_SIZE - sizeof(PgStat_MsgHdr))
> ... all else in pgstat.h as before ...

> StaticAssertStmt(sizeof(PgStat_MsgTabstat) <= PGSTAT_MAX_MSG_SIZE,
>         'bad PgStat_MsgTabstat size');
> ... and similarly for other pgstat message structs ...

After further thought it seems to me that this is a desirable approach,
because it is a direct check of the property we want, and will complain
about *any* mistake that results in too-large struct sizes.  The other
ideas that were kicked around upthread still left a lot of ways to mess up
the array size calculations, for instance referencing the wrong array
element type in the sizeof calculation.  So unless anyone has a concrete
objection, I'll go put in something like this along with Mark's fix.

            regards, tom lane