From 614167c6d7e98d4538c4546754b3c2dba480f71c Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Thu, 10 Apr 2014 23:42:04 +0300
Subject: [PATCH] Fix bugs in GIN "fast scan" with partial match.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There were a couple of bugs here. First, if the fuzzy limit was exceeded,
the loop in entryGetItem might drop out too soon if a whole block needs to
be skipped because it's < advancePast ("continue" in a while-loop checks the
loop condition too). Secondly, the loop checked when stepping to a new page
that there is at least one offset on the page < advancePast, but we cannot
rely on that on subsequent calls of entryGetItem, because advancePast might
change in between. That caused the skipping loop to read bogus items in the
TbmIterateResult's offset array.

First item and fix by Alexander Korotkov, second bug pointed out by Fabrízio
de Royes Mello, by a small variation of Alexander's test query.
---
 src/backend/access/gin/ginget.c | 55 +++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index dfc3dc48c80..fda19cf4e69 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -729,11 +729,18 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
 		/* A bitmap result */
 		BlockNumber advancePastBlk = GinItemPointerGetBlockNumber(&advancePast);
 		OffsetNumber advancePastOff = GinItemPointerGetOffsetNumber(&advancePast);
+		bool		gotitem = false;
 
 		do
 		{
-			if (entry->matchResult == NULL ||
-				entry->offset >= entry->matchResult->ntuples)
+			/*
+			 * If we've exhausted all items on this block, move to next block
+			 * in the bitmap.
+			 */
+			while (entry->matchResult == NULL ||
+				   (entry->matchResult->ntuples >= 0 &&
+					entry->offset >= entry->matchResult->ntuples) ||
+				   entry->matchResult->blockno < advancePastBlk)
 			{
 				entry->matchResult = tbm_iterate(entry->matchIterator);
 
@@ -746,18 +753,6 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
 					break;
 				}
 
-				/*
-				 * If all the matches on this page are <= advancePast, skip
-				 * to next page.
-				 */
-				if (entry->matchResult->blockno < advancePastBlk ||
-					(entry->matchResult->blockno == advancePastBlk &&
-					 entry->matchResult->offsets[entry->offset] <= advancePastOff))
-				{
-					entry->offset = entry->matchResult->ntuples;
-					continue;
-				}
-
 				/*
 				 * Reset counter to the beginning of entry->matchResult. Note:
 				 * entry->offset is still greater than matchResult->ntuples if
@@ -766,30 +761,43 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
 				 */
 				entry->offset = 0;
 			}
+			if (entry->isFinished)
+				break;
 
+			/*
+			 * We're now on the first page after advancePast which has any
+			 * items on it. If it's a lossy result, return that.
+			 */
 			if (entry->matchResult->ntuples < 0)
 			{
-				/*
-				 * lossy result, so we need to check the whole page
-				 */
 				ItemPointerSetLossyPage(&entry->curItem,
 										entry->matchResult->blockno);
 
 				/*
 				 * We might as well fall out of the loop; we could not
 				 * estimate number of results on this page to support correct
-				 * reducing of result even if it's enabled
+				 * reducing of result even if it's enabled.
 				 */
+				gotitem = true;
 				break;
 			}
-
+			/*
+			 * Not a lossy page. Skip over any offsets <= advancePast, and
+			 * return that.
+			 */
 			if (entry->matchResult->blockno == advancePastBlk)
 			{
 				/*
-				 * Skip to the right offset on this page. We already checked
-				 * in above loop that there is at least one item > advancePast
-				 * on the page.
+				 * First, do a quick check against the last offset on the page.
+				 * If that's > advancePast, so are all the other offsets.
 				 */
+				if (entry->matchResult->offsets[entry->matchResult->ntuples - 1] <= advancePastOff)
+				{
+					entry->offset = entry->matchResult->ntuples;
+					continue;
+				}
+
+				/* Otherwise scan to find the first item > advancePast */
 				while (entry->matchResult->offsets[entry->offset] <= advancePastOff)
 					entry->offset++;
 			}
@@ -798,7 +806,8 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
 						   entry->matchResult->blockno,
 						   entry->matchResult->offsets[entry->offset]);
 			entry->offset++;
-		} while (entry->reduceResult == TRUE && dropItem(entry));
+			gotitem = true;
+		} while (!gotitem || (entry->reduceResult == TRUE && dropItem(entry)));
 	}
 	else if (!BufferIsValid(entry->buffer))
 	{