pg.ddx.io  pgsql-patches@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Zdenek Kotala <Zdenek.Kotala@Sun.COM>
To: Heikki Linnakangas <heikki@enterprisedb.com>
Cc: Pavan Deolasee <pavan.deolasee@gmail.com>
Cc: PostgreSQL-patches <pgsql-patches@postgresql.org>
Subject: Re: page macros cleanup
Date: Fri, 04 Jul 2008 12:50:55 +0200
Message-ID: <486E008F.3080304@sun.com> (raw)
In-Reply-To: <486DF644.4070604@enterprisedb.com>
References: <48529B83.9010907@sun.com>
	<2e78013d0807030450m4cab60d0jd99d1acce59fae56@mail.gmail.com>
	<486DD1D1.50604@sun.com>
	<2e78013d0807040150w393ad72codaf06a6ac12a83ce@mail.gmail.com>
	<486DF644.4070604@enterprisedb.com>

Heikki Linnakangas napsal(a):
> Pavan Deolasee wrote:
>> On Fri, Jul 4, 2008 at 1:01 PM, Zdenek Kotala <Zdenek.Kotala@sun.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




view thread (17+ messages)  latest in thread

Message-ID: <486E008F.3080304@sun.com>
Permalink:  ../486E008F.3080304@sun.com/
Also on:    postgresql.org/message-id/486E008F.3080304@sun.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-patches@postgresql.org
  Cc: Zdenek.Kotala@Sun.COM, heikki@enterprisedb.com, pavan.deolasee@gmail.com
  Subject: Re: page macros cleanup
  In-Reply-To: <486E008F.3080304@sun.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox