Re: fix_PGSTAT_NUM_TABENTRIES_macro patch
От | Mark Dilger |
---|---|
Тема | Re: fix_PGSTAT_NUM_TABENTRIES_macro patch |
Дата | |
Msg-id | 1388704558.83467.YahooMailNeo@web125406.mail.ne1.yahoo.com обсуждение исходный текст |
Ответ на | Re: fix_PGSTAT_NUM_TABENTRIES_macro patch (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: fix_PGSTAT_NUM_TABENTRIES_macro patch
(Andres Freund <andres@2ndquadrant.com>)
Re: fix_PGSTAT_NUM_TABENTRIES_macro patch (Tom Lane <tgl@sss.pgh.pa.us>) |
Список | pgsql-hackers |
<div style="color:#000; background-color:#fff; font-family:HelveticaNeue, Helvetica Neue, Helvetica, Arial, Lucida Grande,sans-serif;font-size:12pt">I still don't understand why this case in src/include/pgstat.h<br />is different from caseselsewhere in the code. Taken from<br />src/include/access/heapam_xlog.h:<br /><br /><br />typedef struct xl_heap_header<br/>{<br /> uint16 t_infomask2;<br /> uint16 t_infomask;<br /> uint8 t_hoff;<br />}xl_heap_header;<br /><br />#define SizeOfHeapHeader (offsetof(xl_heap_header, t_hoff) + sizeof(uint8))<br /><br /><br/><br />Now, if somebody changed t_hoff to be a uint16, that SizeOfHeapHeader<br />macro would be wrong. Should weput a static assert in the code for that?<br />I have no objection, but it seems you like the static assert in one place<br/>and not the other, and (perhaps due to some incredible ignorance on my<br />part) I cannot see why. I tried lookingfor an assert of this kind already in<br />the code. The use of this macro is in src/backend/access/heap/heapam.c,<br/>but I didn't see any asserts for it, though there are lots of asserts for other<br/>stuff. Maybe I just didn't recognize it?<br /><br /><br />mark<br /><div class="yahoo_quoted" style="display:block;"><br /><br /><div style="font-family: HelveticaNeue, Helvetica Neue, Helvetica, Arial, Lucida Grande,sans-serif; font-size: 12pt;"><div style="font-family: HelveticaNeue, Helvetica Neue, Helvetica, Arial, Lucida Grande,sans-serif; font-size: 12pt;"><div dir="ltr"><font face="Arial" size="2"> On Thursday, January 2, 2014 2:05 PM, TomLane <tgl@sss.pgh.pa.us> wrote:<br /></font></div><div class="y_msg_container">I wrote:<br clear="none" />> Itoccurs to me that, rather than trying to improve the struct definition<br clear="none" />> methodology, maybe we shouldjust add static asserts to catch any<br clear="none" />> inconsistency here. It wouldn't be that hard:<br clear="none"/><br clear="none" />> #define PGSTAT_MAX_MSG_SIZE 1000<br clear="none" />> #define PGSTAT_MSG_PAYLOAD (PGSTAT_MAX_MSG_SIZE - sizeof(PgStat_MsgHdr))<br clear="none" />> ... all else in pgstat.h as before...<br clear="none" /><br clear="none" />> StaticAssertStmt(sizeof(PgStat_MsgTabstat) <= PGSTAT_MAX_MSG_SIZE,<brclear="none" />> 'bad PgStat_MsgTabstat size');<br clear="none" />> ... and similarlyfor other pgstat message structs ...<br clear="none" /><br clear="none" />After further thought it seems to me thatthis is a desirable approach,<br clear="none" />because it is a direct check of the property we want, and will complain<brclear="none" />about *any* mistake that results in too-large struct sizes. The other<br clear="none" />ideasthat were kicked around upthread still left a lot of ways to mess up<br clear="none" />the array size calculations,for instance referencing the wrong array<br clear="none" />element type in the sizeof calculation. So unlessanyone has a concrete<br clear="none" />objection, I'll go put in something like this along with Mark's fix.<div class="yqt0876931955"id="yqtfd25444"><br clear="none" /><br clear="none" /> regards, tom lane<br clear="none"/><br clear="none" /><br clear="none" />-- <br clear="none" />Sent via pgsql-hackers mailing list (<a href="mailto:pgsql-hackers@postgresql.org"shape="rect" ymailto="mailto:pgsql-hackers@postgresql.org">pgsql-hackers@postgresql.org</a>)<brclear="none" />To make changes to yoursubscription:<br clear="none" /><a href="http://www.postgresql.org/mailpref/pgsql-hackers" shape="rect" target="_blank">http://www.postgresql.org/mailpref/pgsql-hackers</a><brclear="none" /></div><br /><br /></div></div></div></div></div>
В списке pgsql-hackers по дате отправления:
Предыдущее
От: "Erik Rijkers"Дата:
Сообщение: Re: [PATCH] Negative Transition Aggregate Functions (WIP)
Следующее
От: "Erik Rijkers"Дата:
Сообщение: Re: [PATCH] Negative Transition Aggregate Functions (WIP)