Re: fix_PGSTAT_NUM_TABENTRIES_macro patch

I completely understand the padding issues that
you are dealing with.  I was mostly curious why
Tom wanted to use asserts to double-check the
code in one place, while happily not doing so in
what seemed the same kind of situation elsewhere.
He has since made the reason for that clear.

On Thursday, January 2, 2014 3:27 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:

On 2014-01-02 15:15:58 -0800, Mark Dilger wrote:

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

The reason the various SizeOfHeapHeader are written that way is that we
do not want to uselessly store trailing padding in the
WAL. E.g. sizeof(xl_heap_header) will be 6bytes, but SizeOfHeapHeader
will be 5.
I don't see an easy way to guarantee this with asserts and I think you'd
notice pretty fast if you got things wrong there because WAL replay will
just have incomplete data.

Greetings,

Andres Freund

--
Andres Freund                      http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services