Tom Lane wrote:
I was just looking at this macro:
/*
* PageGetMaxOffsetNumber
* Returns the maximum offset number used by the given page.
* Since offset numbers are 1-based, this is also the number
* of items on the page.
*
* NOTE: to ensure sane behavior if the page is not initialized
* (pd_lower == 0), cast the unsigned values to int before dividing.
* That way we get -1 or so, not a huge positive number...
*/
#define PageGetMaxOffsetNumber(page) \
(((int) (((PageHeader) (page))->pd_lower - SizeOfPageHeaderData)) \
/ ((int) sizeof(ItemIdData)))
The macro does the right thing on its own terms when applied to a zeroed
page, but in some places it's used like this:
OffsetNumber n;
OffsetNumber maxoff;
maxoff = PageGetMaxOffsetNumber(p);
for (n = FirstOffsetNumber; n <= maxoff; n++)
and OffsetNumber is uint16 not int. So a loop like this would go nuts
instead of treating the zeroed page as if it were empty. This is not
good (see the comments for PageHeaderIsValid in bufpage.c if you
disremember why).
We could fix this by changing the declarations of the "maxoff" variables
to int, but I think it's probably cleaner to recode
PageGetMaxOffsetNumber like so:
#define PageGetMaxOffsetNumber(page) \
(((PageHeader) (page))->pd_lower <= SizeOfPageHeaderData ? 0 : \
((((PageHeader) (page))->pd_lower - SizeOfPageHeaderData) \
/ sizeof(ItemIdData)))
Well I think that is safe change:
a <= b ? 0 : ( a-b ) /c
in
max( 0, (a-b)/c )
so I think (not tested) you can rewrite that macro in:
#define PageGetMaxOffsetNumber(page) \
(max(0, ((((PageHeader) (page))->pd_lower - SizeOfPageHeaderData) \
/ sizeof(ItemIdData))))
Regards
Gaeatano Mendola
---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend
