Fix intarray's GiST opclasses to not fail for empty arrays with <@.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 6 Aug 2019 22:04:51 +0000 (18:04 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 6 Aug 2019 22:04:51 +0000 (18:04 -0400)
contrib/intarray considers "arraycol <@ constant-array" to be indexable,
but its GiST opclass code fails to reliably find index entries for empty
array values (which of course should trivially match such queries).
This is because the test condition to see whether we should descend
through a non-leaf node is wrong.

Unfortunately, empty array entries could be anywhere in the index,
as these index opclasses are currently designed.  So there's no way
to fix this except by lobotomizing <@ indexscans to scan the whole
index ... which is what this patch does.  That's pretty unfortunate:
the performance is now actually worse than a seqscan, in most cases.
We'd be better off to remove <@ from the GiST opclasses entirely,
and perhaps a future non-back-patchable patch will do so.

In the meantime, applications whose performance is adversely impacted
have a couple of options.  They could switch to a GIN index, which
doesn't have this bug, or they could replace "arraycol <@ constant-array"
with "arraycol <@ constant-array AND arraycol && constant-array".
That will provide about the same performance as before, and it will find
all non-empty subsets of the given constant-array, which is all that
could reliably be expected of the query before.

While at it, add some more regression test cases to improve code
coverage of contrib/intarray.

In passing, adjust resize_intArrayType so that when it's returning an
empty array, it uses construct_empty_array for that rather than
cowboy hacking on the input array.  While the hack produces an array
that looks valid for most purposes, it isn't bitwise equal to empty
arrays produced by other code paths, which could have subtle odd
effects.  I don't think this code path is performance-critical
enough to justify such shortcuts.  (Back-patch this part only as far
as v11; before commit 01783ac36 we were not careful about this in
other intarray code paths either.)

Back-patch the <@ fixes to all supported versions, since this was
broken from day one.

Patch by me; thanks to Alexander Korotkov for review.

Discussion: https://postgr.es/m/458.1565114141@sss.pgh.pa.us

contrib/intarray/_int_gist.c
contrib/intarray/_int_tool.c
contrib/intarray/_intbig_gist.c
contrib/intarray/expected/_int.out
contrib/intarray/sql/_int.sql

index 13dd7ac202c63a33f825ac247d89e99d78e962cb..e5a8011daf814b247141b53312e65a55035e67c6 100644 (file)
@@ -96,8 +96,13 @@ g_int_consistent(PG_FUNCTION_ARGS)
                retval = inner_int_contains(query,
                                            (ArrayType *) DatumGetPointer(entry->key));
            else
-               retval = inner_int_overlap((ArrayType *) DatumGetPointer(entry->key),
-                                          query);
+           {
+               /*
+                * Unfortunately, because empty arrays could be anywhere in
+                * the index, we must search the whole tree.
+                */
+               retval = true;
+           }
            break;
        default:
            retval = false;
index 2d3e50178f6c16b1f0ec1db77995d36c36afbe95..fde8d15e2c2e5faeb635a17842f0867a06a5305a 100644 (file)
@@ -256,7 +256,7 @@ resize_intArrayType(ArrayType *a, int num)
    if (num <= 0)
    {
        Assert(num == 0);
-       ARR_NDIM(a) = 0;
+       a = construct_empty_array(INT4OID);
        return a;
    }
 
index 8d3b3f5e405869d048e6ee1078accb6d84d5046d..2a20abecc6cc7f4bc70b471d24f5f3bac7cdf0f9 100644 (file)
@@ -567,7 +567,13 @@ g_intbig_consistent(PG_FUNCTION_ARGS)
                }
            }
            else
-               retval = _intbig_overlap((GISTTYPE *) DatumGetPointer(entry->key), query);
+           {
+               /*
+                * Unfortunately, because empty arrays could be anywhere in
+                * the index, we must search the whole tree.
+                */
+               retval = true;
+           }
            break;
        default:
            retval = false;
index 105c951badf9c7c034624eb5b323d18a28e6190e..c92a56524a3c31562a44a22c5786e17d43a6a1d3 100644 (file)
@@ -431,6 +431,18 @@ SELECT count(*) from test__int WHERE a @> '{20,23}';
     12
 (1 row)
 
+SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
+ count 
+-------
+    10
+(1 row)
+
+SELECT count(*) from test__int WHERE a = '{73,23,20}';
+ count 
+-------
+     1
+(1 row)
+
 SELECT count(*) from test__int WHERE a @@ '50&68';
  count 
 -------
@@ -449,6 +461,19 @@ SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
     21
 (1 row)
 
+SELECT count(*) from test__int WHERE a @@ '20 | !21';
+ count 
+-------
+  6566
+(1 row)
+
+SELECT count(*) from test__int WHERE a @@ '!20 & !21';
+ count 
+-------
+  6343
+(1 row)
+
+SET enable_seqscan = off;  -- not all of these would use index by default
 CREATE INDEX text_idx on test__int using gist ( a gist__int_ops );
 SELECT count(*) from test__int WHERE a && '{23,50}';
  count 
@@ -480,6 +505,18 @@ SELECT count(*) from test__int WHERE a @> '{20,23}';
     12
 (1 row)
 
+SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
+ count 
+-------
+    10
+(1 row)
+
+SELECT count(*) from test__int WHERE a = '{73,23,20}';
+ count 
+-------
+     1
+(1 row)
+
 SELECT count(*) from test__int WHERE a @@ '50&68';
  count 
 -------
@@ -498,6 +535,18 @@ SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
     21
 (1 row)
 
+SELECT count(*) from test__int WHERE a @@ '20 | !21';
+ count 
+-------
+  6566
+(1 row)
+
+SELECT count(*) from test__int WHERE a @@ '!20 & !21';
+ count 
+-------
+  6343
+(1 row)
+
 DROP INDEX text_idx;
 CREATE INDEX text_idx on test__int using gist ( a gist__intbig_ops );
 SELECT count(*) from test__int WHERE a && '{23,50}';
@@ -530,6 +579,18 @@ SELECT count(*) from test__int WHERE a @> '{20,23}';
     12
 (1 row)
 
+SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
+ count 
+-------
+    10
+(1 row)
+
+SELECT count(*) from test__int WHERE a = '{73,23,20}';
+ count 
+-------
+     1
+(1 row)
+
 SELECT count(*) from test__int WHERE a @@ '50&68';
  count 
 -------
@@ -548,6 +609,18 @@ SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
     21
 (1 row)
 
+SELECT count(*) from test__int WHERE a @@ '20 | !21';
+ count 
+-------
+  6566
+(1 row)
+
+SELECT count(*) from test__int WHERE a @@ '!20 & !21';
+ count 
+-------
+  6343
+(1 row)
+
 DROP INDEX text_idx;
 CREATE INDEX text_idx on test__int using gin ( a gin__int_ops );
 SELECT count(*) from test__int WHERE a && '{23,50}';
@@ -580,6 +653,18 @@ SELECT count(*) from test__int WHERE a @> '{20,23}';
     12
 (1 row)
 
+SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
+ count 
+-------
+    10
+(1 row)
+
+SELECT count(*) from test__int WHERE a = '{73,23,20}';
+ count 
+-------
+     1
+(1 row)
+
 SELECT count(*) from test__int WHERE a @@ '50&68';
  count 
 -------
@@ -598,3 +683,16 @@ SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
     21
 (1 row)
 
+SELECT count(*) from test__int WHERE a @@ '20 | !21';
+ count 
+-------
+  6566
+(1 row)
+
+SELECT count(*) from test__int WHERE a @@ '!20 & !21';
+ count 
+-------
+  6343
+(1 row)
+
+RESET enable_seqscan;
index 40225c65abbfc9c0b9d1dce5074b1754121390a2..6ca7e3cca7eec88b7f3138d879ac268a7f5ce2d6 100644 (file)
@@ -85,9 +85,15 @@ SELECT count(*) from test__int WHERE a @@ '23|50';
 SELECT count(*) from test__int WHERE a @> '{23,50}';
 SELECT count(*) from test__int WHERE a @@ '23&50';
 SELECT count(*) from test__int WHERE a @> '{20,23}';
+SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
+SELECT count(*) from test__int WHERE a = '{73,23,20}';
 SELECT count(*) from test__int WHERE a @@ '50&68';
 SELECT count(*) from test__int WHERE a @> '{20,23}' or a @> '{50,68}';
 SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
+SELECT count(*) from test__int WHERE a @@ '20 | !21';
+SELECT count(*) from test__int WHERE a @@ '!20 & !21';
+
+SET enable_seqscan = off;  -- not all of these would use index by default
 
 CREATE INDEX text_idx on test__int using gist ( a gist__int_ops );
 
@@ -96,9 +102,13 @@ SELECT count(*) from test__int WHERE a @@ '23|50';
 SELECT count(*) from test__int WHERE a @> '{23,50}';
 SELECT count(*) from test__int WHERE a @@ '23&50';
 SELECT count(*) from test__int WHERE a @> '{20,23}';
+SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
+SELECT count(*) from test__int WHERE a = '{73,23,20}';
 SELECT count(*) from test__int WHERE a @@ '50&68';
 SELECT count(*) from test__int WHERE a @> '{20,23}' or a @> '{50,68}';
 SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
+SELECT count(*) from test__int WHERE a @@ '20 | !21';
+SELECT count(*) from test__int WHERE a @@ '!20 & !21';
 
 DROP INDEX text_idx;
 CREATE INDEX text_idx on test__int using gist ( a gist__intbig_ops );
@@ -108,9 +118,13 @@ SELECT count(*) from test__int WHERE a @@ '23|50';
 SELECT count(*) from test__int WHERE a @> '{23,50}';
 SELECT count(*) from test__int WHERE a @@ '23&50';
 SELECT count(*) from test__int WHERE a @> '{20,23}';
+SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
+SELECT count(*) from test__int WHERE a = '{73,23,20}';
 SELECT count(*) from test__int WHERE a @@ '50&68';
 SELECT count(*) from test__int WHERE a @> '{20,23}' or a @> '{50,68}';
 SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
+SELECT count(*) from test__int WHERE a @@ '20 | !21';
+SELECT count(*) from test__int WHERE a @@ '!20 & !21';
 
 DROP INDEX text_idx;
 CREATE INDEX text_idx on test__int using gin ( a gin__int_ops );
@@ -120,6 +134,12 @@ SELECT count(*) from test__int WHERE a @@ '23|50';
 SELECT count(*) from test__int WHERE a @> '{23,50}';
 SELECT count(*) from test__int WHERE a @@ '23&50';
 SELECT count(*) from test__int WHERE a @> '{20,23}';
+SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
+SELECT count(*) from test__int WHERE a = '{73,23,20}';
 SELECT count(*) from test__int WHERE a @@ '50&68';
 SELECT count(*) from test__int WHERE a @> '{20,23}' or a @> '{50,68}';
 SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
+SELECT count(*) from test__int WHERE a @@ '20 | !21';
+SELECT count(*) from test__int WHERE a @@ '!20 & !21';
+
+RESET enable_seqscan;