From 7e6c1cb9fe7468328683fa6d5f68cc5c8743f036 Mon Sep 17 00:00:00 2001
From: Nikita Glukhov <n.gluhov@postgrespro.ru>
Date: Thu, 1 Aug 2019 18:45:41 +0300
Subject: [PATCH 2/5] Avoid GIN recheck for NULLs using new search mode

---
 src/backend/access/gin/ginlogic.c |  3 +-
 src/backend/access/gin/ginscan.c  | 95 ++++++++++++++++++++++++++-------------
 src/include/access/gin.h          |  1 +
 src/test/regress/expected/gin.out | 86 ++++++++++++++++++++++++++++++++---
 src/test/regress/sql/gin.sql      | 63 ++++++++++++++++++++++++--
 5 files changed, 206 insertions(+), 42 deletions(-)

diff --git a/src/backend/access/gin/ginlogic.c b/src/backend/access/gin/ginlogic.c
index 8f85978..7c4805d 100644
--- a/src/backend/access/gin/ginlogic.c
+++ b/src/backend/access/gin/ginlogic.c
@@ -224,7 +224,8 @@ shimTriConsistentFn(GinScanKey key)
 void
 ginInitConsistentFunction(GinState *ginstate, GinScanKey key)
 {
-	if (key->searchMode == GIN_SEARCH_MODE_EVERYTHING)
+	if (key->searchMode == GIN_SEARCH_MODE_EVERYTHING ||
+		key->searchMode == GIN_SEARCH_MODE_NOT_NULL)
 	{
 		key->boolConsistentFn = trueConsistentFn;
 		key->triConsistentFn = trueTriConsistentFn;
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index 7b8de10..903891f 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -200,6 +200,11 @@ ginFillScanKey(GinScanOpaque so, OffsetNumber attnum,
 				case GIN_SEARCH_MODE_EVERYTHING:
 					queryCategory = GIN_CAT_EMPTY_QUERY;
 					break;
+				case GIN_SEARCH_MODE_NOT_NULL:
+					queryCategory = GIN_CAT_EMPTY_QUERY;
+					/* use GIN_SEARCH_MODE_ALL to skip NULLs */
+					searchMode = GIN_SEARCH_MODE_ALL;
+					break;
 				default:
 					elog(ERROR, "unexpected searchMode: %d", searchMode);
 					queryCategory = 0;	/* keep compiler quiet */
@@ -265,6 +270,8 @@ ginNewScanKey(IndexScanDesc scan)
 	GinScanOpaque so = (GinScanOpaque) scan->opaque;
 	int			i;
 	bool		hasNullQuery = false;
+	bool		colNotNull[INDEX_MAX_KEYS] = {0};
+	int			numNotNullKeys = 0;
 	MemoryContext oldCtx;
 
 	/*
@@ -298,6 +305,7 @@ ginNewScanKey(IndexScanDesc scan)
 		bool	   *nullFlags = NULL;
 		GinNullCategory *categories;
 		int32		searchMode = GIN_SEARCH_MODE_DEFAULT;
+		int			colno = skey->sk_attno - 1;
 
 		/*
 		 * We assume that GIN-indexable operators are strict, so a null query
@@ -311,8 +319,8 @@ ginNewScanKey(IndexScanDesc scan)
 
 		/* OK to call the extractQueryFn */
 		queryValues = (Datum *)
-			DatumGetPointer(FunctionCall7Coll(&so->ginstate.extractQueryFn[skey->sk_attno - 1],
-											  so->ginstate.supportCollation[skey->sk_attno - 1],
+			DatumGetPointer(FunctionCall7Coll(&so->ginstate.extractQueryFn[colno],
+											  so->ginstate.supportCollation[colno],
 											  skey->sk_argument,
 											  PointerGetDatum(&nQueryValues),
 											  UInt16GetDatum(skey->sk_strategy),
@@ -349,10 +357,17 @@ ginNewScanKey(IndexScanDesc scan)
 				 * The query probably matches all non-null items, but rather
 				 * than scanning the index in ALL mode, we use forced rechecks
 				 * to verify matches of this scankey.  This wins if there are
-				 * any non-ALL scankeys; otherwise we end up adding an
-				 * EVERYTHING scankey below.
+				 * any non-ALL scankeys; otherwise we end up adding a NOT_NULL
+				 * scankey below.
 				 */
 				so->forcedRecheck = true;
+
+				/*
+				 * Increment the number of columns with NOT NULL constraints.
+				 */
+				colNotNull[colno] = true;
+				numNotNullKeys++;
+
 				continue;
 			}
 
@@ -386,35 +401,53 @@ ginNewScanKey(IndexScanDesc scan)
 					   partial_matches, extra_data);
 	}
 
-	/*
-	 * If there are no regular scan keys, generate an EVERYTHING scankey to
-	 * drive a full-index scan.
-	 */
-	if (so->nkeys == 0 && !so->isVoidRes)
+	if (!so->isVoidRes)
 	{
-		hasNullQuery = true;
-		ginFillScanKey(so, FirstOffsetNumber,
-					   InvalidStrategy, GIN_SEARCH_MODE_EVERYTHING,
-					   (Datum) 0, 0,
-					   NULL, NULL, NULL, NULL);
-	}
+		/*
+		 * If there are no regular scan keys, generate one EVERYTHING or
+		 * several NOT_NULL scankeys to drive a full-index scan.
+		 */
+		if (so->nkeys == 0)
+		{
+			hasNullQuery = true;
 
-	/*
-	 * If the index is version 0, it may be missing null and placeholder
-	 * entries, which would render searches for nulls and full-index scans
-	 * unreliable.  Throw an error if so.
-	 */
-	if (hasNullQuery && !so->isVoidRes)
-	{
-		GinStatsData ginStats;
-
-		ginGetStats(scan->indexRelation, &ginStats);
-		if (ginStats.ginVersion < 1)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("old GIN indexes do not support whole-index scans nor searches for nulls"),
-					 errhint("To fix this, do REINDEX INDEX \"%s\".",
-							 RelationGetRelationName(scan->indexRelation))));
+			/* Initialize EVERYTHING key if there are no non-NULL columns. */
+			if (!numNotNullKeys)
+			{
+				ginFillScanKey(so, FirstOffsetNumber, InvalidStrategy,
+							   GIN_SEARCH_MODE_EVERYTHING, (Datum) 0, 0,
+							   NULL, NULL, NULL, NULL);
+			}
+			else
+			{
+				/* Initialize NOT_NULL key for each non-NULL column. */
+				for (i = 0; i < scan->indexRelation->rd_att->natts; i++)
+				{
+					if (colNotNull[i])
+						ginFillScanKey(so, i + 1, InvalidStrategy,
+									   GIN_SEARCH_MODE_NOT_NULL, (Datum) 0, 0,
+									   NULL, NULL, NULL, NULL);
+				}
+			}
+		}
+
+		/*
+		 * If the index is version 0, it may be missing null and placeholder
+		 * entries, which would render searches for nulls and full-index scans
+		 * unreliable.  Throw an error if so.
+		 */
+		if (hasNullQuery)
+		{
+			GinStatsData ginStats;
+
+			ginGetStats(scan->indexRelation, &ginStats);
+			if (ginStats.ginVersion < 1)
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("old GIN indexes do not support whole-index scans nor searches for nulls"),
+						 errhint("To fix this, do REINDEX INDEX \"%s\".",
+								 RelationGetRelationName(scan->indexRelation))));
+		}
 	}
 
 	MemoryContextSwitchTo(oldCtx);
diff --git a/src/include/access/gin.h b/src/include/access/gin.h
index a8eef5a..069d249 100644
--- a/src/include/access/gin.h
+++ b/src/include/access/gin.h
@@ -34,6 +34,7 @@
 #define GIN_SEARCH_MODE_INCLUDE_EMPTY	1
 #define GIN_SEARCH_MODE_ALL				2
 #define GIN_SEARCH_MODE_EVERYTHING		3	/* for internal use only */
+#define GIN_SEARCH_MODE_NOT_NULL		4	/* for internal use only */
 
 /*
  * GinStatsData represents stats data for planner use
diff --git a/src/test/regress/expected/gin.out b/src/test/regress/expected/gin.out
index 5ba96fa..12a64eb 100644
--- a/src/test/regress/expected/gin.out
+++ b/src/test/regress/expected/gin.out
@@ -38,9 +38,18 @@ vacuum gin_test_tbl;
 -- Test optimization of empty queries
 create temp table t_gin_test_tbl(i int4[], j int4[]);
 create index on t_gin_test_tbl using gin (i, j);
-insert into t_gin_test_tbl select array[100,g], array[200,g]
-from generate_series(1, 10) g;
-insert into t_gin_test_tbl values(array[0,0], null);
+insert into t_gin_test_tbl
+values
+  (null,    null),
+  ('{}',    null),
+  ('{1}',   null),
+  ('{1,2}', null),
+  (null,    '{}'),
+  (null,    '{10}'),
+  ('{1,2}', '{10}'),
+  ('{2}',   '{10}'),
+  ('{1,3}', '{}'),
+  ('{1,1}', '{10}');
 set enable_seqscan = off;
 explain (costs off)
 select * from t_gin_test_tbl where array[0] <@ i;
@@ -53,14 +62,77 @@ select * from t_gin_test_tbl where array[0] <@ i;
 (4 rows)
 
 select * from t_gin_test_tbl where array[0] <@ i;
-   i   | j 
--------+---
- {0,0} | 
-(1 row)
+ i | j 
+---+---
+(0 rows)
 
 select * from t_gin_test_tbl where array[0] <@ i and '{}'::int4[] <@ j;
  i | j 
 ---+---
 (0 rows)
 
+explain (analyze, costs off, timing off, summary off)
+select * from t_gin_test_tbl where i @> '{}';
+                                QUERY PLAN                                 
+---------------------------------------------------------------------------
+ Bitmap Heap Scan on t_gin_test_tbl (actual rows=7 loops=1)
+   Recheck Cond: (i @> '{}'::integer[])
+   Heap Blocks: exact=1
+   ->  Bitmap Index Scan on t_gin_test_tbl_i_j_idx (actual rows=7 loops=1)
+         Index Cond: (i @> '{}'::integer[])
+(5 rows)
+
+create or replace function explain_query_json(query_sql text)
+returns table (explain_line json)
+language plpgsql as
+$$
+begin
+  return query execute 'EXPLAIN (ANALYZE, FORMAT json) ' || query_sql;
+end;
+$$;
+create or replace function execute_text_query(query_sql text)
+returns setof text
+language plpgsql
+as
+$$
+begin
+  return query execute query_sql;
+end;
+$$;
+-- check number of rows returned by index and removed by recheck
+select
+  query,
+  js->0->'Plan'->'Plans'->0->'Actual Rows' as "return by index",
+  js->0->'Plan'->'Rows Removed by Index Recheck' as "removed by recheck",
+  res as "result"
+from
+  (values
+    ($$ i @> '{}' $$),
+    ($$ j @> '{}' $$),
+    ($$ i @> '{}' and j @> '{}' $$),
+    ($$ i @> '{1}' $$),
+    ($$ i @> '{1}' and j @> '{}' $$),
+    ($$ i @> '{1}' and i @> '{}' and j @> '{}' $$),
+    ($$ j @> '{10}' $$),
+    ($$ j @> '{10}' and i @> '{}' $$),
+    ($$ j @> '{10}' and j @> '{}' and i @> '{}' $$),
+    ($$ i @> '{1}' and j @> '{10}' $$)
+  ) q(query),
+  lateral explain_query_json($$select * from t_gin_test_tbl where $$ || query) js,
+  lateral execute_text_query($$select string_agg((i, j)::text, ' ') from t_gin_test_tbl where $$ || query) res;
+                   query                   | return by index | removed by recheck |                                    result                                     
+-------------------------------------------+-----------------+--------------------+-------------------------------------------------------------------------------
+  i @> '{}'                                | 7               | 0                  | ({},) ({1},) ("{1,2}",) ("{1,2}",{10}) ({2},{10}) ("{1,3}",{}) ("{1,1}",{10})
+  j @> '{}'                                | 6               | 0                  | (,{}) (,{10}) ("{1,2}",{10}) ({2},{10}) ("{1,3}",{}) ("{1,1}",{10})
+  i @> '{}' and j @> '{}'                  | 4               | 0                  | ("{1,2}",{10}) ({2},{10}) ("{1,3}",{}) ("{1,1}",{10})
+  i @> '{1}'                               | 5               | 0                  | ({1},) ("{1,2}",) ("{1,2}",{10}) ("{1,3}",{}) ("{1,1}",{10})
+  i @> '{1}' and j @> '{}'                 | 5               | 2                  | ("{1,2}",{10}) ("{1,3}",{}) ("{1,1}",{10})
+  i @> '{1}' and i @> '{}' and j @> '{}'   | 5               | 2                  | ("{1,2}",{10}) ("{1,3}",{}) ("{1,1}",{10})
+  j @> '{10}'                              | 4               | 0                  | (,{10}) ("{1,2}",{10}) ({2},{10}) ("{1,1}",{10})
+  j @> '{10}' and i @> '{}'                | 4               | 1                  | ("{1,2}",{10}) ({2},{10}) ("{1,1}",{10})
+  j @> '{10}' and j @> '{}' and i @> '{}'  | 4               | 1                  | ("{1,2}",{10}) ({2},{10}) ("{1,1}",{10})
+  i @> '{1}' and j @> '{10}'               | 2               | 0                  | ("{1,2}",{10}) ("{1,1}",{10})
+(10 rows)
+
 reset enable_seqscan;
+drop table t_gin_test_tbl;
diff --git a/src/test/regress/sql/gin.sql b/src/test/regress/sql/gin.sql
index f98fb7e..e0700c3 100644
--- a/src/test/regress/sql/gin.sql
+++ b/src/test/regress/sql/gin.sql
@@ -38,12 +38,69 @@ vacuum gin_test_tbl;
 -- Test optimization of empty queries
 create temp table t_gin_test_tbl(i int4[], j int4[]);
 create index on t_gin_test_tbl using gin (i, j);
-insert into t_gin_test_tbl select array[100,g], array[200,g]
-from generate_series(1, 10) g;
-insert into t_gin_test_tbl values(array[0,0], null);
+insert into t_gin_test_tbl
+values
+  (null,    null),
+  ('{}',    null),
+  ('{1}',   null),
+  ('{1,2}', null),
+  (null,    '{}'),
+  (null,    '{10}'),
+  ('{1,2}', '{10}'),
+  ('{2}',   '{10}'),
+  ('{1,3}', '{}'),
+  ('{1,1}', '{10}');
+
 set enable_seqscan = off;
 explain (costs off)
 select * from t_gin_test_tbl where array[0] <@ i;
 select * from t_gin_test_tbl where array[0] <@ i;
 select * from t_gin_test_tbl where array[0] <@ i and '{}'::int4[] <@ j;
+
+explain (analyze, costs off, timing off, summary off)
+select * from t_gin_test_tbl where i @> '{}';
+
+create or replace function explain_query_json(query_sql text)
+returns table (explain_line json)
+language plpgsql as
+$$
+begin
+  return query execute 'EXPLAIN (ANALYZE, FORMAT json) ' || query_sql;
+end;
+$$;
+
+create or replace function execute_text_query(query_sql text)
+returns setof text
+language plpgsql
+as
+$$
+begin
+  return query execute query_sql;
+end;
+$$;
+
+-- check number of rows returned by index and removed by recheck
+select
+  query,
+  js->0->'Plan'->'Plans'->0->'Actual Rows' as "return by index",
+  js->0->'Plan'->'Rows Removed by Index Recheck' as "removed by recheck",
+  res as "result"
+from
+  (values
+    ($$ i @> '{}' $$),
+    ($$ j @> '{}' $$),
+    ($$ i @> '{}' and j @> '{}' $$),
+    ($$ i @> '{1}' $$),
+    ($$ i @> '{1}' and j @> '{}' $$),
+    ($$ i @> '{1}' and i @> '{}' and j @> '{}' $$),
+    ($$ j @> '{10}' $$),
+    ($$ j @> '{10}' and i @> '{}' $$),
+    ($$ j @> '{10}' and j @> '{}' and i @> '{}' $$),
+    ($$ i @> '{1}' and j @> '{10}' $$)
+  ) q(query),
+  lateral explain_query_json($$select * from t_gin_test_tbl where $$ || query) js,
+  lateral execute_text_query($$select string_agg((i, j)::text, ' ') from t_gin_test_tbl where $$ || query) res;
+
 reset enable_seqscan;
+
+drop table t_gin_test_tbl;
-- 
2.7.4

