Re: fix_PGSTAT_NUM_TABENTRIES_macro patch

From: Mark Dilger <markdilger(at)yahoo(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: fix_PGSTAT_NUM_TABENTRIES_macro patch
Date: 2014-01-02 23:54:26
Message-ID: 1388706866.22946.YahooMailNeo@web125402.mail.ne1.yahoo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The mechanism that occurs to me (and I'm not wedded to
this idea) is:

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

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

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

Mark Dilger <markdilger(at)yahoo(dot)com> writes:
> I still don't understand why this case in src/include/pgstat.h
> is different from cases elsewhere in the code.

The reason why I'm exercised about it is that (a) somebody
actually made a
mistake of this type, and (b) it wasn't caught by any automated testing.

The catalog and WAL-related examples you cite would probably crash
and burn in fairly obvious ways if somebody broke them --- for instance,
the most likely way to break SizeOfHeapHeader would be by adding another
field after t_hoff, but we'd notice that before long because said field
would be corrupted on arrival at a replication slave.

In contrast, messing up the pgstats message sizes would have no
consequences worse than a hard-to-detect, and probably platform-specific,
performance penalty for stats transmission.  So unless we think that's
of absolutely zero concern, adding a mechanism to make such bugs more
apparent seems useful.

I'm not against adding more assertions elsewhere, but it's a bit hard to
see what those asserts should test.  I don't see any practical way to
assert that field X is the last one in its struct, for instance.

            regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2014-01-02 23:56:22 Re: INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
Previous Message MauMau 2014-01-02 23:54:18 Re: Streaming replication bug in 9.3.2, "WAL contains references to invalid pages"