From 668cd50fb686dce91c6c68576dcd4d0c5d6b2b28 Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Wed, 23 Aug 2017 16:01:41 +0530 Subject: [PATCH 2/3] Remove redundant hash function _hash_step and do some code cleanup. Remove redundant function _hash_step() and some of the unused members of HashScanOpaqueData. The function _hash_step() used to find the next qualifing tuple in the index page is no more required as new hash index works page at a time which means it reads all the qualifing tuples in a page at once with the help of _hash_readpage(). Patch by Ashutosh Sharma. --- src/backend/access/hash/hashsearch.c | 206 ----------------------------------- src/include/access/hash.h | 15 --- 2 files changed, 221 deletions(-) diff --git a/src/backend/access/hash/hashsearch.c b/src/backend/access/hash/hashsearch.c index 0c3338a..c379a3b 100644 --- a/src/backend/access/hash/hashsearch.c +++ b/src/backend/access/hash/hashsearch.c @@ -438,212 +438,6 @@ _hash_first(IndexScanDesc scan, ScanDirection dir) } /* - * _hash_step() -- step to the next valid item in a scan in the bucket. - * - * If no valid record exists in the requested direction, return - * false. Else, return true and set the hashso_curpos for the - * scan to the right thing. - * - * Here we need to ensure that if the scan has started during split, then - * skip the tuples that are moved by split while scanning bucket being - * populated and then scan the bucket being split to cover all such - * tuples. This is done to ensure that we don't miss tuples in the scans - * that are started during split. - * - * 'bufP' points to the current buffer, which is pinned and read-locked. - * On success exit, we have pin and read-lock on whichever page - * contains the right item; on failure, we have released all buffers. - */ -bool -_hash_step(IndexScanDesc scan, Buffer *bufP, ScanDirection dir) -{ - Relation rel = scan->indexRelation; - HashScanOpaque so = (HashScanOpaque) scan->opaque; - ItemPointer current; - Buffer buf; - Page page; - HashPageOpaque opaque; - OffsetNumber maxoff; - OffsetNumber offnum; - BlockNumber blkno; - IndexTuple itup; - - current = &(so->hashso_curpos); - - buf = *bufP; - _hash_checkpage(rel, buf, LH_BUCKET_PAGE | LH_OVERFLOW_PAGE); - page = BufferGetPage(buf); - opaque = (HashPageOpaque) PageGetSpecialPointer(page); - - /* - * If _hash_step is called from _hash_first, current will not be valid, so - * we can't dereference it. However, in that case, we presumably want to - * start at the beginning/end of the page... - */ - maxoff = PageGetMaxOffsetNumber(page); - if (ItemPointerIsValid(current)) - offnum = ItemPointerGetOffsetNumber(current); - else - offnum = InvalidOffsetNumber; - - /* - * 'offnum' now points to the last tuple we examined (if any). - * - * continue to step through tuples until: 1) we get to the end of the - * bucket chain or 2) we find a valid tuple. - */ - do - { - switch (dir) - { - case ForwardScanDirection: - if (offnum != InvalidOffsetNumber) - offnum = OffsetNumberNext(offnum); /* move forward */ - else - { - /* new page, locate starting position by binary search */ - offnum = _hash_binsearch(page, so->hashso_sk_hash); - } - - for (;;) - { - /* - * check if we're still in the range of items with the - * target hash key - */ - if (offnum <= maxoff) - { - Assert(offnum >= FirstOffsetNumber); - itup = (IndexTuple) PageGetItem(page, PageGetItemId(page, offnum)); - - /* - * skip the tuples that are moved by split operation - * for the scan that has started when split was in - * progress - */ - if (so->hashso_buc_populated && !so->hashso_buc_split && - (itup->t_info & INDEX_MOVED_BY_SPLIT_MASK)) - { - offnum = OffsetNumberNext(offnum); /* move forward */ - continue; - } - - if (so->hashso_sk_hash == _hash_get_indextuple_hashkey(itup)) - break; /* yes, so exit for-loop */ - } - - /* Before leaving current page, deal with any killed items */ - if (so->numKilled > 0) - _hash_kill_items(scan); - - /* - * ran off the end of this page, try the next - */ - _hash_readnext(scan, &buf, &page, &opaque); - if (BufferIsValid(buf)) - { - maxoff = PageGetMaxOffsetNumber(page); - offnum = _hash_binsearch(page, so->hashso_sk_hash); - } - else - { - itup = NULL; - break; /* exit for-loop */ - } - } - break; - - case BackwardScanDirection: - if (offnum != InvalidOffsetNumber) - offnum = OffsetNumberPrev(offnum); /* move back */ - else - { - /* new page, locate starting position by binary search */ - offnum = _hash_binsearch_last(page, so->hashso_sk_hash); - } - - for (;;) - { - /* - * check if we're still in the range of items with the - * target hash key - */ - if (offnum >= FirstOffsetNumber) - { - Assert(offnum <= maxoff); - itup = (IndexTuple) PageGetItem(page, PageGetItemId(page, offnum)); - - /* - * skip the tuples that are moved by split operation - * for the scan that has started when split was in - * progress - */ - if (so->hashso_buc_populated && !so->hashso_buc_split && - (itup->t_info & INDEX_MOVED_BY_SPLIT_MASK)) - { - offnum = OffsetNumberPrev(offnum); /* move back */ - continue; - } - - if (so->hashso_sk_hash == _hash_get_indextuple_hashkey(itup)) - break; /* yes, so exit for-loop */ - } - - /* Before leaving current page, deal with any killed items */ - if (so->numKilled > 0) - _hash_kill_items(scan); - - /* - * ran off the end of this page, try the next - */ - _hash_readprev(scan, &buf, &page, &opaque); - if (BufferIsValid(buf)) - { - TestForOldSnapshot(scan->xs_snapshot, rel, page); - maxoff = PageGetMaxOffsetNumber(page); - offnum = _hash_binsearch_last(page, so->hashso_sk_hash); - } - else - { - itup = NULL; - break; /* exit for-loop */ - } - } - break; - - default: - /* NoMovementScanDirection */ - /* this should not be reached */ - itup = NULL; - break; - } - - if (itup == NULL) - { - /* - * We ran off the end of the bucket without finding a match. - * Release the pin on bucket buffers. Normally, such pins are - * released at end of scan, however scrolling cursors can - * reacquire the bucket lock and pin in the same scan multiple - * times. - */ - *bufP = so->hashso_curbuf = InvalidBuffer; - ItemPointerSetInvalid(current); - _hash_dropscanbuf(rel, so); - return false; - } - - /* check the tuple quals, loop around if not met */ - } while (!_hash_checkqual(scan, itup)); - - /* if we made it to here, we've found a valid tuple */ - blkno = BufferGetBlockNumber(buf); - *bufP = so->hashso_curbuf = buf; - ItemPointerSet(current, blkno, offnum); - return true; -} - -/* * _hash_readpage() -- Load data from current index page into so->currPos * * We scan all the items in the current index page and save them into diff --git a/src/include/access/hash.h b/src/include/access/hash.h index 3e90b89..19fb147 100644 --- a/src/include/access/hash.h +++ b/src/include/access/hash.h @@ -159,14 +159,6 @@ typedef struct HashScanOpaqueData /* Hash value of the scan key, ie, the hash key we seek */ uint32 hashso_sk_hash; - /* - * We also want to remember which buffer we're currently examining in the - * scan. We keep the buffer pinned (but not locked) across hashgettuple - * calls, in order to avoid doing a ReadBuffer() for every tuple in the - * index. - */ - Buffer hashso_curbuf; - /* remember the buffer associated with primary bucket */ Buffer hashso_bucket_buf; @@ -177,12 +169,6 @@ typedef struct HashScanOpaqueData */ Buffer hashso_split_bucket_buf; - /* Current position of the scan, as an index TID */ - ItemPointerData hashso_curpos; - - /* Current position of the scan, as a heap TID */ - ItemPointerData hashso_heappos; - /* Whether scan starts on bucket being populated due to split */ bool hashso_buc_populated; @@ -432,7 +418,6 @@ extern void _hash_finish_split(Relation rel, Buffer metabuf, Buffer obuf, /* hashsearch.c */ extern bool _hash_next(IndexScanDesc scan, ScanDirection dir); extern bool _hash_first(IndexScanDesc scan, ScanDirection dir); -extern bool _hash_step(IndexScanDesc scan, Buffer *bufP, ScanDirection dir); /* hashsort.c */ typedef struct HSpool HSpool; /* opaque struct in hashsort.c */ -- 1.8.4.msysgit.0