Refactor GIN's handling of duplicate search entries.

The original coding could combine duplicate entries only when they
originated from the same qual condition.  In particular it could not
combine cases where multiple qual conditions all give rise to full-index
scan requests, which is an expensive case well worth optimizing.  Refactor
so that duplicates are recognized across all the quals.

src/backend/access/gin/ginget.c

@@ -371,6 +371,7 @@ startScanEntry(GinState *ginstate, GinScanEntry entry)
 
 restartScanEntry:
 	entry->buffer = InvalidBuffer;
+	ItemPointerSetMin(&entry->curItem);
 	entry->offset = InvalidOffsetNumber;
 	entry->list = NULL;
 	entry->nlist = 0;
@@ -379,12 +380,6 @@ restartScanEntry:
 	entry->reduceResult = FALSE;
 	entry->predictNumberResult = 0;
 
-	if (entry->master != NULL)
-	{
-		entry->isFinished = entry->master->isFinished;
-		return;
-	}
-
 	/*
 	 * we should find entry, and begin scan of posting tree or just store
 	 * posting list in memory
@@ -499,16 +494,21 @@ restartScanEntry:
 static void
 startScanKey(GinState *ginstate, GinScanKey key)
 {
-	uint32		i;
-
-	if (!key->firstCall)
-		return;
+	ItemPointerSetMin(&key->curItem);
+	key->curItemMatches = false;
+	key->recheckCurItem = false;
+	key->isFinished = false;
+}
 
-	for (i = 0; i < key->nentries; i++)
-		startScanEntry(ginstate, key->scanEntry + i);
+static void
+startScan(IndexScanDesc scan)
+{
+	GinScanOpaque so = (GinScanOpaque) scan->opaque;
+	GinState   *ginstate = &so->ginstate;
+	uint32		i;
 
-	key->isFinished = FALSE;
-	key->firstCall = FALSE;
+	for (i = 0; i < so->totalentries; i++)
+		startScanEntry(ginstate, so->entries[i]);
 
 	if (GinFuzzySearchLimit > 0)
 	{
@@ -519,27 +519,20 @@ startScanKey(GinState *ginstate, GinScanKey key)
 		 * minimal predictNumberResult.
 		 */
 
-		for (i = 0; i < key->nentries; i++)
-			if (key->scanEntry[i].predictNumberResult <= key->nentries * GinFuzzySearchLimit)
+		for (i = 0; i < so->totalentries; i++)
+			if (so->entries[i]->predictNumberResult <= so->totalentries * GinFuzzySearchLimit)
 				return;
 
-		for (i = 0; i < key->nentries; i++)
-			if (key->scanEntry[i].predictNumberResult > key->nentries * GinFuzzySearchLimit)
+		for (i = 0; i < so->totalentries; i++)
+			if (so->entries[i]->predictNumberResult > so->totalentries * GinFuzzySearchLimit)
 			{
-				key->scanEntry[i].predictNumberResult /= key->nentries;
-				key->scanEntry[i].reduceResult = TRUE;
+				so->entries[i]->predictNumberResult /= so->totalentries;
+				so->entries[i]->reduceResult = TRUE;
 			}
 	}
-}
-
-static void
-startScan(IndexScanDesc scan)
-{
-	uint32		i;
-	GinScanOpaque so = (GinScanOpaque) scan->opaque;
 
 	for (i = 0; i < so->nkeys; i++)
-		startScanKey(&so->ginstate, so->keys + i);
+		startScanKey(ginstate, so->keys + i);
 }
 
 /*
@@ -644,12 +637,7 @@ entryGetItem(GinState *ginstate, GinScanEntry entry)
 {
 	Assert(!entry->isFinished);
 
-	if (entry->master)
-	{
-		entry->isFinished = entry->master->isFinished;
-		entry->curItem = entry->master->curItem;
-	}
-	else if (entry->matchBitmap)
+	if (entry->matchBitmap)
 	{
 		do
 		{
@@ -721,13 +709,16 @@ entryGetItem(GinState *ginstate, GinScanEntry entry)
 }
 
 /*
- * Sets key->curItem to next heap item pointer for one scan key, advancing
- * past any item pointers <= advancePast.
- * Sets key->isFinished to TRUE if there are no more.
+ * Identify the "current" item among the input entry streams for this scan key,
+ * and test whether it passes the scan key qual condition.
+ *
+ * The current item is the smallest curItem among the inputs.  key->curItem
+ * is set to that value.  key->curItemMatches is set to indicate whether that
+ * TID passes the consistentFn test.  If so, key->recheckCurItem is set true
+ * iff recheck is needed for this item pointer (including the case where the
+ * item pointer is a lossy page pointer).
  *
- * On success, key->recheckCurItem is set true iff recheck is needed for this
- * item pointer (including the case where the item pointer is a lossy page
- * pointer).
+ * If all entry streams are exhausted, sets key->isFinished to TRUE.
  *
  * Item pointers must be returned in ascending order.
  *
@@ -738,10 +729,9 @@ entryGetItem(GinState *ginstate, GinScanEntry entry)
  * logic in scanGetItem.)
  */
 static void
-keyGetItem(GinState *ginstate, MemoryContext tempCtx,
-		   GinScanKey key, ItemPointer advancePast)
+keyGetItem(GinState *ginstate, MemoryContext tempCtx, GinScanKey key)
 {
-	ItemPointerData myAdvancePast = *advancePast;
+	ItemPointerData minItem;
 	ItemPointerData curPageLossy;
 	uint32		i;
 	uint32		lossyEntry;
@@ -752,167 +742,159 @@ keyGetItem(GinState *ginstate, MemoryContext tempCtx,
 
 	Assert(!key->isFinished);
 
-	do
-	{
-		/*
-		 * Advance any entries that are <= myAdvancePast.  In particular,
-		 * since entry->curItem was initialized with ItemPointerSetMin, this
-		 * ensures we fetch the first item for each entry on the first call.
-		 * Then set key->curItem to the minimum of the valid entry curItems.
-		 *
-		 * Note: a lossy-page entry is encoded by a ItemPointer with max value
-		 * for offset (0xffff), so that it will sort after any exact entries
-		 * for the same page.  So we'll prefer to return exact pointers not
-		 * lossy pointers, which is good.  Also, when we advance past an exact
-		 * entry after processing it, we will not advance past lossy entries
-		 * for the same page in other keys, which is NECESSARY for correct
-		 * results (since we might have additional entries for the same page
-		 * in the first key).
-		 */
-		ItemPointerSetMax(&key->curItem);
-
-		for (i = 0; i < key->nentries; i++)
-		{
-			entry = key->scanEntry + i;
-
-			while (entry->isFinished == FALSE &&
-				   ginCompareItemPointers(&entry->curItem, &myAdvancePast) <= 0)
-				entryGetItem(ginstate, entry);
-
-			if (entry->isFinished == FALSE &&
-				ginCompareItemPointers(&entry->curItem, &key->curItem) < 0)
-				key->curItem = entry->curItem;
-		}
+	/*
+	 * Find the minimum of the active entry curItems.
+	 *
+	 * Note: a lossy-page entry is encoded by a ItemPointer with max value
+	 * for offset (0xffff), so that it will sort after any exact entries
+	 * for the same page.  So we'll prefer to return exact pointers not
+	 * lossy pointers, which is good.
+	 */
+	ItemPointerSetMax(&minItem);
 
-		if (ItemPointerIsMax(&key->curItem))
-		{
-			/* all entries are finished */
-			key->isFinished = TRUE;
-			return;
-		}
+	for (i = 0; i < key->nentries; i++)
+	{
+		entry = key->scanEntry[i];
+		if (entry->isFinished == FALSE &&
+			ginCompareItemPointers(&entry->curItem, &minItem) < 0)
+			minItem = entry->curItem;
+	}
 
-		/*
-		 * Now key->curItem contains first ItemPointer after previous result.
-		 * Advance myAdvancePast to this value, so that if the consistentFn
-		 * rejects the entry and we loop around again, we will advance to the
-		 * next available item pointer.
-		 */
-		myAdvancePast = key->curItem;
+	if (ItemPointerIsMax(&minItem))
+	{
+		/* all entries are finished */
+		key->isFinished = TRUE;
+		return;
+	}
 
-		/*
-		 * Lossy-page entries pose a problem, since we don't know the correct
-		 * entryRes state to pass to the consistentFn, and we also don't know
-		 * what its combining logic will be (could be AND, OR, or even NOT).
-		 * If the logic is OR then the consistentFn might succeed for all
-		 * items in the lossy page even when none of the other entries match.
-		 *
-		 * If we have a single lossy-page entry then we check to see if the
-		 * consistentFn will succeed with only that entry TRUE.  If so,
-		 * we return a lossy-page pointer to indicate that the whole heap
-		 * page must be checked.  (On the next call, we'll advance past all
-		 * regular and lossy entries for this page before resuming search,
-		 * thus ensuring that we never return both regular and lossy pointers
-		 * for the same page.)
-		 *
-		 * This idea could be generalized to more than one lossy-page entry,
-		 * but ideally lossy-page entries should be infrequent so it would
-		 * seldom be the case that we have more than one at once.  So it
-		 * doesn't seem worth the extra complexity to optimize that case.
-		 * If we do find more than one, we just punt and return a lossy-page
-		 * pointer always.
-		 *
-		 * Note that only lossy-page entries pointing to the current item's
-		 * page should trigger this processing; we might have future lossy
-		 * pages in the entry array, but they aren't relevant yet.
-		 */
-		ItemPointerSetLossyPage(&curPageLossy,
-								GinItemPointerGetBlockNumber(&key->curItem));
+	/*
+	 * We might have already tested this item; if so, no need to repeat work.
+	 * (Note: the ">" case can happen, if minItem is exact but we previously
+	 * had to set curItem to a lossy-page pointer.)
+	 */
+	if (ginCompareItemPointers(&key->curItem, &minItem) >= 0)
+		return;
 
-		lossyEntry = 0;
-		haveLossyEntry = false;
-		for (i = 0; i < key->nentries; i++)
-		{
-			entry = key->scanEntry + i;
-			if (entry->isFinished == FALSE &&
-				ginCompareItemPointers(&entry->curItem, &curPageLossy) == 0)
-			{
-				if (haveLossyEntry)
-				{
-					/* Multiple lossy entries, punt */
-					key->curItem = curPageLossy;
-					key->recheckCurItem = true;
-					return;
-				}
-				lossyEntry = i;
-				haveLossyEntry = true;
-			}
-		}
+	/*
+	 * OK, advance key->curItem and perform consistentFn test.
+	 */
+	key->curItem = minItem;
 
-		/* prepare for calling consistentFn in temp context */
-		oldCtx = MemoryContextSwitchTo(tempCtx);
+	/*
+	 * Lossy-page entries pose a problem, since we don't know the correct
+	 * entryRes state to pass to the consistentFn, and we also don't know
+	 * what its combining logic will be (could be AND, OR, or even NOT).
+	 * If the logic is OR then the consistentFn might succeed for all
+	 * items in the lossy page even when none of the other entries match.
+	 *
+	 * If we have a single lossy-page entry then we check to see if the
+	 * consistentFn will succeed with only that entry TRUE.  If so,
+	 * we return a lossy-page pointer to indicate that the whole heap
+	 * page must be checked.  (On subsequent calls, we'll do nothing until
+	 * minItem is past the page altogether, thus ensuring that we never return
+	 * both regular and lossy pointers for the same page.)
+	 *
+	 * This idea could be generalized to more than one lossy-page entry,
+	 * but ideally lossy-page entries should be infrequent so it would
+	 * seldom be the case that we have more than one at once.  So it
+	 * doesn't seem worth the extra complexity to optimize that case.
+	 * If we do find more than one, we just punt and return a lossy-page
+	 * pointer always.
+	 *
+	 * Note that only lossy-page entries pointing to the current item's
+	 * page should trigger this processing; we might have future lossy
+	 * pages in the entry array, but they aren't relevant yet.
+	 */
+	ItemPointerSetLossyPage(&curPageLossy,
+							GinItemPointerGetBlockNumber(&key->curItem));
 
-		if (haveLossyEntry)
+	lossyEntry = 0;
+	haveLossyEntry = false;
+	for (i = 0; i < key->nentries; i++)
+	{
+		entry = key->scanEntry[i];
+		if (entry->isFinished == FALSE &&
+			ginCompareItemPointers(&entry->curItem, &curPageLossy) == 0)
 		{
-			/* Single lossy-page entry, so see if whole page matches */
-			memset(key->entryRes, FALSE, key->nentries);
-			key->entryRes[lossyEntry] = TRUE;
-
-			if (callConsistentFn(ginstate, key))
+			if (haveLossyEntry)
 			{
-				/* Yes, so clean up ... */
-				MemoryContextSwitchTo(oldCtx);
-				MemoryContextReset(tempCtx);
-
-				/* and return lossy pointer for whole page */
+				/* Multiple lossy entries, punt */
 				key->curItem = curPageLossy;
+				key->curItemMatches = true;
 				key->recheckCurItem = true;
 				return;
 			}
+			lossyEntry = i;
+			haveLossyEntry = true;
 		}
+	}
 
-		/*
-		 * At this point we know that we don't need to return a lossy
-		 * whole-page pointer, but we might have matches for individual exact
-		 * item pointers, possibly in combination with a lossy pointer.  Our
-		 * strategy if there's a lossy pointer is to try the consistentFn both
-		 * ways and return a hit if it accepts either one (forcing the hit to
-		 * be marked lossy so it will be rechecked).  An exception is that
-		 * we don't need to try it both ways if the lossy pointer is in a
-		 * "hidden" entry, because the consistentFn's result can't depend on
-		 * that.
-		 *
-		 * Prepare entryRes array to be passed to consistentFn.
-		 */
-		for (i = 0; i < key->nentries; i++)
-		{
-			entry = key->scanEntry + i;
-			if (entry->isFinished == FALSE &&
-				ginCompareItemPointers(&entry->curItem, &key->curItem) == 0)
-				key->entryRes[i] = TRUE;
-			else
-				key->entryRes[i] = FALSE;
-		}
-		if (haveLossyEntry)
-			key->entryRes[lossyEntry] = TRUE;
+	/* prepare for calling consistentFn in temp context */
+	oldCtx = MemoryContextSwitchTo(tempCtx);
 
-		res = callConsistentFn(ginstate, key);
+	if (haveLossyEntry)
+	{
+		/* Single lossy-page entry, so see if whole page matches */
+		memset(key->entryRes, FALSE, key->nentries);
+		key->entryRes[lossyEntry] = TRUE;
 
-		if (!res && haveLossyEntry && lossyEntry < key->nuserentries)
+		if (callConsistentFn(ginstate, key))
 		{
-			/* try the other way for the lossy item */
-			key->entryRes[lossyEntry] = FALSE;
+			/* Yes, so clean up ... */
+			MemoryContextSwitchTo(oldCtx);
+			MemoryContextReset(tempCtx);
 
-			res = callConsistentFn(ginstate, key);
+			/* and return lossy pointer for whole page */
+			key->curItem = curPageLossy;
+			key->curItemMatches = true;
+			key->recheckCurItem = true;
+			return;
 		}
+	}
 
-		/* clean up after consistentFn calls */
-		MemoryContextSwitchTo(oldCtx);
-		MemoryContextReset(tempCtx);
+	/*
+	 * At this point we know that we don't need to return a lossy
+	 * whole-page pointer, but we might have matches for individual exact
+	 * item pointers, possibly in combination with a lossy pointer.  Our
+	 * strategy if there's a lossy pointer is to try the consistentFn both
+	 * ways and return a hit if it accepts either one (forcing the hit to
+	 * be marked lossy so it will be rechecked).  An exception is that
+	 * we don't need to try it both ways if the lossy pointer is in a
+	 * "hidden" entry, because the consistentFn's result can't depend on
+	 * that.
+	 *
+	 * Prepare entryRes array to be passed to consistentFn.
+	 */
+	for (i = 0; i < key->nentries; i++)
+	{
+		entry = key->scanEntry[i];
+		if (entry->isFinished == FALSE &&
+			ginCompareItemPointers(&entry->curItem, &key->curItem) == 0)
+			key->entryRes[i] = TRUE;
+		else
+			key->entryRes[i] = FALSE;
+	}
+	if (haveLossyEntry)
+		key->entryRes[lossyEntry] = TRUE;
 
-		/* If we matched a lossy entry, force recheckCurItem = true */
-		if (haveLossyEntry)
-			key->recheckCurItem = true;
-	} while (!res);
+	res = callConsistentFn(ginstate, key);
+
+	if (!res && haveLossyEntry && lossyEntry < key->nuserentries)
+	{
+		/* try the other way for the lossy item */
+		key->entryRes[lossyEntry] = FALSE;
+
+		res = callConsistentFn(ginstate, key);
+	}
+
+	key->curItemMatches = res;
+	/* If we matched a lossy entry, force recheckCurItem = true */
+	if (haveLossyEntry)
+		key->recheckCurItem = true;
+
+	/* clean up after consistentFn calls */
+	MemoryContextSwitchTo(oldCtx);
+	MemoryContextReset(tempCtx);
 }
 
 /*
@@ -929,26 +911,45 @@ scanGetItem(IndexScanDesc scan, ItemPointer advancePast,
 			ItemPointerData *item, bool *recheck)
 {
 	GinScanOpaque so = (GinScanOpaque) scan->opaque;
+	GinState   *ginstate = &so->ginstate;
 	ItemPointerData myAdvancePast = *advancePast;
 	uint32		i;
+	bool		allFinished;
 	bool		match;
 
 	for (;;)
 	{
 		/*
-		 * Advance any keys that are <= myAdvancePast.  In particular,
-		 * since key->curItem was initialized with ItemPointerSetMin, this
-		 * ensures we fetch the first item for each key on the first call.
-		 * Then set *item to the minimum of the key curItems.
-		 *
-		 * Note: a lossy-page entry is encoded by a ItemPointer with max value
-		 * for offset (0xffff), so that it will sort after any exact entries
-		 * for the same page.  So we'll prefer to return exact pointers not
-		 * lossy pointers, which is good.  Also, when we advance past an exact
-		 * entry after processing it, we will not advance past lossy entries
-		 * for the same page in other keys, which is NECESSARY for correct
-		 * results (since we might have additional entries for the same page
-		 * in the first key).
+		 * Advance any entries that are <= myAdvancePast.  In particular,
+		 * since entry->curItem was initialized with ItemPointerSetMin, this
+		 * ensures we fetch the first item for each entry on the first call.
+		 */
+		allFinished = TRUE;
+
+		for (i = 0; i < so->totalentries; i++)
+		{
+			GinScanEntry entry = so->entries[i];
+
+			while (entry->isFinished == FALSE &&
+				   ginCompareItemPointers(&entry->curItem,
+										  &myAdvancePast) <= 0)
+				entryGetItem(ginstate, entry);
+
+			if (entry->isFinished == FALSE)
+				allFinished = FALSE;
+		}
+
+		if (allFinished)
+		{
+			/* all entries exhausted, so we're done */
+			return false;
+		}
+
+		/*
+		 * Perform the consistentFn test for each scan key.  If any key
+		 * reports isFinished, meaning its subset of the entries is exhausted,
+		 * we can stop.  Otherwise, set *item to the minimum of the key
+		 * curItems.
 		 */
 		ItemPointerSetMax(item);
 
@@ -956,13 +957,10 @@ scanGetItem(IndexScanDesc scan, ItemPointer advancePast,
 		{
 			GinScanKey	key = so->keys + i;
 
-			while (key->isFinished == FALSE &&
-				   ginCompareItemPointers(&key->curItem, &myAdvancePast) <= 0)
-				keyGetItem(&so->ginstate, so->tempCtx,
-						   key, &myAdvancePast);
+			keyGetItem(&so->ginstate, so->tempCtx, key);
 
 			if (key->isFinished)
-					return FALSE;		/* finished one of keys */
+				return false;		/* finished one of keys */
 
 			if (ginCompareItemPointers(&key->curItem, item) < 0)
 				*item = key->curItem;
@@ -973,7 +971,7 @@ scanGetItem(IndexScanDesc scan, ItemPointer advancePast,
 		/*----------
 		 * Now *item contains first ItemPointer after previous result.
 		 *
-		 * The item is a valid hit only if all the keys returned either
+		 * The item is a valid hit only if all the keys succeeded for either
 		 * that exact TID, or a lossy reference to the same page.
 		 *
 		 * This logic works only if a keyGetItem stream can never contain both
@@ -996,12 +994,15 @@ scanGetItem(IndexScanDesc scan, ItemPointer advancePast,
 		{
 			GinScanKey	key = so->keys + i;
 
-			if (ginCompareItemPointers(item, &key->curItem) == 0)
-				continue;
-			if (ItemPointerIsLossyPage(&key->curItem) &&
-				GinItemPointerGetBlockNumber(&key->curItem) ==
-				GinItemPointerGetBlockNumber(item))
-				continue;
+			if (key->curItemMatches)
+			{
+				if (ginCompareItemPointers(item, &key->curItem) == 0)
+					continue;
+				if (ItemPointerIsLossyPage(&key->curItem) &&
+					GinItemPointerGetBlockNumber(&key->curItem) ==
+					GinItemPointerGetBlockNumber(item))
+					continue;
+			}
 			match = false;
 			break;
 		}
@@ -1247,7 +1248,7 @@ collectMatchesForHeapRow(IndexScanDesc scan, pendingPosition *pos)
 
 			for (j = 0; j < key->nentries; j++)
 			{
-				GinScanEntry entry = key->scanEntry + j;
+				GinScanEntry entry = key->scanEntry[j];
 				OffsetNumber StopLow = pos->firstOffset,
 							StopHigh = pos->lastOffset,
 							StopMiddle;

src/backend/access/gin/ginscan.c

@@ -53,19 +53,95 @@ ginbeginscan(PG_FUNCTION_ARGS)
 }
 
 /*
- * Initialize a GinScanKey using the output from the extractQueryFn
+ * Create a new GinScanEntry, unless an equivalent one already exists,
+ * in which case just return it
+ */
+static GinScanEntry
+ginFillScanEntry(GinScanOpaque so, OffsetNumber attnum,
+				 StrategyNumber strategy, int32 searchMode,
+				 Datum queryKey, GinNullCategory queryCategory,
+				 bool isPartialMatch, Pointer extra_data)
+{
+	GinState   *ginstate = &so->ginstate;
+	GinScanEntry scanEntry;
+	uint32		i;
+
+	/*
+	 * Look for an existing equivalent entry.
+	 *
+	 * Entries with non-null extra_data are never considered identical, since
+	 * we can't know exactly what the opclass might be doing with that.
+	 */
+	if (extra_data == NULL)
+	{
+		for (i = 0; i < so->totalentries; i++)
+		{
+			GinScanEntry prevEntry = so->entries[i];
+
+			if (prevEntry->extra_data == NULL &&
+				prevEntry->isPartialMatch == isPartialMatch &&
+				prevEntry->strategy == strategy &&
+				prevEntry->searchMode == searchMode &&
+				prevEntry->attnum == attnum &&
+				ginCompareEntries(ginstate, attnum,
+								  prevEntry->queryKey,
+								  prevEntry->queryCategory,
+								  queryKey,
+								  queryCategory) == 0)
+			{
+				/* Successful match */
+				return prevEntry;
+			}
+		}
+	}
+
+	/* Nope, create a new entry */
+	scanEntry = (GinScanEntry) palloc(sizeof(GinScanEntryData));
+	scanEntry->queryKey = queryKey;
+	scanEntry->queryCategory = queryCategory;
+	scanEntry->isPartialMatch = isPartialMatch;
+	scanEntry->extra_data = extra_data;
+	scanEntry->strategy = strategy;
+	scanEntry->searchMode = searchMode;
+	scanEntry->attnum = attnum;
+
+	scanEntry->buffer = InvalidBuffer;
+	ItemPointerSetMin(&scanEntry->curItem);
+	scanEntry->matchBitmap = NULL;
+	scanEntry->matchIterator = NULL;
+	scanEntry->matchResult = NULL;
+	scanEntry->list = NULL;
+	scanEntry->nlist = 0;
+	scanEntry->offset = InvalidOffsetNumber;
+	scanEntry->isFinished = false;
+	scanEntry->reduceResult = false;
+
+	/* Add it to so's array */
+	if (so->totalentries >= so->allocentries)
+	{
+		so->allocentries *= 2;
+		so->entries = (GinScanEntry *)
+			repalloc(so->entries, so->allocentries * sizeof(GinScanEntry));
+	}
+	so->entries[so->totalentries++] = scanEntry;
+
+	return scanEntry;
+}
+
+/*
+ * Initialize the next GinScanKey using the output from the extractQueryFn
  */
 static void
-ginFillScanKey(GinState *ginstate, GinScanKey key,
-			   OffsetNumber attnum, Datum query,
+ginFillScanKey(GinScanOpaque so, OffsetNumber attnum,
+			   StrategyNumber strategy, int32 searchMode,
+			   Datum query, uint32 nQueryValues,
 			   Datum *queryValues, GinNullCategory *queryCategories,
-			   bool *partial_matches, uint32 nQueryValues,
-			   StrategyNumber strategy, Pointer *extra_data,
-			   int32 searchMode)
+			   bool *partial_matches, Pointer *extra_data)
 {
+	GinScanKey	key = &(so->keys[so->nkeys++]);
+	GinState   *ginstate = &so->ginstate;
 	uint32		nUserQueryValues = nQueryValues;
-	uint32		i,
-				j;
+	uint32		i;
 
 	/* Non-default search modes add one "hidden" entry to each key */
 	if (searchMode != GIN_SEARCH_MODE_DEFAULT)
@@ -73,8 +149,9 @@ ginFillScanKey(GinState *ginstate, GinScanKey key,
 	key->nentries = nQueryValues;
 	key->nuserentries = nUserQueryValues;
 
-	key->scanEntry = (GinScanEntry) palloc(sizeof(GinScanEntryData) * nQueryValues);
+	key->scanEntry = (GinScanEntry *) palloc(sizeof(GinScanEntry) * nQueryValues);
 	key->entryRes = (bool *) palloc0(sizeof(bool) * nQueryValues);
+
 	key->query = query;
 	key->queryValues = queryValues;
 	key->queryCategories = queryCategories;
@@ -83,156 +160,106 @@ ginFillScanKey(GinState *ginstate, GinScanKey key,
 	key->searchMode = searchMode;
 	key->attnum = attnum;
 
-	key->firstCall = TRUE;
 	ItemPointerSetMin(&key->curItem);
+	key->curItemMatches = false;
+	key->recheckCurItem = false;
+	key->isFinished = false;
 
 	for (i = 0; i < nQueryValues; i++)
 	{
-		GinScanEntry scanEntry = key->scanEntry + i;
+		Datum		queryKey;
+		GinNullCategory queryCategory;
+		bool		isPartialMatch;
+		Pointer		this_extra;
 
-		scanEntry->pval = key->entryRes + i;
 		if (i < nUserQueryValues)
 		{
-			scanEntry->queryKey = queryValues[i];
-			scanEntry->queryCategory = queryCategories[i];
-			scanEntry->isPartialMatch =
+			/* set up normal entry using extractQueryFn's outputs */
+			queryKey = queryValues[i];
+			queryCategory = queryCategories[i];
+			isPartialMatch =
 				(ginstate->canPartialMatch[attnum - 1] && partial_matches)
 				? partial_matches[i] : false;
-			scanEntry->extra_data = (extra_data) ? extra_data[i] : NULL;
+			this_extra = (extra_data) ? extra_data[i] : NULL;
 		}
 		else
 		{
 			/* set up hidden entry */
-			scanEntry->queryKey = (Datum) 0;
+			queryKey = (Datum) 0;
 			switch (searchMode)
 			{
 				case GIN_SEARCH_MODE_INCLUDE_EMPTY:
-					scanEntry->queryCategory = GIN_CAT_EMPTY_ITEM;
+					queryCategory = GIN_CAT_EMPTY_ITEM;
 					break;
 				case GIN_SEARCH_MODE_ALL:
-					scanEntry->queryCategory = GIN_CAT_EMPTY_QUERY;
+					queryCategory = GIN_CAT_EMPTY_QUERY;
 					break;
 				case GIN_SEARCH_MODE_EVERYTHING:
-					scanEntry->queryCategory = GIN_CAT_EMPTY_QUERY;
+					queryCategory = GIN_CAT_EMPTY_QUERY;
 					break;
 				default:
 					elog(ERROR, "unexpected searchMode: %d", searchMode);
+					queryCategory = 0;		/* keep compiler quiet */
 					break;
 			}
-			scanEntry->isPartialMatch = false;
-			scanEntry->extra_data = NULL;
+			isPartialMatch = false;
+			this_extra = NULL;
+
+			/*
+			 * We set the strategy to a fixed value so that ginFillScanEntry
+			 * can combine these entries for different scan keys.  This is
+			 * safe because the strategy value in the entry struct is only
+			 * used for partial-match cases.  It's OK to overwrite our local
+			 * variable here because this is the last loop iteration.
+			 */
+			strategy = InvalidStrategy;
 		}
-		scanEntry->strategy = strategy;
-		scanEntry->searchMode = searchMode;
-		scanEntry->attnum = attnum;
-
-		ItemPointerSetMin(&scanEntry->curItem);
-		scanEntry->isFinished = FALSE;
-		scanEntry->offset = InvalidOffsetNumber;
-		scanEntry->buffer = InvalidBuffer;
-		scanEntry->list = NULL;
-		scanEntry->nlist = 0;
-		scanEntry->matchBitmap = NULL;
-		scanEntry->matchIterator = NULL;
-		scanEntry->matchResult = NULL;
 
-		/*
-		 * Link to any preceding identical entry in current scan key.
-		 *
-		 * Entries with non-null extra_data are never considered identical,
-		 * since we can't know exactly what the opclass might be doing with
-		 * that.
-		 */
-		scanEntry->master = NULL;
-		if (scanEntry->extra_data == NULL)
-		{
-			for (j = 0; j < i; j++)
-			{
-				GinScanEntry prevEntry = key->scanEntry + j;
-
-				if (prevEntry->extra_data == NULL &&
-					scanEntry->isPartialMatch == prevEntry->isPartialMatch &&
-					ginCompareEntries(ginstate, attnum,
-									  scanEntry->queryKey,
-									  scanEntry->queryCategory,
-									  prevEntry->queryKey,
-									  prevEntry->queryCategory) == 0)
-				{
-					scanEntry->master = prevEntry;
-					break;
-				}
-			}
-		}
+		key->scanEntry[i] = ginFillScanEntry(so, attnum,
+											 strategy, searchMode,
+											 queryKey, queryCategory,
+											 isPartialMatch, this_extra);
 	}
 }
 
-#ifdef NOT_USED
-
 static void
-resetScanKeys(GinScanKey keys, uint32 nkeys)
+freeScanKeys(GinScanOpaque so)
 {
-	uint32		i,
-				j;
+	uint32		i;
 
-	if (keys == NULL)
+	if (so->keys == NULL)
 		return;
 
-	for (i = 0; i < nkeys; i++)
+	for (i = 0; i < so->nkeys; i++)
 	{
-		GinScanKey	key = keys + i;
-
-		key->firstCall = TRUE;
-		ItemPointerSetMin(&key->curItem);
+		GinScanKey	key = so->keys + i;
 
-		for (j = 0; j < key->nentries; j++)
-		{
-			if (key->scanEntry[j].buffer != InvalidBuffer)
-				ReleaseBuffer(key->scanEntry[i].buffer);
-
-			ItemPointerSetMin(&key->scanEntry[j].curItem);
-			key->scanEntry[j].isFinished = FALSE;
-			key->scanEntry[j].offset = InvalidOffsetNumber;
-			key->scanEntry[j].buffer = InvalidBuffer;
-			key->scanEntry[j].list = NULL;
-			key->scanEntry[j].nlist = 0;
-			key->scanEntry[j].matchBitmap = NULL;
-			key->scanEntry[j].matchIterator = NULL;
-			key->scanEntry[j].matchResult = NULL;
-		}
+		pfree(key->scanEntry);
+		pfree(key->entryRes);
 	}
-}
-#endif
-
-static void
-freeScanKeys(GinScanKey keys, uint32 nkeys)
-{
-	uint32		i,
-				j;
 
-	if (keys == NULL)
-		return;
+	pfree(so->keys);
+	so->keys = NULL;
+	so->nkeys = 0;
 
-	for (i = 0; i < nkeys; i++)
+	for (i = 0; i < so->totalentries; i++)
 	{
-		GinScanKey	key = keys + i;
-
-		for (j = 0; j < key->nentries; j++)
-		{
-			if (key->scanEntry[j].buffer != InvalidBuffer)
-				ReleaseBuffer(key->scanEntry[j].buffer);
-			if (key->scanEntry[j].list)
-				pfree(key->scanEntry[j].list);
-			if (key->scanEntry[j].matchIterator)
-				tbm_end_iterate(key->scanEntry[j].matchIterator);
-			if (key->scanEntry[j].matchBitmap)
-				tbm_free(key->scanEntry[j].matchBitmap);
-		}
-
-		pfree(key->entryRes);
-		pfree(key->scanEntry);
+		GinScanEntry entry = so->entries[i];
+
+		if (entry->buffer != InvalidBuffer)
+			ReleaseBuffer(entry->buffer);
+		if (entry->list)
+			pfree(entry->list);
+		if (entry->matchIterator)
+			tbm_end_iterate(entry->matchIterator);
+		if (entry->matchBitmap)
+			tbm_free(entry->matchBitmap);
+		pfree(entry);
 	}
 
-	pfree(keys);
+	pfree(so->entries);
+	so->entries = NULL;
+	so->totalentries = 0;
 }
 
 void
@@ -241,12 +268,18 @@ ginNewScanKey(IndexScanDesc scan)
 	ScanKey		scankey = scan->keyData;
 	GinScanOpaque so = (GinScanOpaque) scan->opaque;
 	int			i;
-	uint32		nkeys = 0;
 	bool		hasNullQuery = false;
 
 	/* if no scan keys provided, allocate extra EVERYTHING GinScanKey */
 	so->keys = (GinScanKey)
 		palloc(Max(scan->numberOfKeys, 1) * sizeof(GinScanKeyData));
+	so->nkeys = 0;
+
+	/* initialize expansible array of GinScanEntry pointers */
+	so->totalentries = 0;
+	so->allocentries = 32;
+	so->entries = (GinScanEntry *)
+		palloc0(so->allocentries * sizeof(GinScanEntry));
 
 	so->isVoidRes = false;
 
@@ -331,26 +364,24 @@ ginNewScanKey(IndexScanDesc scan)
 		}
 		/* now we can use the nullFlags as category codes */
 
-		ginFillScanKey(&so->ginstate, &(so->keys[nkeys]),
-					   skey->sk_attno, skey->sk_argument,
+		ginFillScanKey(so, skey->sk_attno,
+					   skey->sk_strategy, searchMode,
+					   skey->sk_argument, nQueryValues,
 					   queryValues, (GinNullCategory *) nullFlags,
-					   partial_matches, nQueryValues,
-					   skey->sk_strategy, extra_data, searchMode);
-		nkeys++;
+					   partial_matches, extra_data);
 	}
 
 	/*
 	 * If there are no regular scan keys, generate an EVERYTHING scankey to
 	 * drive a full-index scan.
 	 */
-	if (nkeys == 0 && !so->isVoidRes)
+	if (so->nkeys == 0 && !so->isVoidRes)
 	{
 		hasNullQuery = true;
-		ginFillScanKey(&so->ginstate, &(so->keys[nkeys]),
-					   FirstOffsetNumber, (Datum) 0,
-					   NULL, NULL, NULL, 0,
-					   InvalidStrategy, NULL, GIN_SEARCH_MODE_EVERYTHING);
-		nkeys++;
+		ginFillScanKey(so, FirstOffsetNumber,
+					   InvalidStrategy, GIN_SEARCH_MODE_EVERYTHING,
+					   (Datum) 0, 0,
+					   NULL, NULL, NULL, NULL);
 	}
 
 	/*
@@ -371,8 +402,6 @@ ginNewScanKey(IndexScanDesc scan)
 							 RelationGetRelationName(scan->indexRelation))));
 	}
 
-	so->nkeys = nkeys;
-
 	pgstat_count_index_scan(scan->indexRelation);
 }
 
@@ -384,8 +413,7 @@ ginrescan(PG_FUNCTION_ARGS)
 	/* remaining arguments are ignored */
 	GinScanOpaque so = (GinScanOpaque) scan->opaque;
 
-	freeScanKeys(so->keys, so->nkeys);
-	so->keys = NULL;
+	freeScanKeys(so);
 
 	if (scankey && scan->numberOfKeys > 0)
 	{
@@ -403,7 +431,7 @@ ginendscan(PG_FUNCTION_ARGS)
 	IndexScanDesc scan = (IndexScanDesc) PG_GETARG_POINTER(0);
 	GinScanOpaque so = (GinScanOpaque) scan->opaque;
 
-	freeScanKeys(so->keys, so->nkeys);
+	freeScanKeys(so);
 
 	MemoryContextDelete(so->tempCtx);
 

src/include/access/gin_private.h

@@ -549,12 +549,19 @@ extern void ginPrepareDataScan(GinBtree btree, Relation index);
 /* ginscan.c */
 
 /*
- * GinScanKeyData describes a single GIN index qualification condition.
- * It contains one GinScanEntryData for each key datum extracted from
- * the qual by the extractQueryFn or added implicitly by ginFillScanKey.
- * nentries is the true number of entries, nuserentries is the number
- * that extractQueryFn returned (which is what we report to consistentFn).
- * The "user" entries must come first.
+ * GinScanKeyData describes a single GIN index qualifier expression.
+ *
+ * From each qual expression, we extract one or more specific index search
+ * conditions, which are represented by GinScanEntryData.  It's quite
+ * possible for identical search conditions to be requested by more than
+ * one qual expression, in which case we merge such conditions to have just
+ * one unique GinScanEntry --- this is particularly important for efficiency
+ * when dealing with full-index-scan entries.  So there can be multiple
+ * GinScanKeyData.scanEntry pointers to the same GinScanEntryData.
+ *
+ * In each GinScanKeyData, nentries is the true number of entries, while
+ * nuserentries is the number that extractQueryFn returned (which is what
+ * we report to consistentFn).  The "user" entries must come first.
  */
 typedef struct GinScanKeyData *GinScanKey;
 
@@ -567,10 +574,10 @@ typedef struct GinScanKeyData
 	/* Number of entries that extractQueryFn and consistentFn know about */
 	uint32		nuserentries;
 
-	/* array of GinScanEntryData, one per key datum */
-	GinScanEntry scanEntry;
+	/* array of GinScanEntry pointers, one per extracted search condition */
+	GinScanEntry *scanEntry;
 
-	/* array of ItemPointer result, reported to consistentFn */
+	/* array of check flags, reported to consistentFn */
 	bool	   *entryRes;
 
 	/* other data needed for calling consistentFn */
@@ -583,22 +590,21 @@ typedef struct GinScanKeyData
 	int32		searchMode;
 	OffsetNumber attnum;
 
-	/* scan status data */
+	/*
+	 * Match status data.  curItem is the TID most recently tested (could be
+	 * a lossy-page pointer).  curItemMatches is TRUE if it passes the
+	 * consistentFn test; if so, recheckCurItem is the recheck flag.
+	 * isFinished means that all the input entry streams are finished, so
+	 * this key cannot succeed for any later TIDs.
+	 */
 	ItemPointerData curItem;
+	bool		curItemMatches;
 	bool		recheckCurItem;
-
-	bool		firstCall;
 	bool		isFinished;
 } GinScanKeyData;
 
 typedef struct GinScanEntryData
 {
-	/* link to any preceding identical entry in current scan key */
-	GinScanEntry master;
-
-	/* ptr to value reported to consistentFn, points to parent->entryRes[i] */
-	bool	   *pval;
-
 	/* query key and other information from extractQueryFn */
 	Datum		queryKey;
 	GinNullCategory queryCategory;
@@ -634,10 +640,14 @@ typedef struct GinScanOpaqueData
 	MemoryContext tempCtx;
 	GinState	ginstate;
 
-	GinScanKey	keys;
+	GinScanKey	keys;			/* one per scan qualifier expr */
 	uint32		nkeys;
-	bool		isVoidRes;		/* true if ginstate.extractQueryFn guarantees
-								 * that nothing will be found */
+
+	GinScanEntry *entries;		/* one per index search condition */
+	uint32		totalentries;
+	uint32		allocentries;	/* allocated length of entries[] */
+
+	bool		isVoidRes;		/* true if query is unsatisfiable */
 } GinScanOpaqueData;
 
 typedef GinScanOpaqueData *GinScanOpaque;