Re: page macros cleanup

Heikki Linnakangas napsal(a):
> Pavan Deolasee wrote:
>> On Fri, Jul 4, 2008 at 1:01 PM, Zdenek Kotala <Zdenek(dot)Kotala(at)sun(dot)com>
>> wrote:
>>>
>>> Good catch. I lost in basic arithmetic. What I see now that original
>>> definition count sizeof(ItemIdData) twice and on other side it does
>>> not take
>>> care about MAXALING correctly. I think correct formula is:
>>>
>>> #define HashMaxItemSize(page) \
>>> (PageGetPageSize(page) - \
>>> ( MAXALIGN(SizeOfPageHeaderData + sizeof(ItemIdData))+ \
>>> MAXALIGN(sizeof(HashPageOpaqueData)) \
>>> )\
>>> )
>>>
>>> What do you think?
>>
>> Yes. I think that's the correct way.
>
> Doesn't look right to me. There's no padding after the first line
> pointer, hence the first MAXALIGN shouldn't be there.

Are you sure? I expecting that tupleheader must be aligned to MAXALIGN. If it is
not required than you are right.

Look on PageAddItem:

00226 alignedSize = MAXALIGN(size);
00227
00228 upper = (int) phdr->pd_upper - (int) alignedSize;

By my opinion first place where tuple should be placed is:

MAXALIGN(SizeOfPageHeaderData + sizeof(ItemIdData))

> BTW, looking at hashinsert.c where it's used, we're actually passing a
> pointer to the meta page to HashMaxItemSize(). So the PageGetPageSize()
> call on that is quite bogus, since it's not the meta page that the tuple
> is going to be inserted to. It's academical, because all pages are the
> same size anyway, but doesn't look right. I think I'd go with BLKCSZ
> instead.
>

Yeah, BLKCSZ looks good. Anyway, I plan to reorganize all *MaxItemSize staff to
be compatible with in-place upgrade.

thanks Zdenek

--
Zdenek Kotala Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql