Received: from localhost (unknown [200.46.204.183]) by postgresql.org (Postfix) with ESMTP id CEEFD64FCEC for ; Mon, 8 Sep 2008 11:24:12 -0300 (ADT) Received: from postgresql.org ([200.46.204.86]) by localhost (mx1.hub.org [200.46.204.183]) (amavisd-maia, port 10024) with ESMTP id 31392-03 for ; Mon, 8 Sep 2008 11:24:09 -0300 (ADT) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from gmp-eb-inf-1.sun.com (gmp-eb-inf-1.sun.com [192.18.6.21]) by postgresql.org (Postfix) with ESMTP id 7FC2C64FCE6 for ; Mon, 8 Sep 2008 11:24:09 -0300 (ADT) Received: from fe-emea-10.sun.com (gmp-eb-lb-2-fe2.eu.sun.com [192.18.6.11]) by gmp-eb-inf-1.sun.com (8.13.7+Sun/8.12.9) with ESMTP id m88EO65A021566 for ; Mon, 8 Sep 2008 14:24:07 GMT Received: from conversion-daemon.fe-emea-10.sun.com by fe-emea-10.sun.com (Sun Java System Messaging Server 6.2-8.04 (built Feb 28 2007)) id <0K6V00D01RS4NJ00@fe-emea-10.sun.com> (original mail from Zdenek.Kotala@Sun.COM) for pgsql-patches@postgresql.org; Mon, 08 Sep 2008 15:24:06 +0100 (BST) Received: from [192.168.1.101] ([89.102.161.149]) by fe-emea-10.sun.com (Sun Java System Messaging Server 6.2-8.04 (built Feb 28 2007)) with ESMTPSA id <0K6V007OAS05M5F0@fe-emea-10.sun.com>; Mon, 08 Sep 2008 15:24:06 +0100 (BST) Date: Mon, 08 Sep 2008 16:20:15 +0200 From: Zdenek Kotala Subject: Re: hash index improving v3 In-reply-to: <22574.1220728178@sss.pgh.pa.us> To: Tom Lane Cc: Alex Hunsaker , Xiao Meng , pgsql-patches@postgresql.org, "Jonah H. Harris" Message-id: <48C5349F.4080300@sun.com> MIME-version: 1.0 Content-type: multipart/mixed; boundary="Boundary_(ID_m24wr7fv51+dn/euXc1KVw)" References: <7627.1220569888@sss.pgh.pa.us> <34d269d40809041742r6a0703dbgfbdd012f7f19de2e@mail.gmail.com> <10995.1220577198@sss.pgh.pa.us> <11525.1220579153@sss.pgh.pa.us> <34d269d40809041854g355a8f68ga3a037a69e3d9fcb@mail.gmail.com> <11962.1220581027@sss.pgh.pa.us> <34d269d40809042048m3b532a5ej7a9588f63a788d0c@mail.gmail.com> <34d269d40809050912u33f2afc8mb90e316bc465136e@mail.gmail.com> <34d269d40809051321w2ed16037od0373a4a29c311f1@mail.gmail.com> <34d269d40809052249h59750f83v4b19a50d0555580@mail.gmail.com> <22574.1220728178@sss.pgh.pa.us> User-Agent: Thunderbird 2.0.0.16 (X11/20080811) X-Virus-Scanned: Maia Mailguard 1.0.1 X-Archive-Number: 200809/91 X-Sequence-Number: 7106 This is a multi-part message in MIME format. --Boundary_(ID_m24wr7fv51+dn/euXc1KVw) Content-type: text/plain; format=flowed; charset=UTF-8 Content-transfer-encoding: 7BIT Tom Lane napsal(a): > "Alex Hunsaker" writes: >> On Fri, Sep 5, 2008 at 2:21 PM, Alex Hunsaker wrote: >>> Ok now that I made it so it actually *test* collisions, with the patch >>> it always returns all rows that matched the hashed "key". > >> And here is the fix, we just forget to set the recheck flag for bitmap scans. > > For the convenience of anyone intending to test, here is an updated > patch against CVS HEAD that incorporates Alex's fix. > Tom, I attach cleanup patch which we discussed last commit fest. It introduce new macros HashGetMetaPage and HashGetBitmap and of course, it break backward on disk format compatibility which we will break it anyway when Xiao's patch will be committed. Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql --Boundary_(ID_m24wr7fv51+dn/euXc1KVw) Content-type: text/x-diff; name=hash_cleanup.patch Content-transfer-encoding: 8BIT Content-disposition: inline; filename=hash_cleanup.patch běžné podadresáře: pgsql_hash.be486df40421/src a pgsql_hash/src běžné podadresáře: pgsql_hash.be486df40421/src/backend a pgsql_hash/src/backend běžné podadresáře: pgsql_hash.be486df40421/src/include a pgsql_hash/src/include běžné podadresáře: pgsql_hash.be486df40421/src/backend/access a pgsql_hash/src/backend/access běžné podadresáře: pgsql_hash.be486df40421/src/backend/access/hash a pgsql_hash/src/backend/access/hash diff -rc pgsql_hash.be486df40421/src/backend/access/hash/hash.c pgsql_hash/src/backend/access/hash/hash.c *** pgsql_hash.be486df40421/src/backend/access/hash/hash.c po zář 8 16:14:00 2008 --- pgsql_hash/src/backend/access/hash/hash.c po zář 8 16:14:00 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.be486df40421/src/backend/access/hash/hashinsert.c pgsql_hash/src/backend/access/hash/hashinsert.c *** pgsql_hash.be486df40421/src/backend/access/hash/hashinsert.c po zář 8 16:14:00 2008 --- pgsql_hash/src/backend/access/hash/hashinsert.c po zář 8 16:14:00 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.be486df40421/src/backend/access/hash/hashovfl.c pgsql_hash/src/backend/access/hash/hashovfl.c *** pgsql_hash.be486df40421/src/backend/access/hash/hashovfl.c po zář 8 16:14:00 2008 --- pgsql_hash/src/backend/access/hash/hashovfl.c po zář 8 16:14:00 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.be486df40421/src/backend/access/hash/hashpage.c pgsql_hash/src/backend/access/hash/hashpage.c *** pgsql_hash.be486df40421/src/backend/access/hash/hashpage.c po zář 8 16:14:00 2008 --- pgsql_hash/src/backend/access/hash/hashpage.c po zář 8 16:14:00 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; *************** *** 402,414 **** metap->hashm_ntuples = 0; metap->hashm_nmaps = 0; metap->hashm_ffactor = ffactor; ! metap->hashm_bsize = BufferGetPageSize(metabuf); /* find largest bitmap array size that will fit in page size */ for (i = _hash_log2(metap->hashm_bsize); i > 0; --i) { ! if ((1 << i) <= (metap->hashm_bsize - ! (MAXALIGN(sizeof(PageHeaderData)) + ! MAXALIGN(sizeof(HashPageOpaqueData))))) break; } Assert(i > 0); --- 402,412 ---- metap->hashm_ntuples = 0; metap->hashm_nmaps = 0; metap->hashm_ffactor = ffactor; ! metap->hashm_bsize = HashGetMaxBitmapSize(pg); /* find largest bitmap array size that will fit in page size */ for (i = _hash_log2(metap->hashm_bsize); i > 0; --i) { ! if ((1 << i) <= metap->hashm_bsize) break; } Assert(i > 0); *************** *** 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 --- 530,536 ---- _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.be486df40421/src/backend/access/hash/hashsearch.c pgsql_hash/src/backend/access/hash/hashsearch.c *** pgsql_hash.be486df40421/src/backend/access/hash/hashsearch.c po zář 8 16:14:00 2008 --- pgsql_hash/src/backend/access/hash/hashsearch.c po zář 8 16:14:00 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.be486df40421/src/backend/access/hash/hashutil.c pgsql_hash/src/backend/access/hash/hashutil.c *** pgsql_hash.be486df40421/src/backend/access/hash/hashutil.c po zář 8 16:14:00 2008 --- pgsql_hash/src/backend/access/hash/hashutil.c po zář 8 16:14:00 2008 *************** *** 190,196 **** */ if (flags == LH_META_PAGE) { ! HashMetaPage metap = (HashMetaPage) page; if (metap->hashm_magic != HASH_MAGIC) ereport(ERROR, --- 190,196 ---- */ if (flags == LH_META_PAGE) { ! HashMetaPage metap = HashPageGetMeta(page); if (metap->hashm_magic != HASH_MAGIC) ereport(ERROR, běžné podadresáře: pgsql_hash.be486df40421/src/include/access a pgsql_hash/src/include/access diff -rc pgsql_hash.be486df40421/src/include/access/hash.h pgsql_hash/src/include/access/hash.h *** pgsql_hash.be486df40421/src/include/access/hash.h po zář 8 16:14:00 2008 --- pgsql_hash/src/include/access/hash.h po zář 8 16:14:00 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 ---- *************** *** 191,199 **** #define BMPGSZ_BIT(metap) ((metap)->hashm_bmsize << BYTE_TO_BIT) #define BMPG_SHIFT(metap) ((metap)->hashm_bmshift) #define BMPG_MASK(metap) (BMPGSZ_BIT(metap) - 1) - #define HashPageGetBitmap(pg) \ - ((uint32 *) (((char *) (pg)) + MAXALIGN(sizeof(PageHeaderData)))) /* * The number of bits in an ovflpage bitmap word. */ --- 190,207 ---- #define BMPGSZ_BIT(metap) ((metap)->hashm_bmsize << BYTE_TO_BIT) #define BMPG_SHIFT(metap) ((metap)->hashm_bmshift) #define BMPG_MASK(metap) (BMPGSZ_BIT(metap) - 1) + #define HashPageGetBitmap(page) \ + ((uint32*) PageGetContents(page)) + + #define HashGetMaxBitmapSize(page) \ + (PageGetPageSize((Page) page)- \ + (MAXALIGN(SizeOfPageHeaderData) + \ + MAXALIGN(sizeof(HashPageOpaqueData))) ) + + #define HashPageGetMeta(page) \ + ((HashMetaPage) PageGetContents(page)) + /* * The number of bits in an ovflpage bitmap word. */ --Boundary_(ID_m24wr7fv51+dn/euXc1KVw)--