From 41b1bd04f63e1f546e08b022223a1425fcf2b5b4 Mon Sep 17 00:00:00 2001
From: Nikita Glukhov <n.gluhov@postgrespro.ru>
Date: Sat, 3 Aug 2019 01:59:57 +0300
Subject: [PATCH 4/5] Avoid GIN full scan for non-empty ALL keys

---
 src/backend/access/gin/ginget.c       | 125 ++++++++++++++++---------
 src/backend/access/gin/ginscan.c      | 168 +++++++++++++++++++++++++++-------
 src/backend/utils/adt/selfuncs.c      |  46 ++++------
 src/include/access/gin_private.h      |   6 ++
 src/test/regress/expected/tsearch.out |  23 +++++
 src/test/regress/sql/tsearch.sql      |   7 ++
 6 files changed, 267 insertions(+), 108 deletions(-)

diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 65ed8b2..a7f5c56 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -1231,11 +1231,12 @@ keyGetItem(GinState *ginstate, MemoryContext tempCtx, GinScanKey key,
  */
 static bool
 scanGetItem(IndexScanDesc scan, ItemPointerData advancePast,
-			ItemPointerData *item, bool *recheck)
+			ItemPointerData *item, bool *recheckp)
 {
 	GinScanOpaque so = (GinScanOpaque) scan->opaque;
 	uint32		i;
 	bool		match;
+	bool		recheck;
 
 	/*----------
 	 * Advance the scan keys in lock-step, until we find an item that matches
@@ -1262,6 +1263,8 @@ scanGetItem(IndexScanDesc scan, ItemPointerData advancePast,
 	{
 		ItemPointerSetMin(item);
 		match = true;
+		recheck = false;
+
 		for (i = 0; i < so->nkeys && match; i++)
 		{
 			GinScanKey	key = so->keys + i;
@@ -1270,44 +1273,55 @@ scanGetItem(IndexScanDesc scan, ItemPointerData advancePast,
 			keyGetItem(&so->ginstate, so->tempCtx, key, advancePast,
 					   scan->xs_snapshot);
 
-			if (key->isFinished)
-				return false;
-
-			/*
-			 * If it's not a match, we can immediately conclude that nothing
-			 * <= this item matches, without checking the rest of the keys.
-			 */
-			if (!key->curItemMatches)
+			if (!key->includeNonMatching)	/* postpone curItem checks */
 			{
-				advancePast = key->curItem;
-				match = false;
-				break;
-			}
+				if (key->isFinished)
+					return false;
 
-			/*
-			 * It's a match. We can conclude that nothing < matches, so the
-			 * other key streams can skip to this item.
-			 *
-			 * Beware of lossy pointers, though; from a lossy pointer, we can
-			 * only conclude that nothing smaller than this *block* matches.
-			 */
-			if (ItemPointerIsLossyPage(&key->curItem))
-			{
-				if (GinItemPointerGetBlockNumber(&advancePast) <
-					GinItemPointerGetBlockNumber(&key->curItem))
+				/*
+				 * If it's not a match, we can immediately conclude that
+				 * nothing <= this item matches, without checking the rest of
+				 * the keys.
+				 */
+				if (!key->curItemMatches)
+				{
+					advancePast = key->curItem;
+					match = false;
+					break;
+				}
+
+				/*
+				 * We must return recheck = true if any of the keys are marked
+				 * recheck.
+				 */
+				recheck |= key->recheckCurItem;
+
+				/*
+				 * It's a match. We can conclude that nothing < matches, so the
+				 * other key streams can skip to this item.
+				 *
+				 * Beware of lossy pointers, though; from a lossy pointer, we
+				 * can only conclude that nothing smaller than this *block*
+				 * matches.
+				 */
+				if (ItemPointerIsLossyPage(&key->curItem))
+				{
+					if (GinItemPointerGetBlockNumber(&advancePast) <
+						GinItemPointerGetBlockNumber(&key->curItem))
+					{
+						ItemPointerSet(&advancePast,
+									   GinItemPointerGetBlockNumber(&key->curItem),
+									   InvalidOffsetNumber);
+					}
+				}
+				else
 				{
+					Assert(GinItemPointerGetOffsetNumber(&key->curItem) > 0);
 					ItemPointerSet(&advancePast,
 								   GinItemPointerGetBlockNumber(&key->curItem),
-								   InvalidOffsetNumber);
+								   OffsetNumberPrev(GinItemPointerGetOffsetNumber(&key->curItem)));
 				}
 			}
-			else
-			{
-				Assert(GinItemPointerGetOffsetNumber(&key->curItem) > 0);
-				ItemPointerSet(&advancePast,
-							   GinItemPointerGetBlockNumber(&key->curItem),
-							   OffsetNumberPrev(GinItemPointerGetOffsetNumber(&key->curItem)));
-			}
 
 			/*
 			 * If this is the first key, remember this location as a potential
@@ -1322,9 +1336,18 @@ scanGetItem(IndexScanDesc scan, ItemPointerData advancePast,
 			if (i == 0)
 			{
 				*item = key->curItem;
+				/* The first key must be have includeNonMatching == false. */
+				Assert(key->includeNonMatching);
 			}
 			else
 			{
+				if (key->includeNonMatching && key->isFinished)
+				{
+					/* Accept all non-matching items after key was finished. */
+					recheck |= key->recheckNonMatching;
+					continue;
+				}
+
 				if (ItemPointerIsLossyPage(&key->curItem) ||
 					ItemPointerIsLossyPage(item))
 				{
@@ -1337,6 +1360,30 @@ scanGetItem(IndexScanDesc scan, ItemPointerData advancePast,
 					Assert(ginCompareItemPointers(&key->curItem, item) >= 0);
 					match = (ginCompareItemPointers(&key->curItem, item) == 0);
 				}
+
+				if (key->includeNonMatching)
+				{
+					/*
+					 * If the current item matches, then apply ordinary rules,
+					 * else force recheck if needed.
+					 */
+					if (match)
+					{
+						if (!key->curItemMatches)
+						{
+							advancePast = key->curItem;
+							match = false;
+							break;
+						}
+
+						recheck |= key->recheckCurItem;
+					}
+					else
+					{
+						match = true;
+						recheck |= key->recheckNonMatching;
+					}
+				}
 			}
 		}
 	} while (!match);
@@ -1347,20 +1394,8 @@ scanGetItem(IndexScanDesc scan, ItemPointerData advancePast,
 	 * Now *item contains the first ItemPointer after previous result that
 	 * satisfied all the keys for that exact TID, or a lossy reference to the
 	 * same page.
-	 *
-	 * We must return recheck = true if any of the keys are marked recheck.
 	 */
-	*recheck = false;
-	for (i = 0; i < so->nkeys; i++)
-	{
-		GinScanKey	key = so->keys + i;
-
-		if (key->recheckCurItem)
-		{
-			*recheck = true;
-			break;
-		}
-	}
+	*recheckp = recheck;
 
 	return true;
 }
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index 47beef3..df90b18 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -137,19 +137,29 @@ ginFillScanKey(GinScanOpaque so, GinScanKey	key, bool initHiddenEntries,
 {
 	GinState   *ginstate = &so->ginstate;
 	uint32		nUserQueryValues = nQueryValues;
+	uint32		nAllocatedQueryValues = nQueryValues;
 	uint32		i;
 
 	if (key == NULL)
 		key = &(so->keys[so->nkeys++]);
 
-	/* Non-default search modes add one "hidden" entry to each key */
-	if (searchMode != GIN_SEARCH_MODE_DEFAULT && initHiddenEntries)
-		nQueryValues++;
+	/*
+	 * Non-default search modes add one "hidden" entry to each key, but this
+	 * entry is initialized only if requested by initHiddenEntries flag.
+	 */
+	if (searchMode != GIN_SEARCH_MODE_DEFAULT)
+	{
+		nAllocatedQueryValues++;
+
+		if (initHiddenEntries)
+			nQueryValues++;
+	}
+
 	key->nentries = nQueryValues;
 	key->nuserentries = nUserQueryValues;
 
-	key->scanEntry = (GinScanEntry *) palloc(sizeof(GinScanEntry) * nQueryValues);
-	key->entryRes = (GinTernaryValue *) palloc0(sizeof(GinTernaryValue) * nQueryValues);
+	key->scanEntry = (GinScanEntry *) palloc(sizeof(GinScanEntry) * nAllocatedQueryValues);
+	key->entryRes = (GinTernaryValue *) palloc0(sizeof(GinTernaryValue) * nAllocatedQueryValues);
 
 	key->query = query;
 	key->queryValues = queryValues;
@@ -158,6 +168,8 @@ ginFillScanKey(GinScanOpaque so, GinScanKey	key, bool initHiddenEntries,
 	key->strategy = strategy;
 	key->searchMode = searchMode;
 	key->attnum = attnum;
+	key->includeNonMatching = false;
+	key->recheckNonMatching = false;
 
 	ItemPointerSetMin(&key->curItem);
 	key->curItemMatches = false;
@@ -272,6 +284,8 @@ ginNewScanKey(IndexScanDesc scan)
 	GinScanOpaque so = (GinScanOpaque) scan->opaque;
 	int			i;
 	bool		hasNullQuery = false;
+	bool		hasAllQuery = false;
+	bool		hasRegularQuery = false;
 	/*
 	 * GIN_FALSE - column has no search keys
 	 * GIN_TRUE  - NOT NULL is implied by some non-empty search key
@@ -359,7 +373,25 @@ ginNewScanKey(IndexScanDesc scan)
 				so->isVoidRes = true;
 				break;
 			}
-			else if (searchMode == GIN_SEARCH_MODE_ALL)
+
+			nQueryValues = 0;	/* ensure sane value */
+		}
+
+		if (searchMode == GIN_SEARCH_MODE_ALL)
+		{
+			hasAllQuery = true;
+
+			/*
+			 * Increment the number of columns with NOT NULL constraints
+			 * if NOT NULL is not yet implied by some non-ALL key.
+			 */
+			if (colNotNull[colno] == GIN_FALSE)
+			{
+				colNotNull[colno] = GIN_MAYBE;
+				colNotNullCount++;
+			}
+
+			if (!nQueryValues)
 			{
 				/*
 				 * The query probably matches all non-null items, but rather
@@ -386,32 +418,22 @@ ginNewScanKey(IndexScanDesc scan)
 				}
 
 				so->forcedRecheck |= res != GIN_TRUE;
-
-				/*
-				 * Increment the number of columns with NOT NULL constraints
-				 * if NOT NULL is not yet implied by some non-empty key.
-				 */
-				if (colNotNull[colno] == GIN_FALSE)
-				{
-					colNotNull[colno] = GIN_MAYBE;
-					colNotNullCount++;
-				}
-
 				continue;
 			}
-
-			nQueryValues = 0;	/* ensure sane value */
 		}
+		else
+		{
+			hasRegularQuery = true;
 
-		/*
-		 * Current non-empty key implies that column is NOT NULL, so decrement
-		 * the number of columns with NOT NULL if NOT NULL already is implied by
-		 * some empty ALL key.
-		 */
-		if (colNotNull[colno] == GIN_MAYBE)
-			colNotNullCount--;
+			/*
+			 * Current key implies that column is NOT NULL, so decrement the
+			 * number of columns with NOT NULL constraints.
+			 */
+			if (colNotNull[colno] == GIN_MAYBE)
+				colNotNullCount--;
 
-		colNotNull[colno] = GIN_TRUE;
+			colNotNull[colno] = GIN_TRUE;
+		}
 
 		/*
 		 * Create GinNullCategory representation.  If the extractQueryFn
@@ -433,7 +455,10 @@ ginNewScanKey(IndexScanDesc scan)
 			}
 		}
 
-		ginFillScanKey(so, NULL, true, skey->sk_attno,
+		ginFillScanKey(so, NULL,
+					   /* postpone initialization of hidden ALL entry */
+					   searchMode != GIN_SEARCH_MODE_ALL,
+					   skey->sk_attno,
 					   skey->sk_strategy, searchMode,
 					   skey->sk_argument, nQueryValues,
 					   queryValues, categories,
@@ -469,13 +494,88 @@ ginNewScanKey(IndexScanDesc scan)
 				}
 			}
 		}
-		else if (colNotNullCount)
+		else
 		{
-			/*
-			 * We use recheck instead of adding NOT_NULL entries to eliminate
-			 * rows with NULL columns.
-			 */
-			so->forcedRecheck = true;
+			if (hasAllQuery)
+			{
+				GinScanKey	nonAllKey = NULL;
+				int			nkeys = so->nkeys;
+				int			i = 0;
+
+				/*
+				 * Finalize non-empty ALL keys: if we have regular keys, enable
+				 * includeNonMatching mode in ALL keys, otherwise initialize
+				 * previously skipped hidden ALL entries.
+				 */
+				for (i = 0; i < nkeys; i++)
+				{
+					GinScanKey	key = &so->keys[i];
+
+					if (key->searchMode != GIN_SEARCH_MODE_ALL)
+					{
+						nonAllKey = key;
+					}
+					else if (hasRegularQuery)
+					{
+						memset(key->entryRes, GIN_FALSE, key->nentries);
+
+						switch (key->triConsistentFn(key))
+						{
+							case GIN_TRUE:
+								key->includeNonMatching = true;
+								key->recheckNonMatching = false;
+								break;
+							case GIN_MAYBE:
+								key->includeNonMatching = true;
+								key->recheckNonMatching = true;
+							default:
+								/*
+								 * Items with no matching entries are not
+								 * accepted, leave the key as is.
+								 */
+								break;
+						}
+					}
+					else
+					{
+						/* Initialize missing hidden ALL entry */
+						key->scanEntry[key->nentries++] =
+							ginFillScanEntry(so, key->attnum, key->strategy,
+											 GIN_SEARCH_MODE_ALL, (Datum) 0,
+											 GIN_CAT_EMPTY_QUERY, false, NULL);
+
+						/*
+						 * ALL entry implies NOT NULL, so update the number of
+						 * NOT NULL columns.
+						 */
+						if (colNotNull[key->attnum - 1] == GIN_MAYBE)
+						{
+							colNotNull[key->attnum - 1] = GIN_TRUE;
+							colNotNullCount--;
+						}
+					}
+				}
+
+				/* Move some non-ALL key to the beginning (see scanGetItem()) */
+				if (so->keys[0].includeNonMatching)
+				{
+					GinScanKeyData tmp = so->keys[0];
+
+					Assert(nonAllKey);
+
+					so->keys[0] = *nonAllKey;
+					*nonAllKey = tmp;
+				}
+			}
+
+			if (colNotNullCount > 0)
+			{
+				/*
+				 * We use recheck instead of adding NOT_NULL entries to
+				 * eliminate rows with NULL columns.
+				 */
+				so->forcedRecheck = true;
+			}
 		}
 
 		/*
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 46fff24..35998ed 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -6240,7 +6240,6 @@ spgcostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
 
 typedef struct
 {
-	bool		haveFullScan;
 	double		partialEntries;
 	double		exactEntries;
 	double		searchEntries;
@@ -6317,23 +6316,21 @@ gincost_pattern(IndexOptInfo *index, int indexcol,
 						 PointerGetDatum(&searchMode));
 
 	/* Special cases for queries that contain no keys */
-	if (nentries <= 0)
+	if (nentries <= 0 && searchMode == GIN_SEARCH_MODE_DEFAULT)
 	{
-		if (searchMode == GIN_SEARCH_MODE_DEFAULT)
-		{
-			/* In default mode, no keys means an unsatisfiable query */
-			return false;
-		}
-		else if (searchMode == GIN_SEARCH_MODE_ALL)
-		{
-			/*
-			 * ginNewScanKey() does not emit scankeys for a key-less ALL
-			 * query.  Instead it will emit an EVERYTHING key, but only if
-			 * there are no other regular keys.  We don't know that yet, so
-			 * postpone setting the haveFullScan flag.
-			 */
-			return true;
-		}
+		/* In default mode, no keys means an unsatisfiable query */
+		return false;
+	}
+
+	if (searchMode == GIN_SEARCH_MODE_ALL)
+	{
+		/*
+		 * ginNewScanKey() does not emit scankeys for a key-less ALL
+		 * query.  Instead it will emit an EVERYTHING key, but only if
+		 * there are no other regular keys.  We don't know that yet, so
+		 * postpone setting the haveFullScan flag.
+		 */
+		return true;
 	}
 
 	for (i = 0; i < nentries; i++)
@@ -6356,11 +6353,8 @@ gincost_pattern(IndexOptInfo *index, int indexcol,
 		counts->exactEntries++;
 		counts->searchEntries++;
 	}
-	else if (searchMode != GIN_SEARCH_MODE_DEFAULT)
-	{
-		/* It's GIN_SEARCH_MODE_ALL */
-		counts->haveFullScan = true;
-	}
+
+	Assert(counts->searchEntries > 0);
 
 	return true;
 }
@@ -6497,9 +6491,6 @@ gincost_scalararrayopexpr(PlannerInfo *root,
 
 			/* If no regular scan keys, assume an EVERYTHING scan is needed */
 			if (elemcounts.searchEntries == 0)
-				elemcounts.haveFullScan = true;
-
-			if (elemcounts.haveFullScan)
 			{
 				/*
 				 * Full index scan will be required.  We treat this as if
@@ -6724,10 +6715,7 @@ gincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
 	}
 
 	/* If no regular scan keys, assume an EVERYTHING scan is needed */
-	if (counts.searchEntries == 0)
-		counts.haveFullScan = true;
-
-	if (counts.haveFullScan || indexQuals == NIL)
+	if (!counts.searchEntries || indexQuals == NIL)
 	{
 		/*
 		 * Full index scan will be required.  We treat this as if every key in
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 9d2ee3a..2c04496 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -297,6 +297,12 @@ typedef struct GinScanKeyData
 	StrategyNumber strategy;
 	int32		searchMode;
 	OffsetNumber attnum;
+	/*
+	 * Include/recheck items from other scankeys that has no entries of this
+	 * scankey. (This is used for execution of ALL keys without a full scan.)
+	 */
+	bool		includeNonMatching;
+	bool		recheckNonMatching;
 
 	/*
 	 * Match status data.  curItem is the TID most recently tested (could be a
diff --git a/src/test/regress/expected/tsearch.out b/src/test/regress/expected/tsearch.out
index 7af2899..506cd58 100644
--- a/src/test/regress/expected/tsearch.out
+++ b/src/test/regress/expected/tsearch.out
@@ -337,6 +337,29 @@ SELECT count(*) FROM test_tsvector WHERE a @@ '!no_such_lexeme';
    508
 (1 row)
 
+-- Test optimization of non-empty GIN_SEARCH_MODE_ALL queries
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF)
+SELECT * FROM test_tsvector WHERE a @@ '!qh';
+                         QUERY PLAN                          
+-------------------------------------------------------------
+ Bitmap Heap Scan on test_tsvector (actual rows=410 loops=1)
+   Recheck Cond: (a @@ '!''qh'''::tsquery)
+   Heap Blocks: exact=25
+   ->  Bitmap Index Scan on wowidx (actual rows=410 loops=1)
+         Index Cond: (a @@ '!''qh'''::tsquery)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF)
+SELECT * FROM test_tsvector WHERE a @@ 'wr' AND a @@ '!qh';
+                                  QUERY PLAN                                  
+------------------------------------------------------------------------------
+ Bitmap Heap Scan on test_tsvector (actual rows=60 loops=1)
+   Recheck Cond: ((a @@ '''wr'''::tsquery) AND (a @@ '!''qh'''::tsquery))
+   Heap Blocks: exact=22
+   ->  Bitmap Index Scan on wowidx (actual rows=60 loops=1)
+         Index Cond: ((a @@ '''wr'''::tsquery) AND (a @@ '!''qh'''::tsquery))
+(5 rows)
+
 RESET enable_seqscan;
 INSERT INTO test_tsvector VALUES ('???', 'DFG:1A,2B,6C,10 FGH');
 SELECT * FROM ts_stat('SELECT a FROM test_tsvector') ORDER BY ndoc DESC, nentry DESC, word LIMIT 10;
diff --git a/src/test/regress/sql/tsearch.sql b/src/test/regress/sql/tsearch.sql
index ece80b9..54a5eef 100644
--- a/src/test/regress/sql/tsearch.sql
+++ b/src/test/regress/sql/tsearch.sql
@@ -111,6 +111,13 @@ SELECT count(*) FROM test_tsvector WHERE a @@ any ('{wr,qh}');
 SELECT count(*) FROM test_tsvector WHERE a @@ 'no_such_lexeme';
 SELECT count(*) FROM test_tsvector WHERE a @@ '!no_such_lexeme';
 
+-- Test optimization of non-empty GIN_SEARCH_MODE_ALL queries
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF)
+SELECT * FROM test_tsvector WHERE a @@ '!qh';
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF)
+SELECT * FROM test_tsvector WHERE a @@ 'wr' AND a @@ '!qh';
+
 RESET enable_seqscan;
 
 INSERT INTO test_tsvector VALUES ('???', 'DFG:1A,2B,6C,10 FGH');
-- 
2.7.4

