>From 38760dd746cc5f345edf9a29654b9b2917377a44 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 13 Nov 2013 18:15:14 +0200 Subject: [PATCH 1/4] Further GIN refactoring. Merge some functions that were always called together. Makes the code little bit more readable. --- src/backend/access/gin/ginbtree.c | 54 +++++++++--------------- src/backend/access/gin/gindatapage.c | 79 ++++++++++++++--------------------- src/backend/access/gin/ginentrypage.c | 5 +-- src/backend/access/gin/ginget.c | 24 +++++------ src/backend/access/gin/gininsert.c | 21 ++++------ src/include/access/gin_private.h | 19 ++------- 6 files changed, 74 insertions(+), 128 deletions(-) diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c index 010812d..0d93c52 100644 --- a/src/backend/access/gin/ginbtree.c +++ b/src/backend/access/gin/ginbtree.c @@ -52,52 +52,38 @@ ginTraverseLock(Buffer buffer, bool searchMode) return access; } -GinBtreeStack * -ginPrepareFindLeafPage(GinBtree btree, BlockNumber blkno) -{ - GinBtreeStack *stack = (GinBtreeStack *) palloc(sizeof(GinBtreeStack)); - - stack->blkno = blkno; - stack->buffer = ReadBuffer(btree->index, stack->blkno); - stack->parent = NULL; - stack->predictNumber = 1; - - ginTraverseLock(stack->buffer, btree->searchMode); - - return stack; -} - /* - * Locates leaf page contained tuple + * Descends the tree to the leaf page that contains or would contain the + * key we're searching for. The key should already be filled in 'btree', + * in tree-type specific manner. If btree->fullScan is true, descends to the + * leftmost leaf page. + * + * If 'searchmode' is false, on return stack->buffer is exclusively locked, + * and the stack represents the full path to the root. Otherwise stack->buffer + * is share-locked, and stack->parent is NULL. */ GinBtreeStack * -ginFindLeafPage(GinBtree btree, GinBtreeStack *stack) +ginFindLeafPage(GinBtree btree, BlockNumber rootBlkno, bool searchMode) { - bool isfirst = TRUE; - BlockNumber rootBlkno; + GinBtreeStack *stack; - if (!stack) - stack = ginPrepareFindLeafPage(btree, GIN_ROOT_BLKNO); - rootBlkno = stack->blkno; + stack = (GinBtreeStack *) palloc(sizeof(GinBtreeStack)); + stack->blkno = rootBlkno; + stack->buffer = ReadBuffer(btree->index, rootBlkno); + stack->parent = NULL; + stack->predictNumber = 1; for (;;) { Page page; BlockNumber child; - int access = GIN_SHARE; + int access; stack->off = InvalidOffsetNumber; page = BufferGetPage(stack->buffer); - if (isfirst) - { - if (GinPageIsLeaf(page) && !btree->searchMode) - access = GIN_EXCLUSIVE; - isfirst = FALSE; - } - else - access = ginTraverseLock(stack->buffer, btree->searchMode); + access = ginTraverseLock(stack->buffer, searchMode); /* * ok, page is correctly locked, we should check to move right .., @@ -127,7 +113,7 @@ ginFindLeafPage(GinBtree btree, GinBtreeStack *stack) Assert(child != InvalidBlockNumber); Assert(stack->blkno != child); - if (btree->searchMode) + if (searchMode) { /* in search mode we may forget path to leaf */ stack->blkno = child; @@ -251,7 +237,7 @@ ginFindParents(GinBtree btree, GinBtreeStack *stack, return; } - leftmostBlkno = blkno = btree->getLeftMostPage(btree, page); + leftmostBlkno = blkno = btree->getLeftMostChild(btree, page); LockBuffer(root->buffer, GIN_UNLOCK); Assert(blkno != InvalidBlockNumber); @@ -263,7 +249,7 @@ ginFindParents(GinBtree btree, GinBtreeStack *stack, if (GinPageIsLeaf(page)) elog(ERROR, "Lost path"); - leftmostBlkno = btree->getLeftMostPage(btree, page); + leftmostBlkno = btree->getLeftMostChild(btree, page); while ((offset = btree->findChildPtr(btree, page, stack->blkno, InvalidOffsetNumber)) == InvalidOffsetNumber) { diff --git a/src/backend/access/gin/gindatapage.c b/src/backend/access/gin/gindatapage.c index c9506cc..221b39e 100644 --- a/src/backend/access/gin/gindatapage.c +++ b/src/backend/access/gin/gindatapage.c @@ -54,7 +54,7 @@ dataLocateItem(GinBtree btree, GinBtreeStack *stack) { stack->off = FirstOffsetNumber; stack->predictNumber *= GinPageGetOpaque(page)->maxoff; - return btree->getLeftMostPage(btree, page); + return btree->getLeftMostChild(btree, page); } low = FirstOffsetNumber; @@ -680,17 +680,10 @@ createPostingTree(Relation index, ItemPointerData *items, uint32 nitems, */ if (nitems > nrootitems) { - GinPostingTreeScan *gdi; - - gdi = ginPrepareScanPostingTree(index, blkno, FALSE); - gdi->btree.isBuild = (buildStats != NULL); - - ginInsertItemPointers(gdi, + ginInsertItemPointers(index, blkno, items + nrootitems, nitems - nrootitems, buildStats); - - pfree(gdi); } return blkno; @@ -704,76 +697,66 @@ ginPrepareDataScan(GinBtree btree, Relation index) btree->index = index; btree->findChildPage = dataLocateItem; + btree->getLeftMostChild = dataGetLeftMostPage; btree->isMoveRight = dataIsMoveRight; btree->findItem = dataLocateLeafItem; btree->findChildPtr = dataFindChildPtr; - btree->getLeftMostPage = dataGetLeftMostPage; btree->placeToPage = dataPlaceToPage; btree->splitPage = dataSplitPage; btree->fillRoot = ginDataFillRoot; btree->isData = TRUE; - btree->searchMode = FALSE; btree->isDelete = FALSE; btree->fullScan = FALSE; btree->isBuild = FALSE; } -GinPostingTreeScan * -ginPrepareScanPostingTree(Relation index, BlockNumber rootBlkno, bool searchMode) -{ - GinPostingTreeScan *gdi = (GinPostingTreeScan *) palloc0(sizeof(GinPostingTreeScan)); - - ginPrepareDataScan(&gdi->btree, index); - - gdi->btree.searchMode = searchMode; - gdi->btree.fullScan = searchMode; - - gdi->stack = ginPrepareFindLeafPage(&gdi->btree, rootBlkno); - - return gdi; -} - /* * Inserts array of item pointers, may execute several tree scan (very rare) */ void -ginInsertItemPointers(GinPostingTreeScan *gdi, +ginInsertItemPointers(Relation index, BlockNumber rootBlkno, ItemPointerData *items, uint32 nitem, GinStatsData *buildStats) { - BlockNumber rootBlkno = gdi->stack->blkno; + GinBtreeData btree; + GinBtreeStack *stack; - gdi->btree.items = items; - gdi->btree.nitem = nitem; - gdi->btree.curitem = 0; + ginPrepareDataScan(&btree, index); + btree.isBuild = (buildStats != NULL); + btree.items = items; + btree.nitem = nitem; + btree.curitem = 0; - while (gdi->btree.curitem < gdi->btree.nitem) + while (btree.curitem < btree.nitem) { - if (!gdi->stack) - gdi->stack = ginPrepareFindLeafPage(&gdi->btree, rootBlkno); + stack = ginFindLeafPage(&btree, rootBlkno, false); - gdi->stack = ginFindLeafPage(&gdi->btree, gdi->stack); - - if (gdi->btree.findItem(&(gdi->btree), gdi->stack)) + if (btree.findItem(&btree, stack)) { /* - * gdi->btree.items[gdi->btree.curitem] already exists in index + * btree.items[btree.curitem] already exists in index */ - gdi->btree.curitem++; - LockBuffer(gdi->stack->buffer, GIN_UNLOCK); - freeGinBtreeStack(gdi->stack); + btree.curitem++; + LockBuffer(stack->buffer, GIN_UNLOCK); + freeGinBtreeStack(stack); } else - ginInsertValue(&(gdi->btree), gdi->stack, buildStats); - - gdi->stack = NULL; + ginInsertValue(&btree, stack, buildStats); } } -Buffer -ginScanBeginPostingTree(GinPostingTreeScan *gdi) +GinBtreeStack * +ginScanBeginPostingTree(Relation index, BlockNumber rootBlkno) { - gdi->stack = ginFindLeafPage(&gdi->btree, gdi->stack); - return gdi->stack->buffer; + GinBtreeData btree; + GinBtreeStack *stack; + + ginPrepareDataScan(&btree, index); + + btree.fullScan = TRUE; + + stack = ginFindLeafPage(&btree, rootBlkno, TRUE); + + return stack; } diff --git a/src/backend/access/gin/ginentrypage.c b/src/backend/access/gin/ginentrypage.c index 0ed0a3d..a22fd32 100644 --- a/src/backend/access/gin/ginentrypage.c +++ b/src/backend/access/gin/ginentrypage.c @@ -258,7 +258,7 @@ entryLocateEntry(GinBtree btree, GinBtreeStack *stack) { stack->off = FirstOffsetNumber; stack->predictNumber *= PageGetMaxOffsetNumber(page); - return btree->getLeftMostPage(btree, page); + return btree->getLeftMostChild(btree, page); } low = FirstOffsetNumber; @@ -729,16 +729,15 @@ ginPrepareEntryScan(GinBtree btree, OffsetNumber attnum, btree->ginstate = ginstate; btree->findChildPage = entryLocateEntry; + btree->getLeftMostChild = entryGetLeftMostPage; btree->isMoveRight = entryIsMoveRight; btree->findItem = entryLocateLeafEntry; btree->findChildPtr = entryFindChildPtr; - btree->getLeftMostPage = entryGetLeftMostPage; btree->placeToPage = entryPlaceToPage; btree->splitPage = entrySplitPage; btree->fillRoot = ginEntryFillRoot; btree->isData = FALSE; - btree->searchMode = FALSE; btree->fullScan = FALSE; btree->isBuild = FALSE; diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c index acbdc5f..2a3991f 100644 --- a/src/backend/access/gin/ginget.c +++ b/src/backend/access/gin/ginget.c @@ -124,18 +124,16 @@ static void scanPostingTree(Relation index, GinScanEntry scanEntry, BlockNumber rootPostingTree) { - GinPostingTreeScan *gdi; + GinBtreeStack *stack; Buffer buffer; Page page; /* Descend to the leftmost leaf page */ - gdi = ginPrepareScanPostingTree(index, rootPostingTree, TRUE); - - buffer = ginScanBeginPostingTree(gdi); + stack = ginScanBeginPostingTree(index, rootPostingTree); + buffer = stack->buffer; IncrBufferRefCount(buffer); /* prevent unpin in freeGinBtreeStack */ - freeGinBtreeStack(gdi->stack); - pfree(gdi); + freeGinBtreeStack(stack); /* * Loop iterates through all leaf pages of posting tree @@ -376,8 +374,7 @@ restartScanEntry: ginPrepareEntryScan(&btreeEntry, entry->attnum, entry->queryKey, entry->queryCategory, ginstate); - btreeEntry.searchMode = TRUE; - stackEntry = ginFindLeafPage(&btreeEntry, NULL); + stackEntry = ginFindLeafPage(&btreeEntry, GIN_ROOT_BLKNO, true); page = BufferGetPage(stackEntry->buffer); needUnlock = TRUE; @@ -427,7 +424,7 @@ restartScanEntry: if (GinIsPostingTree(itup)) { BlockNumber rootPostingTree = GinGetPostingTree(itup); - GinPostingTreeScan *gdi; + GinBtreeStack *stack; Page page; /* @@ -439,9 +436,9 @@ restartScanEntry: */ LockBuffer(stackEntry->buffer, GIN_UNLOCK); needUnlock = FALSE; - gdi = ginPrepareScanPostingTree(ginstate->index, rootPostingTree, TRUE); - entry->buffer = ginScanBeginPostingTree(gdi); + stack = ginScanBeginPostingTree(ginstate->index, rootPostingTree); + entry->buffer = stack->buffer; /* * We keep buffer pinned because we need to prevent deletion of @@ -451,7 +448,7 @@ restartScanEntry: IncrBufferRefCount(entry->buffer); page = BufferGetPage(entry->buffer); - entry->predictNumberResult = gdi->stack->predictNumber * GinPageGetOpaque(page)->maxoff; + entry->predictNumberResult = stack->predictNumber * GinPageGetOpaque(page)->maxoff; /* * Keep page content in memory to prevent durable page locking @@ -463,8 +460,7 @@ restartScanEntry: GinPageGetOpaque(page)->maxoff * sizeof(ItemPointerData)); LockBuffer(entry->buffer, GIN_UNLOCK); - freeGinBtreeStack(gdi->stack); - pfree(gdi); + freeGinBtreeStack(stack); entry->isFinished = FALSE; } else if (GinGetNPosting(itup) > 0) diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c index fb7b4ea..0a2883a 100644 --- a/src/backend/access/gin/gininsert.c +++ b/src/backend/access/gin/gininsert.c @@ -81,7 +81,6 @@ addItemPointersToLeafTuple(GinState *ginstate, { /* posting list would be too big, convert to posting tree */ BlockNumber postingRoot; - GinPostingTreeScan *gdi; /* * Initialize posting tree with the old tuple's posting list. It's @@ -94,12 +93,9 @@ addItemPointersToLeafTuple(GinState *ginstate, buildStats); /* Now insert the TIDs-to-be-added into the posting tree */ - gdi = ginPrepareScanPostingTree(ginstate->index, postingRoot, FALSE); - gdi->btree.isBuild = (buildStats != NULL); - - ginInsertItemPointers(gdi, items, nitem, buildStats); - - pfree(gdi); + ginInsertItemPointers(ginstate->index, postingRoot, + items, nitem, + buildStats); /* And build a new posting-tree-only result tuple */ res = GinFormTuple(ginstate, attnum, key, category, NULL, 0, true); @@ -177,7 +173,7 @@ ginEntryInsert(GinState *ginstate, ginPrepareEntryScan(&btree, attnum, key, category, ginstate); - stack = ginFindLeafPage(&btree, NULL); + stack = ginFindLeafPage(&btree, GIN_ROOT_BLKNO, false); page = BufferGetPage(stack->buffer); if (btree.findItem(&btree, stack)) @@ -189,18 +185,15 @@ ginEntryInsert(GinState *ginstate, { /* add entries to existing posting tree */ BlockNumber rootPostingTree = GinGetPostingTree(itup); - GinPostingTreeScan *gdi; /* release all stack */ LockBuffer(stack->buffer, GIN_UNLOCK); freeGinBtreeStack(stack); /* insert into posting tree */ - gdi = ginPrepareScanPostingTree(ginstate->index, rootPostingTree, FALSE); - gdi->btree.isBuild = (buildStats != NULL); - ginInsertItemPointers(gdi, items, nitem, buildStats); - pfree(gdi); - + ginInsertItemPointers(ginstate->index, rootPostingTree, + items, nitem, + buildStats); return; } diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h index d22cc62..7491d7c 100644 --- a/src/include/access/gin_private.h +++ b/src/include/access/gin_private.h @@ -479,18 +479,17 @@ typedef struct GinBtreeData { /* search methods */ BlockNumber (*findChildPage) (GinBtree, GinBtreeStack *); + BlockNumber (*getLeftMostChild) (GinBtree, Page); bool (*isMoveRight) (GinBtree, Page); bool (*findItem) (GinBtree, GinBtreeStack *); /* insert methods */ OffsetNumber (*findChildPtr) (GinBtree, Page, BlockNumber, OffsetNumber); - BlockNumber (*getLeftMostPage) (GinBtree, Page); bool (*placeToPage) (GinBtree, Buffer, OffsetNumber, XLogRecData **); Page (*splitPage) (GinBtree, Buffer, Buffer, OffsetNumber, XLogRecData **); void (*fillRoot) (GinBtree, Buffer, Buffer, Buffer); bool isData; - bool searchMode; Relation index; GinState *ginstate; /* not valid in a data scan */ @@ -514,8 +513,7 @@ typedef struct GinBtreeData PostingItem pitem; } GinBtreeData; -extern GinBtreeStack *ginPrepareFindLeafPage(GinBtree btree, BlockNumber blkno); -extern GinBtreeStack *ginFindLeafPage(GinBtree btree, GinBtreeStack *stack); +extern GinBtreeStack *ginFindLeafPage(GinBtree btree, BlockNumber rootBlkno, bool searchMode); extern Buffer ginStepRight(Buffer buffer, Relation index, int lockmode); extern void freeGinBtreeStack(GinBtreeStack *stack); extern void ginInsertValue(GinBtree btree, GinBtreeStack *stack, @@ -540,19 +538,10 @@ extern BlockNumber createPostingTree(Relation index, extern void GinDataPageAddItemPointer(Page page, ItemPointer data, OffsetNumber offset); extern void GinDataPageAddPostingItem(Page page, PostingItem *data, OffsetNumber offset); extern void GinPageDeletePostingItem(Page page, OffsetNumber offset); - -typedef struct -{ - GinBtreeData btree; - GinBtreeStack *stack; -} GinPostingTreeScan; - -extern GinPostingTreeScan *ginPrepareScanPostingTree(Relation index, - BlockNumber rootBlkno, bool searchMode); -extern void ginInsertItemPointers(GinPostingTreeScan *gdi, +extern void ginInsertItemPointers(Relation index, BlockNumber rootBlkno, ItemPointerData *items, uint32 nitem, GinStatsData *buildStats); -extern Buffer ginScanBeginPostingTree(GinPostingTreeScan *gdi); +extern GinBtreeStack *ginScanBeginPostingTree(Relation index, BlockNumber rootBlkno); extern void ginDataFillRoot(GinBtree btree, Buffer root, Buffer lbuf, Buffer rbuf); extern void ginPrepareDataScan(GinBtree btree, Relation index); -- 1.8.4.rc3