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





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)
   */
-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Reply via email to