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 (ver 04)
Date: Wed, 09 Jul 2008 15:43:59 +0200
Message-ID: <4874C09F.4000302@sun.com> (raw)
In-Reply-To: <4874A6E0.9070909@enterprisedb.com>
References: <48529B83.9010907@sun.com>
<2e78013d0807030450m4cab60d0jd99d1acce59fae56@mail.gmail.com>
<486DD1D1.50604@sun.com>
<2e78013d0807040150w393ad72codaf06a6ac12a83ce@mail.gmail.com>
<486DF644.4070604@enterprisedb.com>
<2e78013d0807040350m3b2913cfkceaa70d8f9d697c2@mail.gmail.com>
<486E01B0.9050100@enterprisedb.com>
<2e78013d0807040404q5ff7f7faq9f87a39e0f0552fa@mail.gmail.com>
<486E138F.4060900@sun.com>
<4874A6E0.9070909@enterprisedb.com>
Heikki Linnakangas napsal(a):
> Zdenek Kotala wrote:
>> Pavan Deolasee napsal(a):
> There's another academical discrepancy between these two hunks:
<snip>
> I admit I don't understand what that bitmap is, it looks like we're
> assuming it can take up all the space between page header and the
> special portion, without any line pointers, but in HashPageGetBitmap,
> we're reserving space for one pointer. It looks like the actual size of
> the bitmap is only the largest power of 2 below the maximum size, so
> that won't be an issue in practice. That macro is actually doing the
> same thing as PageGetContents, so I switched to using that. As that
> moves the data sligthly on those bitmap pages, I guess we'll need a
> catversion bump.
Good catch. if we have to bump catalog version then I have there small patch
which introduce following macro and remove PageHeaderData stucture from
HashMetaPageData:
#define HashPageGetMeta(page) ((HashMetaPage) (PageGetContents(page)))
It also needs bump a catalog version and if we do it now I think it is better to
connect both these patches and do bump only once.
> Attached is an updated patch. I also fixed some whitespace and comments
> that were no longer valid. How does it look to you now?
Perfect. Thanks
Zdenek
Attachments:hash.patch (text/x-patch, 7.1K) download
diff -rc pgsql_hash.851ed84fb6aa/src/backend/access/hash/hash.c pgsql_hash/src/backend/access/hash/hash.c
*** pgsql_hash.851ed84fb6aa/src/backend/access/hash/hash.c út črc 8 21:31:38 2008
--- pgsql_hash/src/backend/access/hash/hash.c út črc 8 21:31:38 2008
***************
*** 527,533 ****
* each bucket.
*/
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
! metap = (HashMetaPage) BufferGetPage(metabuf);
orig_maxbucket = metap->hashm_maxbucket;
orig_ntuples = metap->hashm_ntuples;
memcpy(&local_metapage, metap, sizeof(local_metapage));
--- 527,533 ----
* each bucket.
*/
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
! metap = HashPageGetMeta(BufferGetPage(metabuf));
orig_maxbucket = metap->hashm_maxbucket;
orig_ntuples = metap->hashm_ntuples;
memcpy(&local_metapage, metap, sizeof(local_metapage));
***************
*** 629,635 ****
/* Write-lock metapage and check for split since we started */
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_WRITE, LH_META_PAGE);
! metap = (HashMetaPage) BufferGetPage(metabuf);
if (cur_maxbucket != metap->hashm_maxbucket)
{
--- 629,635 ----
/* Write-lock metapage and check for split since we started */
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_WRITE, LH_META_PAGE);
! metap = HashPageGetMeta(BufferGetPage(metabuf));
if (cur_maxbucket != metap->hashm_maxbucket)
{
diff -rc pgsql_hash.851ed84fb6aa/src/backend/access/hash/hashinsert.c pgsql_hash/src/backend/access/hash/hashinsert.c
*** pgsql_hash.851ed84fb6aa/src/backend/access/hash/hashinsert.c út črc 8 21:31:38 2008
--- pgsql_hash/src/backend/access/hash/hashinsert.c út črc 8 21:31:38 2008
***************
*** 69,75 ****
/* Read the metapage */
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
! metap = (HashMetaPage) BufferGetPage(metabuf);
/*
* Check whether the item can fit on a hash page at all. (Eventually, we
--- 69,75 ----
/* Read the metapage */
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
! metap = HashPageGetMeta(BufferGetPage(metabuf));
/*
* Check whether the item can fit on a hash page at all. (Eventually, we
diff -rc pgsql_hash.851ed84fb6aa/src/backend/access/hash/hashovfl.c pgsql_hash/src/backend/access/hash/hashovfl.c
*** pgsql_hash.851ed84fb6aa/src/backend/access/hash/hashovfl.c út črc 8 21:31:38 2008
--- pgsql_hash/src/backend/access/hash/hashovfl.c út črc 8 21:31:38 2008
***************
*** 187,193 ****
_hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_WRITE);
_hash_checkpage(rel, metabuf, LH_META_PAGE);
! metap = (HashMetaPage) BufferGetPage(metabuf);
/* start search at hashm_firstfree */
orig_firstfree = metap->hashm_firstfree;
--- 187,193 ----
_hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_WRITE);
_hash_checkpage(rel, metabuf, LH_META_PAGE);
! metap = HashPageGetMeta(BufferGetPage(metabuf));
/* start search at hashm_firstfree */
orig_firstfree = metap->hashm_firstfree;
***************
*** 450,456 ****
/* Read the metapage so we can determine which bitmap page to use */
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
! metap = (HashMetaPage) BufferGetPage(metabuf);
/* Identify which bit to set */
ovflbitno = blkno_to_bitno(metap, ovflblkno);
--- 450,456 ----
/* Read the metapage so we can determine which bitmap page to use */
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
! metap = HashPageGetMeta(BufferGetPage(metabuf));
/* Identify which bit to set */
ovflbitno = blkno_to_bitno(metap, ovflblkno);
diff -rc pgsql_hash.851ed84fb6aa/src/backend/access/hash/hashpage.c pgsql_hash/src/backend/access/hash/hashpage.c
*** pgsql_hash.851ed84fb6aa/src/backend/access/hash/hashpage.c út črc 8 21:31:38 2008
--- pgsql_hash/src/backend/access/hash/hashpage.c út črc 8 21:31:38 2008
***************
*** 395,401 ****
pageopaque->hasho_flag = LH_META_PAGE;
pageopaque->hasho_page_id = HASHO_PAGE_ID;
! metap = (HashMetaPage) pg;
metap->hashm_magic = HASH_MAGIC;
metap->hashm_version = HASH_VERSION;
--- 395,401 ----
pageopaque->hasho_flag = LH_META_PAGE;
pageopaque->hasho_page_id = HASHO_PAGE_ID;
! metap = HashPageGetMeta(pg);
metap->hashm_magic = HASH_MAGIC;
metap->hashm_version = HASH_VERSION;
***************
*** 532,538 ****
_hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_WRITE);
_hash_checkpage(rel, metabuf, LH_META_PAGE);
! metap = (HashMetaPage) BufferGetPage(metabuf);
/*
* Check to see if split is still needed; someone else might have already
--- 532,538 ----
_hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_WRITE);
_hash_checkpage(rel, metabuf, LH_META_PAGE);
! metap = HashPageGetMeta(BufferGetPage(metabuf));
/*
* Check to see if split is still needed; someone else might have already
diff -rc pgsql_hash.851ed84fb6aa/src/backend/access/hash/hashsearch.c pgsql_hash/src/backend/access/hash/hashsearch.c
*** pgsql_hash.851ed84fb6aa/src/backend/access/hash/hashsearch.c út črc 8 21:31:38 2008
--- pgsql_hash/src/backend/access/hash/hashsearch.c út črc 8 21:31:38 2008
***************
*** 186,192 ****
/* Read the metapage */
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
! metap = (HashMetaPage) BufferGetPage(metabuf);
/*
* Compute the target bucket number, and convert to block number.
--- 186,192 ----
/* Read the metapage */
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
! metap = HashPageGetMeta(BufferGetPage(metabuf));
/*
* Compute the target bucket number, and convert to block number.
diff -rc pgsql_hash.851ed84fb6aa/src/backend/access/hash/hashutil.c pgsql_hash/src/backend/access/hash/hashutil.c
*** pgsql_hash.851ed84fb6aa/src/backend/access/hash/hashutil.c út črc 8 21:31:38 2008
--- pgsql_hash/src/backend/access/hash/hashutil.c út črc 8 21:31:38 2008
***************
*** 191,197 ****
*/
if (flags == LH_META_PAGE)
{
! HashMetaPage metap = (HashMetaPage) page;
if (metap->hashm_magic != HASH_MAGIC)
ereport(ERROR,
--- 191,197 ----
*/
if (flags == LH_META_PAGE)
{
! HashMetaPage metap = HashPageGetMeta(page);
if (metap->hashm_magic != HASH_MAGIC)
ereport(ERROR,
diff -rc pgsql_hash.851ed84fb6aa/src/include/access/hash.h pgsql_hash/src/include/access/hash.h
*** pgsql_hash.851ed84fb6aa/src/include/access/hash.h út črc 8 21:31:38 2008
--- pgsql_hash/src/include/access/hash.h út črc 8 21:31:38 2008
***************
*** 138,144 ****
typedef struct HashMetaPageData
{
- PageHeaderData hashm_phdr; /* pad for page header (do not use) */
uint32 hashm_magic; /* magic no. for hash tables */
uint32 hashm_version; /* version ID */
double hashm_ntuples; /* number of tuples stored in the table */
--- 138,143 ----
***************
*** 162,167 ****
--- 161,168 ----
typedef HashMetaPageData *HashMetaPage;
+ #define HashPageGetMeta(page) ((HashMetaPage) (PageGetContents(page)))+
/*
* Maximum size of a hash index item (it's okay to have only one per page)
*/
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 (ver 04)
In-Reply-To: <4874C09F.4000302@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