Put "excludeOnly" GIN scan keys at the end of the scankey array.

Commit 4b754d6c1 introduced the concept of an excludeOnly scan key,
which cannot select matching index entries but can reject
non-matching tuples, for example a tsquery such as '!term'.  There are
poorly-documented assumptions that such scan keys do not appear as the
first scan key.  ginNewScanKey did nothing to ensure that, however,
with the result that certain GIN index searches could go into an
infinite loop while apparently-equivalent queries with the clauses in
a different order were fine.

Fix by teaching ginNewScanKey to place all excludeOnly scan keys
after all not-excludeOnly ones.  So far as we know at present,
it might be sufficient to avoid the case where the very first
scan key is excludeOnly; but I'm not very convinced that there
aren't other dependencies on the ordering.

Bug: #19031
Reported-by: Tim Wood <washwithcare@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19031-0638148643d25548@postgresql.org
Backpatch-through: 13

diff --git a/contrib/pg_trgm/expected/pg_trgm.out b/contrib/pg_trgm/expected/pg_trgm.out
index b4654fc24a770b4f40297ea3cf560832166c70ed..418ba05ce08f700b22c4a7e74d5a6bfc92596ab2 100644 (file)
--- a/contrib/pg_trgm/expected/pg_trgm.out
+++ b/contrib/pg_trgm/expected/pg_trgm.out
@@ -4693,6 +4693,23 @@ select count(*) from test_trgm where t like '%99%' and t like '%qw%';
     19
 (1 row)
 
+explain (costs off)
+select count(*) from test_trgm where t %> '' and t %> '%qwerty%';
+                               QUERY PLAN                                
+-------------------------------------------------------------------------
+ Aggregate
+   ->  Bitmap Heap Scan on test_trgm
+         Recheck Cond: ((t %> ''::text) AND (t %> '%qwerty%'::text))
+         ->  Bitmap Index Scan on trgm_idx
+               Index Cond: ((t %> ''::text) AND (t %> '%qwerty%'::text))
+(5 rows)
+
+select count(*) from test_trgm where t %> '' and t %> '%qwerty%';
+ count 
+-------
+     0
+(1 row)
+
 -- ensure that pending-list items are handled correctly, too
 create temp table t_test_trgm(t text COLLATE "C");
 create index t_trgm_idx on t_test_trgm using gin (t gin_trgm_ops);
@@ -4731,6 +4748,23 @@ select count(*) from t_test_trgm where t like '%99%' and t like '%qw%';
      1
 (1 row)
 
+explain (costs off)
+select count(*) from t_test_trgm where t %> '' and t %> '%qwerty%';
+                               QUERY PLAN                                
+-------------------------------------------------------------------------
+ Aggregate
+   ->  Bitmap Heap Scan on t_test_trgm
+         Recheck Cond: ((t %> ''::text) AND (t %> '%qwerty%'::text))
+         ->  Bitmap Index Scan on t_trgm_idx
+               Index Cond: ((t %> ''::text) AND (t %> '%qwerty%'::text))
+(5 rows)
+
+select count(*) from t_test_trgm where t %> '' and t %> '%qwerty%';
+ count 
+-------
+     0
+(1 row)
+
 -- run the same queries with sequential scan to check the results
 set enable_bitmapscan=off;
 set enable_seqscan=on;
@@ -4746,6 +4780,12 @@ select count(*) from test_trgm where t like '%99%' and t like '%qw%';
     19
 (1 row)
 
+select count(*) from test_trgm where t %> '' and t %> '%qwerty%';
+ count 
+-------
+     0
+(1 row)
+
 select count(*) from t_test_trgm where t like '%99%' and t like '%qwerty%';
  count 
 -------
@@ -4758,6 +4798,12 @@ select count(*) from t_test_trgm where t like '%99%' and t like '%qw%';
      1
 (1 row)
 
+select count(*) from t_test_trgm where t %> '' and t %> '%qwerty%';
+ count 
+-------
+     0
+(1 row)
+
 reset enable_bitmapscan;
 create table test2(t text COLLATE "C");
 insert into test2 values ('abcdef');

diff --git a/contrib/pg_trgm/sql/pg_trgm.sql b/contrib/pg_trgm/sql/pg_trgm.sql
index cc5b4df03ff9d09a9672afccdafefca7532d54d2..5f5aee628c8cfc3e6ace1ec9b6d3b048adc78cb6 100644 (file)
--- a/contrib/pg_trgm/sql/pg_trgm.sql
+++ b/contrib/pg_trgm/sql/pg_trgm.sql
@@ -80,6 +80,9 @@ select count(*) from test_trgm where t like '%99%' and t like '%qwerty%';
 explain (costs off)
 select count(*) from test_trgm where t like '%99%' and t like '%qw%';
 select count(*) from test_trgm where t like '%99%' and t like '%qw%';
+explain (costs off)
+select count(*) from test_trgm where t %> '' and t %> '%qwerty%';
+select count(*) from test_trgm where t %> '' and t %> '%qwerty%';
 -- ensure that pending-list items are handled correctly, too
 create temp table t_test_trgm(t text COLLATE "C");
 create index t_trgm_idx on t_test_trgm using gin (t gin_trgm_ops);
@@ -90,14 +93,19 @@ select count(*) from t_test_trgm where t like '%99%' and t like '%qwerty%';
 explain (costs off)
 select count(*) from t_test_trgm where t like '%99%' and t like '%qw%';
 select count(*) from t_test_trgm where t like '%99%' and t like '%qw%';
+explain (costs off)
+select count(*) from t_test_trgm where t %> '' and t %> '%qwerty%';
+select count(*) from t_test_trgm where t %> '' and t %> '%qwerty%';
 
 -- run the same queries with sequential scan to check the results
 set enable_bitmapscan=off;
 set enable_seqscan=on;
 select count(*) from test_trgm where t like '%99%' and t like '%qwerty%';
 select count(*) from test_trgm where t like '%99%' and t like '%qw%';
+select count(*) from test_trgm where t %> '' and t %> '%qwerty%';
 select count(*) from t_test_trgm where t like '%99%' and t like '%qwerty%';
 select count(*) from t_test_trgm where t like '%99%' and t like '%qw%';
+select count(*) from t_test_trgm where t %> '' and t %> '%qwerty%';
 reset enable_bitmapscan;
 
 create table test2(t text COLLATE "C");

diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index 2510747b044e914f370288278eeb34d0cb769e13..08943d79aa6945490410148106e4db9941ff41db 100644 (file)
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -270,6 +270,7 @@ ginNewScanKey(IndexScanDesc scan)
        ScanKey         scankey = scan->keyData;
        GinScanOpaque so = (GinScanOpaque) scan->opaque;
        int                     i;
+       int                     numExcludeOnly;
        bool            hasNullQuery = false;
        bool            attrHasNormalScan[INDEX_MAX_KEYS] = {false};
        MemoryContext oldCtx;
@@ -392,6 +393,7 @@ ginNewScanKey(IndexScanDesc scan)
         * excludeOnly scan key must receive a GIN_CAT_EMPTY_QUERY hidden entry
         * and be set to normal (excludeOnly = false).
         */
+       numExcludeOnly = 0;
        for (i = 0; i < so->nkeys; i++)
        {
                GinScanKey      key = &so->keys[i];
@@ -405,6 +407,47 @@ ginNewScanKey(IndexScanDesc scan)
                        ginScanKeyAddHiddenEntry(so, key, GIN_CAT_EMPTY_QUERY);
                        attrHasNormalScan[key->attnum - 1] = true;
                }
+               else
+                       numExcludeOnly++;
+       }
+
+       /*
+        * If we left any excludeOnly scan keys as-is, move them to the end of the
+        * scan key array: they must appear after normal key(s).
+        */
+       if (numExcludeOnly > 0)
+       {
+               GinScanKey      tmpkeys;
+               int                     iNormalKey;
+               int                     iExcludeOnly;
+
+               /* We'd better have made at least one normal key */
+               Assert(numExcludeOnly < so->nkeys);
+               /* Make a temporary array to hold the re-ordered scan keys */
+               tmpkeys = (GinScanKey) palloc(so->nkeys * sizeof(GinScanKeyData));
+               /* Re-order the keys ... */
+               iNormalKey = 0;
+               iExcludeOnly = so->nkeys - numExcludeOnly;
+               for (i = 0; i < so->nkeys; i++)
+               {
+                       GinScanKey      key = &so->keys[i];
+
+                       if (key->excludeOnly)
+                       {
+                               memcpy(tmpkeys + iExcludeOnly, key, sizeof(GinScanKeyData));
+                               iExcludeOnly++;
+                       }
+                       else
+                       {
+                               memcpy(tmpkeys + iNormalKey, key, sizeof(GinScanKeyData));
+                               iNormalKey++;
+                       }
+               }
+               Assert(iNormalKey == so->nkeys - numExcludeOnly);
+               Assert(iExcludeOnly == so->nkeys);
+               /* ... and copy them back to so->keys[] */
+               memcpy(so->keys, tmpkeys, so->nkeys * sizeof(GinScanKeyData));
+               pfree(tmpkeys);
        }
 
        /*