mirror of
https://github.com/postgres/postgres.git
synced 2025-04-18 13:44:19 +03:00
Fix GIN's shimTriConsistentFn to not corrupt its input.
Commit 0f21db36d made an assumption that GIN triConsistentFns would not modify their input entryRes[] arrays. But in fact, the "shim" triConsistentFn that we use for opclasses that don't supply their own did exactly that, potentially leading to wrong answers from a GIN index search. Through bad luck, none of the test cases that we have for such opclasses exposed the bug. One response to this could be that the assumption of consistency check functions not modifying entryRes[] arrays is a bad one, but it still seems reasonable to me. Notably, shimTriConsistentFn is itself assuming that with respect to the underlying boolean consistentFn, so it's sure being self-centered in supposing that it gets to do so. Fortunately, it's quite simple to fix shimTriConsistentFn to restore the entry-time state of entryRes[], so let's do that instead. This issue doesn't affect any core GIN opclasses, since they all supply their own triConsistentFns. It does affect contrib modules btree_gin, hstore, and intarray. Along the way, I (tgl) noticed that shimTriConsistentFn failed to pick up on a "recheck" flag returned by its first call to the boolean consistentFn. This may be only a latent problem, since it would be unlikely for a consistentFn to set recheck for the all-false case and not any other cases. (Indeed, none of our contrib modules do that.) Nonetheless, it's formally wrong. Reported-by: Vinod Sridharan <vsridh90@gmail.com> Author: Vinod Sridharan <vsridh90@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAFMdLD7XzsXfi1+DpTqTgrD8XU0i2C99KuF=5VHLWjx4C1pkcg@mail.gmail.com Backpatch-through: 13
This commit is contained in:
parent
a6cab6a78e
commit
e708ffe79d
@ -492,6 +492,12 @@ SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
|||||||
6344
|
6344
|
||||||
(1 row)
|
(1 row)
|
||||||
|
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
count
|
||||||
|
-------
|
||||||
|
12
|
||||||
|
(1 row)
|
||||||
|
|
||||||
SET enable_seqscan = off; -- not all of these would use index by default
|
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 );
|
CREATE INDEX text_idx on test__int using gist ( a gist__int_ops );
|
||||||
SELECT count(*) from test__int WHERE a && '{23,50}';
|
SELECT count(*) from test__int WHERE a && '{23,50}';
|
||||||
@ -566,6 +572,12 @@ SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
|||||||
6344
|
6344
|
||||||
(1 row)
|
(1 row)
|
||||||
|
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
count
|
||||||
|
-------
|
||||||
|
12
|
||||||
|
(1 row)
|
||||||
|
|
||||||
INSERT INTO test__int SELECT array(SELECT x FROM generate_series(1, 1001) x); -- should fail
|
INSERT INTO test__int SELECT array(SELECT x FROM generate_series(1, 1001) x); -- should fail
|
||||||
ERROR: input array is too big (199 maximum allowed, 1001 current), use gist__intbig_ops opclass instead
|
ERROR: input array is too big (199 maximum allowed, 1001 current), use gist__intbig_ops opclass instead
|
||||||
DROP INDEX text_idx;
|
DROP INDEX text_idx;
|
||||||
@ -648,6 +660,12 @@ SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
|||||||
6344
|
6344
|
||||||
(1 row)
|
(1 row)
|
||||||
|
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
count
|
||||||
|
-------
|
||||||
|
12
|
||||||
|
(1 row)
|
||||||
|
|
||||||
DROP INDEX text_idx;
|
DROP INDEX text_idx;
|
||||||
CREATE INDEX text_idx on test__int using gist (a gist__intbig_ops(siglen = 0));
|
CREATE INDEX text_idx on test__int using gist (a gist__intbig_ops(siglen = 0));
|
||||||
ERROR: value 0 out of bounds for option "siglen"
|
ERROR: value 0 out of bounds for option "siglen"
|
||||||
@ -728,6 +746,12 @@ SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
|||||||
6344
|
6344
|
||||||
(1 row)
|
(1 row)
|
||||||
|
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
count
|
||||||
|
-------
|
||||||
|
12
|
||||||
|
(1 row)
|
||||||
|
|
||||||
DROP INDEX text_idx;
|
DROP INDEX text_idx;
|
||||||
CREATE INDEX text_idx on test__int using gist ( a gist__intbig_ops );
|
CREATE INDEX text_idx on test__int using gist ( a gist__intbig_ops );
|
||||||
SELECT count(*) from test__int WHERE a && '{23,50}';
|
SELECT count(*) from test__int WHERE a && '{23,50}';
|
||||||
@ -802,6 +826,12 @@ SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
|||||||
6344
|
6344
|
||||||
(1 row)
|
(1 row)
|
||||||
|
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
count
|
||||||
|
-------
|
||||||
|
12
|
||||||
|
(1 row)
|
||||||
|
|
||||||
DROP INDEX text_idx;
|
DROP INDEX text_idx;
|
||||||
CREATE INDEX text_idx on test__int using gin ( a gin__int_ops );
|
CREATE INDEX text_idx on test__int using gin ( a gin__int_ops );
|
||||||
SELECT count(*) from test__int WHERE a && '{23,50}';
|
SELECT count(*) from test__int WHERE a && '{23,50}';
|
||||||
@ -876,6 +906,12 @@ SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
|||||||
6344
|
6344
|
||||||
(1 row)
|
(1 row)
|
||||||
|
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
count
|
||||||
|
-------
|
||||||
|
12
|
||||||
|
(1 row)
|
||||||
|
|
||||||
DROP INDEX text_idx;
|
DROP INDEX text_idx;
|
||||||
-- Repeat the same queries with an extended data set. The data set is the
|
-- Repeat the same queries with an extended data set. The data set is the
|
||||||
-- same that we used before, except that each element in the array is
|
-- same that we used before, except that each element in the array is
|
||||||
@ -968,4 +1004,10 @@ SELECT count(*) from more__int WHERE a @@ '!20 & !21';
|
|||||||
6344
|
6344
|
||||||
(1 row)
|
(1 row)
|
||||||
|
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
count
|
||||||
|
-------
|
||||||
|
12
|
||||||
|
(1 row)
|
||||||
|
|
||||||
RESET enable_seqscan;
|
RESET enable_seqscan;
|
||||||
|
@ -107,6 +107,7 @@ 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&23)|(50&68)';
|
||||||
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
||||||
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
|
||||||
SET enable_seqscan = off; -- not all of these would use index by default
|
SET enable_seqscan = off; -- not all of these would use index by default
|
||||||
|
|
||||||
@ -124,6 +125,7 @@ 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&23)|(50&68)';
|
||||||
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
||||||
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
|
||||||
INSERT INTO test__int SELECT array(SELECT x FROM generate_series(1, 1001) x); -- should fail
|
INSERT INTO test__int SELECT array(SELECT x FROM generate_series(1, 1001) x); -- should fail
|
||||||
|
|
||||||
@ -144,6 +146,7 @@ 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&23)|(50&68)';
|
||||||
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
||||||
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
|
||||||
DROP INDEX text_idx;
|
DROP INDEX text_idx;
|
||||||
CREATE INDEX text_idx on test__int using gist (a gist__intbig_ops(siglen = 0));
|
CREATE INDEX text_idx on test__int using gist (a gist__intbig_ops(siglen = 0));
|
||||||
@ -162,6 +165,7 @@ 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&23)|(50&68)';
|
||||||
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
||||||
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
|
||||||
DROP INDEX text_idx;
|
DROP INDEX text_idx;
|
||||||
CREATE INDEX text_idx on test__int using gist ( a gist__intbig_ops );
|
CREATE INDEX text_idx on test__int using gist ( a gist__intbig_ops );
|
||||||
@ -178,6 +182,7 @@ 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&23)|(50&68)';
|
||||||
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
||||||
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
|
||||||
DROP INDEX text_idx;
|
DROP INDEX text_idx;
|
||||||
CREATE INDEX text_idx on test__int using gin ( a gin__int_ops );
|
CREATE INDEX text_idx on test__int using gin ( a gin__int_ops );
|
||||||
@ -194,6 +199,7 @@ 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&23)|(50&68)';
|
||||||
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
SELECT count(*) from test__int WHERE a @@ '20 | !21';
|
||||||
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
SELECT count(*) from test__int WHERE a @@ '!20 & !21';
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
|
||||||
DROP INDEX text_idx;
|
DROP INDEX text_idx;
|
||||||
|
|
||||||
@ -229,6 +235,7 @@ SELECT count(*) from more__int WHERE a @> '{20,23}' or a @> '{50,68}';
|
|||||||
SELECT count(*) from more__int WHERE a @@ '(20&23)|(50&68)';
|
SELECT count(*) from more__int WHERE a @@ '(20&23)|(50&68)';
|
||||||
SELECT count(*) from more__int WHERE a @@ '20 | !21';
|
SELECT count(*) from more__int WHERE a @@ '20 | !21';
|
||||||
SELECT count(*) from more__int WHERE a @@ '!20 & !21';
|
SELECT count(*) from more__int WHERE a @@ '!20 & !21';
|
||||||
|
SELECT count(*) from test__int WHERE a @@ '!2733 & (2738 | 254)';
|
||||||
|
|
||||||
|
|
||||||
RESET enable_seqscan;
|
RESET enable_seqscan;
|
||||||
|
@ -140,7 +140,9 @@ shimBoolConsistentFn(GinScanKey key)
|
|||||||
* every combination is O(n^2), so this is only feasible for a small number of
|
* every combination is O(n^2), so this is only feasible for a small number of
|
||||||
* MAYBE inputs.
|
* MAYBE inputs.
|
||||||
*
|
*
|
||||||
* NB: This function modifies the key->entryRes array!
|
* NB: This function modifies the key->entryRes array. For now that's okay
|
||||||
|
* so long as we restore the entry-time contents before returning. This may
|
||||||
|
* need revisiting if we ever invent multithreaded GIN scans, though.
|
||||||
*/
|
*/
|
||||||
static GinTernaryValue
|
static GinTernaryValue
|
||||||
shimTriConsistentFn(GinScanKey key)
|
shimTriConsistentFn(GinScanKey key)
|
||||||
@ -149,7 +151,7 @@ shimTriConsistentFn(GinScanKey key)
|
|||||||
int maybeEntries[MAX_MAYBE_ENTRIES];
|
int maybeEntries[MAX_MAYBE_ENTRIES];
|
||||||
int i;
|
int i;
|
||||||
bool boolResult;
|
bool boolResult;
|
||||||
bool recheck = false;
|
bool recheck;
|
||||||
GinTernaryValue curResult;
|
GinTernaryValue curResult;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@ -169,8 +171,8 @@ shimTriConsistentFn(GinScanKey key)
|
|||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* If none of the inputs were MAYBE, so we can just call consistent
|
* If none of the inputs were MAYBE, we can just call the consistent
|
||||||
* function as is.
|
* function as-is.
|
||||||
*/
|
*/
|
||||||
if (nmaybe == 0)
|
if (nmaybe == 0)
|
||||||
return directBoolConsistentFn(key);
|
return directBoolConsistentFn(key);
|
||||||
@ -179,6 +181,7 @@ shimTriConsistentFn(GinScanKey key)
|
|||||||
for (i = 0; i < nmaybe; i++)
|
for (i = 0; i < nmaybe; i++)
|
||||||
key->entryRes[maybeEntries[i]] = GIN_FALSE;
|
key->entryRes[maybeEntries[i]] = GIN_FALSE;
|
||||||
curResult = directBoolConsistentFn(key);
|
curResult = directBoolConsistentFn(key);
|
||||||
|
recheck = key->recheckCurItem;
|
||||||
|
|
||||||
for (;;)
|
for (;;)
|
||||||
{
|
{
|
||||||
@ -200,13 +203,20 @@ shimTriConsistentFn(GinScanKey key)
|
|||||||
recheck |= key->recheckCurItem;
|
recheck |= key->recheckCurItem;
|
||||||
|
|
||||||
if (curResult != boolResult)
|
if (curResult != boolResult)
|
||||||
return GIN_MAYBE;
|
{
|
||||||
|
curResult = GIN_MAYBE;
|
||||||
|
break;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/* TRUE with recheck is taken to mean MAYBE */
|
/* TRUE with recheck is taken to mean MAYBE */
|
||||||
if (curResult == GIN_TRUE && recheck)
|
if (curResult == GIN_TRUE && recheck)
|
||||||
curResult = GIN_MAYBE;
|
curResult = GIN_MAYBE;
|
||||||
|
|
||||||
|
/* We must restore the original state of the entryRes array */
|
||||||
|
for (i = 0; i < nmaybe; i++)
|
||||||
|
key->entryRes[maybeEntries[i]] = GIN_MAYBE;
|
||||||
|
|
||||||
return curResult;
|
return curResult;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user