Re: page macros cleanup (ver 04)

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)
   */