postgres.git / summary / log / commit / refs

commit    5b3e6c13f753cb10b476bf7755b5622aef2fbdce
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Tue Aug 06 22:04:51 2019 +0000
First released in: 13.0, 9.4.25, 9.5.20, 9.6.16, 10.11, 11.6, 12.0

    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 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 | 9 +++- contrib/intarray/_intbig_gist.c | 8 +++- contrib/intarray/expected/_int.out | 98 ++++++++++++++++++++++++++++++++++++++ contrib/intarray/sql/_int.sql | 20 ++++++++ 4 files changed, 132 insertions(+), 3 deletions(-) diff --git a/contrib/intarray/_int_gist.c b/contrib/intarray/_int_gist.c index b2a84a9b68b..b2f5a3addf6 100644 --- a/contrib/intarray/_int_gist.c +++ b/contrib/intarray/_int_gist.c @@ -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; diff --git a/contrib/intarray/_intbig_gist.c b/contrib/intarray/_intbig_gist.c index 6dae7c91c12..f3c1ab764dc 100644 --- a/contrib/intarray/_intbig_gist.c +++ b/contrib/intarray/_intbig_gist.c @@ -591,7 +591,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; diff --git a/contrib/intarray/expected/_int.out b/contrib/intarray/expected/_int.out index 0a5dd463acb..77f827b1f68 100644 --- a/contrib/intarray/expected/_int.out +++ b/contrib/intarray/expected/_int.out @@ -407,6 +407,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 ------- @@ -425,6 +437,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 @@ -456,6 +481,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 ------- @@ -474,6 +511,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}'; @@ -506,6 +555,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 ------- @@ -524,6 +585,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}'; @@ -556,6 +629,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 ------- @@ -574,3 +659,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; diff --git a/contrib/intarray/sql/_int.sql b/contrib/intarray/sql/_int.sql index 44e1a729b4f..fd3be8f271c 100644 --- a/contrib/intarray/sql/_int.sql +++ b/contrib/intarray/sql/_int.sql @@ -81,9 +81,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 ); @@ -92,9 +98,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 ); @@ -104,9 +114,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 ); @@ -116,6 +130,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; [parent: 2fbecb66c45e]