Skip to content

Commit efc77cf

Browse files
tglsfdctglsfdc
committed
Fix intarray's GiST opclasses to not fail for empty arrays with <@.
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 01783ac 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/[email protected]
1 parent 4ecd05c commit efc77cf

File tree

5 files changed

+133
-4
lines changed

5 files changed

+133
-4
lines changed

‎contrib/intarray/_int_gist.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,13 @@ g_int_consistent(PG_FUNCTION_ARGS)
9696
retval = inner_int_contains(query,
9797
(ArrayType *) DatumGetPointer(entry->key));
9898
else
99-
retval = inner_int_overlap((ArrayType *) DatumGetPointer(entry->key),
100-
query);
99+
{
100+
/*
101+
* Unfortunately, because empty arrays could be anywhere in
102+
* the index, we must search the whole tree.
103+
*/
104+
retval = true;
105+
}
101106
break;
102107
default:
103108
retval = false;

‎contrib/intarray/_int_tool.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ resize_intArrayType(ArrayType *a, int num)
256256
if (num <= 0)
257257
{
258258
Assert(num == 0);
259-
ARR_NDIM(a) = 0;
259+
a = construct_empty_array(INT4OID);
260260
return a;
261261
}
262262

‎contrib/intarray/_intbig_gist.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,13 @@ g_intbig_consistent(PG_FUNCTION_ARGS)
567567
}
568568
}
569569
else
570-
retval = _intbig_overlap((GISTTYPE *) DatumGetPointer(entry->key), query);
570+
{
571+
/*
572+
* Unfortunately, because empty arrays could be anywhere in
573+
* the index, we must search the whole tree.
574+
*/
575+
retval = true;
576+
}
571577
break;
572578
default:
573579
retval = false;

‎contrib/intarray/expected/_int.out

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,18 @@ SELECT count(*) from test__int WHERE a @> '{20,23}';
431431
12
432432
(1 row)
433433

434+
SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
435+
count
436+
-------
437+
10
438+
(1 row)
439+
440+
SELECT count(*) from test__int WHERE a = '{73,23,20}';
441+
count
442+
-------
443+
1
444+
(1 row)
445+
434446
SELECT count(*) from test__int WHERE a @@ '50&68';
435447
count
436448
-------
@@ -449,6 +461,19 @@ SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
449461
21
450462
(1 row)
451463

464+
SELECT count(*) from test__int WHERE a @@ '20 | !21';
465+
count
466+
-------
467+
6566
468+
(1 row)
469+
470+
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
471+
count
472+
-------
473+
6343
474+
(1 row)
475+
476+
SET enable_seqscan = off; -- not all of these would use index by default
452477
CREATE INDEX text_idx on test__int using gist ( a gist__int_ops );
453478
SELECT count(*) from test__int WHERE a && '{23,50}';
454479
count
@@ -480,6 +505,18 @@ SELECT count(*) from test__int WHERE a @> '{20,23}';
480505
12
481506
(1 row)
482507

508+
SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
509+
count
510+
-------
511+
10
512+
(1 row)
513+
514+
SELECT count(*) from test__int WHERE a = '{73,23,20}';
515+
count
516+
-------
517+
1
518+
(1 row)
519+
483520
SELECT count(*) from test__int WHERE a @@ '50&68';
484521
count
485522
-------
@@ -498,6 +535,18 @@ SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
498535
21
499536
(1 row)
500537

538+
SELECT count(*) from test__int WHERE a @@ '20 | !21';
539+
count
540+
-------
541+
6566
542+
(1 row)
543+
544+
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
545+
count
546+
-------
547+
6343
548+
(1 row)
549+
501550
DROP INDEX text_idx;
502551
CREATE INDEX text_idx on test__int using gist ( a gist__intbig_ops );
503552
SELECT count(*) from test__int WHERE a && '{23,50}';
@@ -530,6 +579,18 @@ SELECT count(*) from test__int WHERE a @> '{20,23}';
530579
12
531580
(1 row)
532581

582+
SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
583+
count
584+
-------
585+
10
586+
(1 row)
587+
588+
SELECT count(*) from test__int WHERE a = '{73,23,20}';
589+
count
590+
-------
591+
1
592+
(1 row)
593+
533594
SELECT count(*) from test__int WHERE a @@ '50&68';
534595
count
535596
-------
@@ -548,6 +609,18 @@ SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
548609
21
549610
(1 row)
550611

612+
SELECT count(*) from test__int WHERE a @@ '20 | !21';
613+
count
614+
-------
615+
6566
616+
(1 row)
617+
618+
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
619+
count
620+
-------
621+
6343
622+
(1 row)
623+
551624
DROP INDEX text_idx;
552625
CREATE INDEX text_idx on test__int using gin ( a gin__int_ops );
553626
SELECT count(*) from test__int WHERE a && '{23,50}';
@@ -580,6 +653,18 @@ SELECT count(*) from test__int WHERE a @> '{20,23}';
580653
12
581654
(1 row)
582655

656+
SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
657+
count
658+
-------
659+
10
660+
(1 row)
661+
662+
SELECT count(*) from test__int WHERE a = '{73,23,20}';
663+
count
664+
-------
665+
1
666+
(1 row)
667+
583668
SELECT count(*) from test__int WHERE a @@ '50&68';
584669
count
585670
-------
@@ -598,3 +683,16 @@ SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
598683
21
599684
(1 row)
600685

686+
SELECT count(*) from test__int WHERE a @@ '20 | !21';
687+
count
688+
-------
689+
6566
690+
(1 row)
691+
692+
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
693+
count
694+
-------
695+
6343
696+
(1 row)
697+
698+
RESET enable_seqscan;

‎contrib/intarray/sql/_int.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,15 @@ SELECT count(*) from test__int WHERE a @@ '23|50';
8585
SELECT count(*) from test__int WHERE a @> '{23,50}';
8686
SELECT count(*) from test__int WHERE a @@ '23&50';
8787
SELECT count(*) from test__int WHERE a @> '{20,23}';
88+
SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
89+
SELECT count(*) from test__int WHERE a = '{73,23,20}';
8890
SELECT count(*) from test__int WHERE a @@ '50&68';
8991
SELECT count(*) from test__int WHERE a @> '{20,23}' or a @> '{50,68}';
9092
SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
93+
SELECT count(*) from test__int WHERE a @@ '20 | !21';
94+
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
95+
96+
SET enable_seqscan = off; -- not all of these would use index by default
9197

9298
CREATE INDEX text_idx on test__int using gist ( a gist__int_ops );
9399

@@ -96,9 +102,13 @@ SELECT count(*) from test__int WHERE a @@ '23|50';
96102
SELECT count(*) from test__int WHERE a @> '{23,50}';
97103
SELECT count(*) from test__int WHERE a @@ '23&50';
98104
SELECT count(*) from test__int WHERE a @> '{20,23}';
105+
SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
106+
SELECT count(*) from test__int WHERE a = '{73,23,20}';
99107
SELECT count(*) from test__int WHERE a @@ '50&68';
100108
SELECT count(*) from test__int WHERE a @> '{20,23}' or a @> '{50,68}';
101109
SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
110+
SELECT count(*) from test__int WHERE a @@ '20 | !21';
111+
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
102112

103113
DROP INDEX text_idx;
104114
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';
108118
SELECT count(*) from test__int WHERE a @> '{23,50}';
109119
SELECT count(*) from test__int WHERE a @@ '23&50';
110120
SELECT count(*) from test__int WHERE a @> '{20,23}';
121+
SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
122+
SELECT count(*) from test__int WHERE a = '{73,23,20}';
111123
SELECT count(*) from test__int WHERE a @@ '50&68';
112124
SELECT count(*) from test__int WHERE a @> '{20,23}' or a @> '{50,68}';
113125
SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
126+
SELECT count(*) from test__int WHERE a @@ '20 | !21';
127+
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
114128

115129
DROP INDEX text_idx;
116130
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';
120134
SELECT count(*) from test__int WHERE a @> '{23,50}';
121135
SELECT count(*) from test__int WHERE a @@ '23&50';
122136
SELECT count(*) from test__int WHERE a @> '{20,23}';
137+
SELECT count(*) from test__int WHERE a <@ '{73,23,20}';
138+
SELECT count(*) from test__int WHERE a = '{73,23,20}';
123139
SELECT count(*) from test__int WHERE a @@ '50&68';
124140
SELECT count(*) from test__int WHERE a @> '{20,23}' or a @> '{50,68}';
125141
SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)';
142+
SELECT count(*) from test__int WHERE a @@ '20 | !21';
143+
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
144+
145+
RESET enable_seqscan;

0 commit comments

Comments
 (0)