Re: Small patch for GiST: move childoffnum to child

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>

From: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, Jeff Janes <jeff.janes@gmail.com>
Date: 2011-07-14T08:56:58Z
Lists: pgsql-hackers
I think there's two bugs in the existing gistFindPath code:

> 		if (top->parent && XLByteLT(top->parent->lsn, GistPageGetOpaque(page)->nsn) &&
> 			GistPageGetOpaque(page)->rightlink != InvalidBlockNumber /* sanity check */ )
> 		{
> 			/* page splited while we thinking of... */
> 			ptr = (GISTInsertStack *) palloc0(sizeof(GISTInsertStack));
> 			ptr->blkno = GistPageGetOpaque(page)->rightlink;
> 			ptr->childoffnum = InvalidOffsetNumber;
> 			ptr->parent = top;
> 			ptr->next = NULL;
> 			tail->next = ptr;
> 			tail = ptr;
> 		}

First, notice that we're setting "ptr->parent = top". 'top' is the 
current node we're processing, and ptr represents the node to the right 
of the current node. The current node is *not* the parent of the node to 
the right. I believe that line should be "ptr->parent = top->parent".

Second, we're adding the entry for the right sibling to the end of the 
list of nodes to visit. But when we process entries from the list, we 
exit immediately when we see a leaf page. That means that the right 
sibling can get queued up behind leaf pages, and thus never visited.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com