Thread: Index-only scan returns incorrect results when using a compositeGIST index with a gist_trgm_ops column.
Index-only scan returns incorrect results when using a compositeGIST index with a gist_trgm_ops column.
From
David Pereiro Lagares
Date:
Hi,
I was playing a bit with different types of indexes when I noticed that
I was getting an incorrect result for composite GIST indexes if the
first column of the index uses pg_trgm options and the execution plan is
an index only scan.
Here are the (verbose) steps to reproduce the problem in an empty
database:
Setup:
root=# SELECT version();
version
---------------------------------------------------------------
9.6.4 on x86_64-pc-linux-gnu, compiled by gcc (Debian 6.3.0-18)
6.3.0 20170516, 64-bit
(1 fila)
CREATE EXTENSION IF NOT EXISTS pg_trgm;
CREATE EXTENSION IF NOT EXISTS btree_gist;
CREATE TABLE words ( id SERIAL PRIMARY KEY, w VARCHAR );
INSERT INTO words (w) VALUES ('Lorem'), ('ipsum');
Queries that make a seq scan yield correct results:
root=# SELECT w FROM words WHERE w LIKE '%e%';
w
-------
Lorem
(1 fila)
root=# EXPLAIN ANALYZE SELECT w FROM words WHERE w LIKE '%e%';
QUERY PLAN
--------------------------------------------------------------
Seq Scan on words (cost=0.00..1.02 rows=2 width=6) (actual
time=0.018..0.020 rows=1 loops=1)
Filter: ((w)::text ~~ '%e%'::text)
Rows Removed by Filter: 1
Planning time: 0.112 ms
Execution time: 0.040 ms
(5 filas)
Index scan with simple index works fine also:
root=# SET enable_seqscan = OFF;
SET
root=# CREATE INDEX ON words USING GIST(w gist_trgm_ops);
CREATE INDEX
root=# SELECT w FROM words WHERE w LIKE '%e%';
w
-------
Lorem
(1 fila)
root=# EXPLAIN ANALYZE SELECT w FROM words WHERE w LIKE '%e%';
QUERY
PLAN
------------------------------------------------------------------
Index Scan using words_w_idx on words (cost=0.13..8.16 rows=2
width=32) (actual time=0.053..0.054 rows=1 loops=1)
Index Cond: ((w)::text ~~ '%e%'::text)
Rows Removed by Index Recheck: 1
Planning time: 0.101 ms
Execution time: 0.114 ms
(5 filas)
Queries that use the index only scan return no results:
root=# CREATE INDEX ON words USING GIST(w gist_trgm_ops, w);
CREATE INDEX
root=# VACUUM ANALYZE words;
VACUUM
root=# SELECT w FROM words WHERE w LIKE '%e%';
w
---
(0 filas)
root=# EXPLAIN ANALYZE SELECT w FROM words WHERE w LIKE '%e%';
QUERY
PLAN
-------------------------------------------------------------------- Index Only Scan using words_w_w1_idx on
words (cost=0.13..4.16 rows=2 width=6) (actual time=0.043..0.043 rows=0 loops=1)
Index Cond: (w ~~ '%e%'::text)
Rows Removed by Index Recheck: 2
Heap Fetches: 0
Planning time: 0.114 ms
Execution time: 0.103 ms
(6 filas)
Thank you for your help.
Regards.
Attachment
Re: Index-only scan returns incorrect results when using a composite GIST index with a gist_trgm_ops column.
From
Sergei Kornilov
Date:
Hello I can reproduce on actual 9.6.6, 10.1 and fresh master build (9c7d06d60680c7f00d931233873dee81fdb311c6 commit). I did notcheck earlier versions set enable_indexonlyscan to off ; postgres=# SELECT w FROM words WHERE w LIKE '%e%'; w ------- Lorem Index scan result is correct. Affected only index only scan, PS: i find GIST(w gist_trgm_ops, w); some strange idea, but result is incorrect in any case. best regards, Sergei
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
Hello, Gist imposes the ninth strategy to perform index only scan but planner is not considering that. At Wed, 17 Jan 2018 22:26:15 +0300, Sergei Kornilov <sk@zsrv.org> wrote in <412861516217175@web38o.yandex.ru> > Hello > I can reproduce on actual 9.6.6, 10.1 and fresh master build > (9c7d06d60680c7f00d931233873dee81fdb311c6 commit). I did not check > earlier versions > > set enable_indexonlyscan to off ; > postgres=# SELECT w FROM words WHERE w LIKE '%e%'; > w > ------- > Lorem > Index scan result is correct. Affected only index only scan, > > PS: i find GIST(w gist_trgm_ops, w); some strange idea, but result > is incorrect in any case. The cause is that gist_trgm_ops lacks "fetch" method but planner is failing to find that. https://www.postgresql.org/docs/10/static/gist-extensibility.html > The optional ninth method fetch is needed if the operator class > wishes to support index-only scans. Index only scan is not usable in the case since the first index column cannot be rechecked but check_index_only makes wrong decision by the second occurance of "w'. There may be a chance that recheck is not required but we cannot predict that until actually acquire a tuple during execution. Please find the attached patch. regards, -- Kyotaro Horiguchi NTT Open Source Software Center *** a/src/backend/optimizer/path/indxpath.c --- b/src/backend/optimizer/path/indxpath.c *************** *** 1866,1871 **** check_index_only(RelOptInfo *rel, IndexOptInfo *index) --- 1866,1872 ---- bool result; Bitmapset *attrs_used = NULL; Bitmapset *index_canreturn_attrs = NULL; + Bitmapset *index_cannotreturn_attrs = NULL; ListCell *lc; int i; *************** *** 1905,1911 **** check_index_only(RelOptInfo *rel, IndexOptInfo *index) /* * Construct a bitmapset of columns that the index can return back in an ! * index-only scan. */ for (i = 0; i < index->ncolumns; i++) { --- 1906,1913 ---- /* * Construct a bitmapset of columns that the index can return back in an ! * index-only scan. We must have a value for all occurances of the same ! * attribute since it can be used for rechecking. */ for (i = 0; i < index->ncolumns; i++) { *************** *** 1922,1932 **** check_index_only(RelOptInfo *rel, IndexOptInfo *index) --- 1924,1942 ---- index_canreturn_attrs = bms_add_member(index_canreturn_attrs, attno - FirstLowInvalidHeapAttributeNumber); + else + index_cannotreturn_attrs = + bms_add_member(index_cannotreturn_attrs, + attno - FirstLowInvalidHeapAttributeNumber); } + index_canreturn_attrs = bms_del_members(index_canreturn_attrs, + index_cannotreturn_attrs); + /* Do we have all the necessary attributes? */ result = bms_is_subset(attrs_used, index_canreturn_attrs); + bms_free(attrs_used); bms_free(index_canreturn_attrs);
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
Hello, Gist imposes the ninth strategy to perform index only scan but planner is not considering that. At Wed, 17 Jan 2018 22:26:15 +0300, Sergei Kornilov <sk@zsrv.org> wrote in <412861516217175@web38o.yandex.ru> > Hello > I can reproduce on actual 9.6.6, 10.1 and fresh master build > (9c7d06d60680c7f00d931233873dee81fdb311c6 commit). I did not check > earlier versions > > set enable_indexonlyscan to off ; > postgres=# SELECT w FROM words WHERE w LIKE '%e%'; > w > ------- > Lorem > Index scan result is correct. Affected only index only scan, > > PS: i find GIST(w gist_trgm_ops, w); some strange idea, but result > is incorrect in any case. The cause is that gist_trgm_ops lacks "fetch" method but planner is failing to find that. https://www.postgresql.org/docs/10/static/gist-extensibility.html > The optional ninth method fetch is needed if the operator class > wishes to support index-only scans. Index only scan is not usable in the case since the first index column cannot be rechecked but check_index_only makes wrong decision by the second occurance of "w'. There may be a chance that recheck is not required but we cannot predict that until actually acquire a tuple during execution. Please find the attached patch. regards, -- Kyotaro Horiguchi NTT Open Source Software Center *** a/src/backend/optimizer/path/indxpath.c --- b/src/backend/optimizer/path/indxpath.c *************** *** 1866,1871 **** check_index_only(RelOptInfo *rel, IndexOptInfo *index) --- 1866,1872 ---- bool result; Bitmapset *attrs_used = NULL; Bitmapset *index_canreturn_attrs = NULL; + Bitmapset *index_cannotreturn_attrs = NULL; ListCell *lc; int i; *************** *** 1905,1911 **** check_index_only(RelOptInfo *rel, IndexOptInfo *index) /* * Construct a bitmapset of columns that the index can return back in an ! * index-only scan. */ for (i = 0; i < index->ncolumns; i++) { --- 1906,1913 ---- /* * Construct a bitmapset of columns that the index can return back in an ! * index-only scan. We must have a value for all occurances of the same ! * attribute since it can be used for rechecking. */ for (i = 0; i < index->ncolumns; i++) { *************** *** 1922,1932 **** check_index_only(RelOptInfo *rel, IndexOptInfo *index) --- 1924,1942 ---- index_canreturn_attrs = bms_add_member(index_canreturn_attrs, attno - FirstLowInvalidHeapAttributeNumber); + else + index_cannotreturn_attrs = + bms_add_member(index_cannotreturn_attrs, + attno - FirstLowInvalidHeapAttributeNumber); } + index_canreturn_attrs = bms_del_members(index_canreturn_attrs, + index_cannotreturn_attrs); + /* Do we have all the necessary attributes? */ result = bms_is_subset(attrs_used, index_canreturn_attrs); + bms_free(attrs_used); bms_free(index_canreturn_attrs);
Re: Index-only scan returns incorrect results when using a compositeGIST index with a gist_trgm_ops column.
From
Andrey Borodin
Date:
Hello! > 18 янв. 2018 г., в 10:48, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> написал(а): > > Gist imposes the ninth strategy to perform index only scan but > planner is not considering that > .... > Please find the attached patch. I agree with you that current behavior is a bug and your patch seems correct. I'm a bit worried about ninth strategy words: fetch is not necessary now, if opclass lacks compress methods - index onlyscan is possible. See https://github.com/postgres/postgres/commit/d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128 for details. Though there are tests in cube and seg for that, if your patch passes check-world, than this behavior is not affected. Thank you for the patch! Best regards, Andrey Borodin.
Re: Index-only scan returns incorrect results when using a compositeGIST index with a gist_trgm_ops column.
From
Andrey Borodin
Date:
Hello! > 18 янв. 2018 г., в 10:48, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> написал(а): > > Gist imposes the ninth strategy to perform index only scan but > planner is not considering that > .... > Please find the attached patch. I agree with you that current behavior is a bug and your patch seems correct. I'm a bit worried about ninth strategy words: fetch is not necessary now, if opclass lacks compress methods - index onlyscan is possible. See https://github.com/postgres/postgres/commit/d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128 for details. Though there are tests in cube and seg for that, if your patch passes check-world, than this behavior is not affected. Thank you for the patch! Best regards, Andrey Borodin.
Re: Index-only scan returns incorrect results when using a compositeGIST index with a gist_trgm_ops column.
From
Michael Paquier
Date:
On Thu, Jan 18, 2018 at 12:57:38PM +0500, Andrey Borodin wrote: >> 18 янв. 2018 г., в 10:48, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> написал(а): >> >> Gist imposes the ninth strategy to perform index only scan but >> planner is not considering that >> .... >> Please find the attached patch. > I agree with you that current behavior is a bug and your patch seems correct. > I'm a bit worried about ninth strategy words: fetch is not necessary >> now, if opclass lacks compress methods - index only scan is >> possible. See >> https://github.com/postgres/postgres/commit/d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128 >> for details. > > Though there are tests in cube and seg for that, if your patch passes > check-world, than this behavior is not affected. The proposed patch has no regression tests. If the current set is not enough to stress the problem, you surely should add some (haven't checked the patch in detail, sorry ;p ). -- Michael
Attachment
Re: Index-only scan returns incorrect results when using a compositeGIST index with a gist_trgm_ops column.
From
Michael Paquier
Date:
On Thu, Jan 18, 2018 at 12:57:38PM +0500, Andrey Borodin wrote: >> 18 янв. 2018 г., в 10:48, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> написал(а): >> >> Gist imposes the ninth strategy to perform index only scan but >> planner is not considering that >> .... >> Please find the attached patch. > I agree with you that current behavior is a bug and your patch seems correct. > I'm a bit worried about ninth strategy words: fetch is not necessary >> now, if opclass lacks compress methods - index only scan is >> possible. See >> https://github.com/postgres/postgres/commit/d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128 >> for details. > > Though there are tests in cube and seg for that, if your patch passes > check-world, than this behavior is not affected. The proposed patch has no regression tests. If the current set is not enough to stress the problem, you surely should add some (haven't checked the patch in detail, sorry ;p ). -- Michael
Attachment
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
At Thu, 18 Jan 2018 12:57:38 +0500, Andrey Borodin <x4mmm@yandex-team.ru> wrote in <62C2B9A0-51BC-40FF-9BCA-F203784CB9C1@yandex-team.ru> > Hello! > > 18 янв. 2018 г., в 10:48, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> написал(а): > > > > Gist imposes the ninth strategy to perform index only scan but > > planner is not considering that > > .... > > Please find the attached patch. > I agree with you that current behavior is a bug and your patch seems correct. > I'm a bit worried about ninth strategy words: fetch is not necessary now, if opclass lacks compress methods - index onlyscan is possible. See https://github.com/postgres/postgres/commit/d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128 for details. It is true. I don't think that fetch for trigm is feasible but it is workable if created. > Though there are tests in cube and seg for that, if your patch passes check-world, than this behavior is not affected. regards, -- Kyotaro Horiguchi NTT Open Source Software Center
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
At Thu, 18 Jan 2018 12:57:38 +0500, Andrey Borodin <x4mmm@yandex-team.ru> wrote in <62C2B9A0-51BC-40FF-9BCA-F203784CB9C1@yandex-team.ru> > Hello! > > 18 янв. 2018 г., в 10:48, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> написал(а): > > > > Gist imposes the ninth strategy to perform index only scan but > > planner is not considering that > > .... > > Please find the attached patch. > I agree with you that current behavior is a bug and your patch seems correct. > I'm a bit worried about ninth strategy words: fetch is not necessary now, if opclass lacks compress methods - index onlyscan is possible. See https://github.com/postgres/postgres/commit/d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128 for details. It is true. I don't think that fetch for trigm is feasible but it is workable if created. > Though there are tests in cube and seg for that, if your patch passes check-world, than this behavior is not affected. regards, -- Kyotaro Horiguchi NTT Open Source Software Center
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
Hello. At Thu, 18 Jan 2018 17:25:05 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <20180118082505.GA84508@paquier.xyz> > On Thu, Jan 18, 2018 at 12:57:38PM +0500, Andrey Borodin wrote: > >> Please find the attached patch. > > I agree with you that current behavior is a bug and your patch seems correct. > > I'm a bit worried about ninth strategy words: fetch is not necessary > >> now, if opclass lacks compress methods - index only scan is > >> possible. See > >> https://github.com/postgres/postgres/commit/d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128 > >> for details. > > > > Though there are tests in cube and seg for that, if your patch passes > > check-world, than this behavior is not affected. > > The proposed patch has no regression tests. If the current set is not > enough to stress the problem, you surely should add some (haven't > checked the patch in detail, sorry ;p ). Uggg.. I'm beaten again.. You're definitely right! It was a bit hard to find the way to cause the failure without extension but the first attached file is that. regards, -- Kyotaro Horiguchi NTT Open Source Software Center From dc496cba91feb8cb3aea4438337c98efdfac9b8c Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp> Date: Thu, 18 Jan 2018 20:57:13 +0900 Subject: [PATCH 1/2] Regression test for the failure of check_index_only. check_index_only forgets the case where two or more index columns on the same table column but with different operator class. --- src/test/regress/expected/create_index.out | 41 ++++++++++++++++++++++++++++++ src/test/regress/sql/create_index.sql | 26 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 031a0bc..1d107f6 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2990,6 +2990,47 @@ explain (costs off) (4 rows) -- +-- Check for duplicate indexkey with different opclasses +-- +-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST +CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS + OPERATOR 3 &&, + FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal), + FUNCTION 2 inet_gist_union (internal, internal), + FUNCTION 3 inet_gist_compress (internal), + FUNCTION 5 inet_gist_penalty (internal, internal, internal), + FUNCTION 6 inet_gist_picksplit (internal, internal), + FUNCTION 7 inet_gist_same (inet, inet, internal); +CREATE TABLE t (a inet); +CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops); +INSERT INTO t VALUES ('192.168.0.1'); +VACUUM t; +SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value + a +------------- + 192.168.0.1 +(1 row) + +-- enfoce GiST +SET enable_seqscan TO false; +SET enable_bitmapscan TO false; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index onlyscan + QUERY PLAN +---------------------------------------------------------- + Index Scan using t_a_a1_idx on t (actual rows=1 loops=1) + Index Cond: (a && '192.168.0.1'::inet) +(2 rows) + +SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value + a +------------- + 192.168.0.1 +(1 row) + +-- cleanup +DROP TABLE t; +DROP OPERATOR CLASS test_inet_ops USING gist; +-- -- REINDEX (VERBOSE) -- CREATE TABLE reindex_verbose(id integer primary key); diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index a45e8eb..e8016f4 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -1026,6 +1026,32 @@ explain (costs off) select * from boolindex where not b order by i limit 10; -- +-- Check for duplicate indexkey with different opclasses +-- +-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST +CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS + OPERATOR 3 &&, + FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal), + FUNCTION 2 inet_gist_union (internal, internal), + FUNCTION 3 inet_gist_compress (internal), + FUNCTION 5 inet_gist_penalty (internal, internal, internal), + FUNCTION 6 inet_gist_picksplit (internal, internal), + FUNCTION 7 inet_gist_same (inet, inet, internal); +CREATE TABLE t (a inet); +CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops); +INSERT INTO t VALUES ('192.168.0.1'); +VACUUM t; +SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value +-- enfoce GiST +SET enable_seqscan TO false; +SET enable_bitmapscan TO false; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index onlyscan +SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value +-- cleanup +DROP TABLE t; +DROP OPERATOR CLASS test_inet_ops USING gist; + +-- -- REINDEX (VERBOSE) -- CREATE TABLE reindex_verbose(id integer primary key); -- 2.9.2 From d95fcda94f1ef39be6b46deb64a441e053d05f31 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp> Date: Thu, 18 Jan 2018 21:04:18 +0900 Subject: [PATCH 2/2] Fix check_index_only for the case of duplicate keycolumn If there is an index that have multiple keys on the same attribute but using different operator class, only the last key on the same attribute affects the availability of the attribute. This causes failure of rechecking index tuples. An attribute should be considered as unavailable if any key on it cannot return the attribute. --- src/backend/optimizer/path/indxpath.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 7fc7080..2fede1d 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1866,6 +1866,7 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index) bool result; Bitmapset *attrs_used = NULL; Bitmapset *index_canreturn_attrs = NULL; + Bitmapset *index_cannotreturn_attrs = NULL; ListCell *lc; int i; @@ -1905,7 +1906,8 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index) /* * Construct a bitmapset of columns that the index can return back in an - * index-only scan. + * index-only scan. We must have a value for all occurances of the same + * attribute since it can be used for rechecking. */ for (i = 0; i < index->ncolumns; i++) { @@ -1922,11 +1924,19 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index) index_canreturn_attrs = bms_add_member(index_canreturn_attrs, attno - FirstLowInvalidHeapAttributeNumber); + else + index_cannotreturn_attrs = + bms_add_member(index_cannotreturn_attrs, + attno - FirstLowInvalidHeapAttributeNumber); } + index_canreturn_attrs = bms_del_members(index_canreturn_attrs, + index_cannotreturn_attrs); + /* Do we have all the necessary attributes? */ result = bms_is_subset(attrs_used, index_canreturn_attrs); + bms_free(attrs_used); bms_free(index_canreturn_attrs); -- 2.9.2
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
Hello. At Thu, 18 Jan 2018 17:25:05 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <20180118082505.GA84508@paquier.xyz> > On Thu, Jan 18, 2018 at 12:57:38PM +0500, Andrey Borodin wrote: > >> Please find the attached patch. > > I agree with you that current behavior is a bug and your patch seems correct. > > I'm a bit worried about ninth strategy words: fetch is not necessary > >> now, if opclass lacks compress methods - index only scan is > >> possible. See > >> https://github.com/postgres/postgres/commit/d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128 > >> for details. > > > > Though there are tests in cube and seg for that, if your patch passes > > check-world, than this behavior is not affected. > > The proposed patch has no regression tests. If the current set is not > enough to stress the problem, you surely should add some (haven't > checked the patch in detail, sorry ;p ). Uggg.. I'm beaten again.. You're definitely right! It was a bit hard to find the way to cause the failure without extension but the first attached file is that. regards, -- Kyotaro Horiguchi NTT Open Source Software Center From dc496cba91feb8cb3aea4438337c98efdfac9b8c Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp> Date: Thu, 18 Jan 2018 20:57:13 +0900 Subject: [PATCH 1/2] Regression test for the failure of check_index_only. check_index_only forgets the case where two or more index columns on the same table column but with different operator class. --- src/test/regress/expected/create_index.out | 41 ++++++++++++++++++++++++++++++ src/test/regress/sql/create_index.sql | 26 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 031a0bc..1d107f6 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2990,6 +2990,47 @@ explain (costs off) (4 rows) -- +-- Check for duplicate indexkey with different opclasses +-- +-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST +CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS + OPERATOR 3 &&, + FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal), + FUNCTION 2 inet_gist_union (internal, internal), + FUNCTION 3 inet_gist_compress (internal), + FUNCTION 5 inet_gist_penalty (internal, internal, internal), + FUNCTION 6 inet_gist_picksplit (internal, internal), + FUNCTION 7 inet_gist_same (inet, inet, internal); +CREATE TABLE t (a inet); +CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops); +INSERT INTO t VALUES ('192.168.0.1'); +VACUUM t; +SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value + a +------------- + 192.168.0.1 +(1 row) + +-- enfoce GiST +SET enable_seqscan TO false; +SET enable_bitmapscan TO false; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index onlyscan + QUERY PLAN +---------------------------------------------------------- + Index Scan using t_a_a1_idx on t (actual rows=1 loops=1) + Index Cond: (a && '192.168.0.1'::inet) +(2 rows) + +SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value + a +------------- + 192.168.0.1 +(1 row) + +-- cleanup +DROP TABLE t; +DROP OPERATOR CLASS test_inet_ops USING gist; +-- -- REINDEX (VERBOSE) -- CREATE TABLE reindex_verbose(id integer primary key); diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index a45e8eb..e8016f4 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -1026,6 +1026,32 @@ explain (costs off) select * from boolindex where not b order by i limit 10; -- +-- Check for duplicate indexkey with different opclasses +-- +-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST +CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS + OPERATOR 3 &&, + FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal), + FUNCTION 2 inet_gist_union (internal, internal), + FUNCTION 3 inet_gist_compress (internal), + FUNCTION 5 inet_gist_penalty (internal, internal, internal), + FUNCTION 6 inet_gist_picksplit (internal, internal), + FUNCTION 7 inet_gist_same (inet, inet, internal); +CREATE TABLE t (a inet); +CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops); +INSERT INTO t VALUES ('192.168.0.1'); +VACUUM t; +SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value +-- enfoce GiST +SET enable_seqscan TO false; +SET enable_bitmapscan TO false; +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index onlyscan +SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value +-- cleanup +DROP TABLE t; +DROP OPERATOR CLASS test_inet_ops USING gist; + +-- -- REINDEX (VERBOSE) -- CREATE TABLE reindex_verbose(id integer primary key); -- 2.9.2 From d95fcda94f1ef39be6b46deb64a441e053d05f31 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp> Date: Thu, 18 Jan 2018 21:04:18 +0900 Subject: [PATCH 2/2] Fix check_index_only for the case of duplicate keycolumn If there is an index that have multiple keys on the same attribute but using different operator class, only the last key on the same attribute affects the availability of the attribute. This causes failure of rechecking index tuples. An attribute should be considered as unavailable if any key on it cannot return the attribute. --- src/backend/optimizer/path/indxpath.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 7fc7080..2fede1d 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1866,6 +1866,7 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index) bool result; Bitmapset *attrs_used = NULL; Bitmapset *index_canreturn_attrs = NULL; + Bitmapset *index_cannotreturn_attrs = NULL; ListCell *lc; int i; @@ -1905,7 +1906,8 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index) /* * Construct a bitmapset of columns that the index can return back in an - * index-only scan. + * index-only scan. We must have a value for all occurances of the same + * attribute since it can be used for rechecking. */ for (i = 0; i < index->ncolumns; i++) { @@ -1922,11 +1924,19 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index) index_canreturn_attrs = bms_add_member(index_canreturn_attrs, attno - FirstLowInvalidHeapAttributeNumber); + else + index_cannotreturn_attrs = + bms_add_member(index_cannotreturn_attrs, + attno - FirstLowInvalidHeapAttributeNumber); } + index_canreturn_attrs = bms_del_members(index_canreturn_attrs, + index_cannotreturn_attrs); + /* Do we have all the necessary attributes? */ result = bms_is_subset(attrs_used, index_canreturn_attrs); + bms_free(attrs_used); bms_free(index_canreturn_attrs); -- 2.9.2
Re: Index-only scan returns incorrect results when using a compositeGIST index with a gist_trgm_ops column.
From
Alexander Korotkov
Date:
On Thu, Jan 18, 2018 at 8:48 AM, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
I didn't get this point. Same column is indexed twice using two
At Wed, 17 Jan 2018 22:26:15 +0300, Sergei Kornilov <sk@zsrv.org>
wrote in <412861516217175@web38o.yandex.ru>
> Hello
> I can reproduce on actual 9.6.6, 10.1 and fresh master build
> (9c7d06d60680c7f00d931233873dee 81fdb311c6 commit). I did not check
> earlier versions
>
> set enable_indexonlyscan to off ;
> postgres=# SELECT w FROM words WHERE w LIKE '%e%';
> w
> -------
> Lorem
> Index scan result is correct. Affected only index only scan,
>
> PS: i find GIST(w gist_trgm_ops, w); some strange idea, but result
> is incorrect in any case.
The cause is that gist_trgm_ops lacks "fetch" method but planner
is failing to find that.
https://www.postgresql.org/docs/10/static/gist- extensibility.html
> The optional ninth method fetch is needed if the operator class
> wishes to support index-only scans.
Index only scan is not usable in the case since the first index
column cannot be rechecked but check_index_only makes wrong
decision by the second occurance of "w'. There may be a chance
that recheck is not required but we cannot predict that until
actually acquire a tuple during execution.
different opclasses. The first opclass doesn't support index-only scan,
while the second opclass does support it. So, if the first opclass needs
recheck then it can be rechecked using the value got from the second
opclass. Thus, I think we can potentially support index-only scan
in this case.
Another thing is that it's probably hard to do in our current optimizer/executor/am
infrastructure. And assuming that use-case is quite narrow, and it looks
OK to just disable such feature as bug fix.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Re: Index-only scan returns incorrect results when using a compositeGIST index with a gist_trgm_ops column.
From
Alexander Korotkov
Date:
On Thu, Jan 18, 2018 at 8:48 AM, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
I didn't get this point. Same column is indexed twice using two
At Wed, 17 Jan 2018 22:26:15 +0300, Sergei Kornilov <sk@zsrv.org>
wrote in <412861516217175@web38o.yandex.ru>
> Hello
> I can reproduce on actual 9.6.6, 10.1 and fresh master build
> (9c7d06d60680c7f00d931233873dee 81fdb311c6 commit). I did not check
> earlier versions
>
> set enable_indexonlyscan to off ;
> postgres=# SELECT w FROM words WHERE w LIKE '%e%';
> w
> -------
> Lorem
> Index scan result is correct. Affected only index only scan,
>
> PS: i find GIST(w gist_trgm_ops, w); some strange idea, but result
> is incorrect in any case.
The cause is that gist_trgm_ops lacks "fetch" method but planner
is failing to find that.
https://www.postgresql.org/docs/10/static/gist- extensibility.html
> The optional ninth method fetch is needed if the operator class
> wishes to support index-only scans.
Index only scan is not usable in the case since the first index
column cannot be rechecked but check_index_only makes wrong
decision by the second occurance of "w'. There may be a chance
that recheck is not required but we cannot predict that until
actually acquire a tuple during execution.
different opclasses. The first opclass doesn't support index-only scan,
while the second opclass does support it. So, if the first opclass needs
recheck then it can be rechecked using the value got from the second
opclass. Thus, I think we can potentially support index-only scan
in this case.
Another thing is that it's probably hard to do in our current optimizer/executor/am
infrastructure. And assuming that use-case is quite narrow, and it looks
OK to just disable such feature as bug fix.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
Hello. At Fri, 19 Jan 2018 01:16:56 +0300, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote in <CAPpHfdvJeYrnAsXhKRfO_NMDUaWQyK+wyhcv4zOdRzTdfNCkkw@mail.gmail.com> > On Thu, Jan 18, 2018 at 8:48 AM, Kyotaro HORIGUCHI < > horiguchi.kyotaro@lab.ntt.co.jp> wrote: > > Index only scan is not usable in the case since the first index > > column cannot be rechecked but check_index_only makes wrong > > decision by the second occurance of "w'. There may be a chance > > that recheck is not required but we cannot predict that until > > actually acquire a tuple during execution. > > > > I didn't get this point. Same column is indexed twice using two > different opclasses. The first opclass doesn't support index-only scan, > while the second opclass does support it. So, if the first opclass needs > recheck then it can be rechecked using the value got from the second > opclass. Thus, I think we can potentially support index-only scan > in this case. > Another thing is that it's probably hard to do in our current > optimizer/executor/am > infrastructure. And assuming that use-case is quite narrow, and it looks To be exact, you're right. My description was abridged by omitting exactly what you mentioned above. The complexity needed to allow that doesn't seem worth adding. > OK to just disable such feature as bug fix. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
Hello. At Fri, 19 Jan 2018 01:16:56 +0300, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote in <CAPpHfdvJeYrnAsXhKRfO_NMDUaWQyK+wyhcv4zOdRzTdfNCkkw@mail.gmail.com> > On Thu, Jan 18, 2018 at 8:48 AM, Kyotaro HORIGUCHI < > horiguchi.kyotaro@lab.ntt.co.jp> wrote: > > Index only scan is not usable in the case since the first index > > column cannot be rechecked but check_index_only makes wrong > > decision by the second occurance of "w'. There may be a chance > > that recheck is not required but we cannot predict that until > > actually acquire a tuple during execution. > > > > I didn't get this point. Same column is indexed twice using two > different opclasses. The first opclass doesn't support index-only scan, > while the second opclass does support it. So, if the first opclass needs > recheck then it can be rechecked using the value got from the second > opclass. Thus, I think we can potentially support index-only scan > in this case. > Another thing is that it's probably hard to do in our current > optimizer/executor/am > infrastructure. And assuming that use-case is quite narrow, and it looks To be exact, you're right. My description was abridged by omitting exactly what you mentioned above. The complexity needed to allow that doesn't seem worth adding. > OK to just disable such feature as bug fix. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: Index-only scan returns incorrect results when using a composite GIST index with a gist_trgm_ops column.
From
Sergei Kornilov
Date:
Hello I tested this patch and think it can be commited to master. Is there a CF record? I can not find one. Should we also make backport to older versions? I test on REL_10_STABLE - patch builds and works ok, but "make check" failson new testcase with error: > CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops); >+ ERROR: missing support function 4 for attribute 1 of index "t_a_a1_idx" and with different explain results. regards, Sergei
Re: Index-only scan returns incorrect results when using a composite GIST index with a gist_trgm_ops column.
From
Sergei Kornilov
Date:
Hello I tested this patch and think it can be commited to master. Is there a CF record? I can not find one. Should we also make backport to older versions? I test on REL_10_STABLE - patch builds and works ok, but "make check" failson new testcase with error: > CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops); >+ ERROR: missing support function 4 for attribute 1 of index "t_a_a1_idx" and with different explain results. regards, Sergei
Re: Index-only scan returns incorrect results when using a compositeGIST index with a gist_trgm_ops column.
From
Andrey Borodin
Date:
Hi! > 24 янв. 2018 г., в 2:13, Sergei Kornilov <sk@zsrv.org> написал(а): > > Should we also make backport to older versions? I test on REL_10_STABLE - patch builds and works ok, but "make check" failson new testcase with error: >> CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops); >> + ERROR: missing support function 4 for attribute 1 of index "t_a_a1_idx" > and with different explain results. To be usable for 10 the patch must use full-featured opclass in tests: there is no decompress GiST function in test_inet_opswhich was required before 11. I think, explain results will be identical. Best regards, Andrey Borodin.
Re: Index-only scan returns incorrect results when using a compositeGIST index with a gist_trgm_ops column.
From
Andrey Borodin
Date:
Hi! > 24 янв. 2018 г., в 2:13, Sergei Kornilov <sk@zsrv.org> написал(а): > > Should we also make backport to older versions? I test on REL_10_STABLE - patch builds and works ok, but "make check" failson new testcase with error: >> CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops); >> + ERROR: missing support function 4 for attribute 1 of index "t_a_a1_idx" > and with different explain results. To be usable for 10 the patch must use full-featured opclass in tests: there is no decompress GiST function in test_inet_opswhich was required before 11. I think, explain results will be identical. Best regards, Andrey Borodin.
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
Thank you for looking this.
At Wed, 24 Jan 2018 00:13:51 +0300, Sergei Kornilov <sk@zsrv.org> wrote in <348951516742031@web54j.yandex.ru>
> Hello
> I tested this patch and think it can be commited to master. Is there a CF record? I can not find one.
Not yet. I'm thinking of creating an entry in the next CF after
waiting someone to pickup this.
> Should we also make backport to older versions? I test on REL_10_STABLE - patch builds and works ok, but "make check"
failson new testcase with error:
> > CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
> >+ ERROR: missing support function 4 for attribute 1 of index "t_a_a1_idx"
> and with different explain results.
Thank you for checking that. d3a4f89 allowed that and
inet_gist_decompress is removed at the same time. Unfortunately I
didn't find a type on master that has both decompress and fetch
methods so I prefer to split the regression patch among target
versions.
master has its own one. 9.6 and 10 share the same patch. 9.5 has
a different one since the signature of consistent method has
changed as of 9.6. This is not required for 9.4 and earlier since
they don't check canreturn on attribute basis. (And they don't
try index-only on GiST.)
On the other hand the fix patch applies to all affected versions.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 0e221b3396215d67167622cbc07d04b5185d6f66 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Wed, 24 Jan 2018 15:25:48 +0900
Subject: [PATCH] Regression test for the failure of check_index_only.
check_index_only forgets the case where two or more index columns on
the same table column but with different operator class.
---
src/test/regress/expected/create_index.out | 42 ++++++++++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 27 +++++++++++++++++++
2 files changed, 69 insertions(+)
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 9c0f3e3..d95157c 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2913,6 +2913,48 @@ explain (costs off)
(2 rows)
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, integer, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 4 inet_gist_decompress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+ QUERY PLAN
+------------------------------------------
+ Index Scan using t_a_a1_idx on t
+ Index Cond: (a && '192.168.0.1'::inet)
+(2 rows)
+
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index b199c23..95a5060 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -982,6 +982,33 @@ explain (costs off)
select * from tenk1 where (thousand, tenthous) in ((1,1001), (null,null));
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, integer, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 4 inet_gist_decompress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
--
2.9.2
From 0b734a1950077aee1f43b3d3141bde8e8eeb2778 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Wed, 24 Jan 2018 14:54:56 +0900
Subject: [PATCH] Regression test for the failure of check_index_only.
check_index_only forgets the case where two or more index columns on
the same table column but with different operator class.
---
src/test/regress/expected/create_index.out | 42 ++++++++++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 27 +++++++++++++++++++
2 files changed, 69 insertions(+)
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 064adb4..483522e 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2990,6 +2990,48 @@ explain (costs off)
(4 rows)
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 4 inet_gist_decompress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+ QUERY PLAN
+------------------------------------------
+ Index Scan using t_a_a1_idx on t
+ Index Cond: (a && '192.168.0.1'::inet)
+(2 rows)
+
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 67470db..4d58ef0 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1026,6 +1026,33 @@ explain (costs off)
select * from boolindex where not b order by i limit 10;
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 4 inet_gist_decompress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
--
2.9.2
From 4bbd78bc8d3219c2be7d388553e6c9f23b242a05 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Jan 2018 20:57:13 +0900
Subject: [PATCH 1/2] Regression test for the failure of check_index_only.
check_index_only forgets the case where two or more index columns on
the same table column but with different operator class.
---
src/test/regress/expected/create_index.out | 41 ++++++++++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 26 +++++++++++++++++++
2 files changed, 67 insertions(+)
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 031a0bc..065aad4 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2990,6 +2990,47 @@ explain (costs off)
(4 rows)
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+ QUERY PLAN
+------------------------------------------
+ Index Scan using t_a_a1_idx on t
+ Index Cond: (a && '192.168.0.1'::inet)
+(2 rows)
+
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index a45e8eb..642ac33 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1026,6 +1026,32 @@ explain (costs off)
select * from boolindex where not b order by i limit 10;
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
--
2.9.2
From fec0283e4b8ea5b692c82a2abeca782b64e3d5ad Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Jan 2018 21:04:18 +0900
Subject: [PATCH 2/2] Fix check_index_only for the case of duplicate keycolumn
If there is an index that have multiple keys on the same attribute but
using different operator class, only the last key on the same
attribute affects the availability of the attribute. This causes
failure of rechecking index tuples. An attribute should be considered
as unavailable if any key on it cannot return the attribute.
---
src/backend/optimizer/path/indxpath.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 7fc7080..2fede1d 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1866,6 +1866,7 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
bool result;
Bitmapset *attrs_used = NULL;
Bitmapset *index_canreturn_attrs = NULL;
+ Bitmapset *index_cannotreturn_attrs = NULL;
ListCell *lc;
int i;
@@ -1905,7 +1906,8 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
/*
* Construct a bitmapset of columns that the index can return back in an
- * index-only scan.
+ * index-only scan. We must have a value for all occurances of the same
+ * attribute since it can be used for rechecking.
*/
for (i = 0; i < index->ncolumns; i++)
{
@@ -1922,11 +1924,19 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
index_canreturn_attrs =
bms_add_member(index_canreturn_attrs,
attno - FirstLowInvalidHeapAttributeNumber);
+ else
+ index_cannotreturn_attrs =
+ bms_add_member(index_cannotreturn_attrs,
+ attno - FirstLowInvalidHeapAttributeNumber);
}
+ index_canreturn_attrs = bms_del_members(index_canreturn_attrs,
+ index_cannotreturn_attrs);
+
/* Do we have all the necessary attributes? */
result = bms_is_subset(attrs_used, index_canreturn_attrs);
+
bms_free(attrs_used);
bms_free(index_canreturn_attrs);
--
2.9.2
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
Thank you for looking this.
At Wed, 24 Jan 2018 00:13:51 +0300, Sergei Kornilov <sk@zsrv.org> wrote in <348951516742031@web54j.yandex.ru>
> Hello
> I tested this patch and think it can be commited to master. Is there a CF record? I can not find one.
Not yet. I'm thinking of creating an entry in the next CF after
waiting someone to pickup this.
> Should we also make backport to older versions? I test on REL_10_STABLE - patch builds and works ok, but "make check"
failson new testcase with error:
> > CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
> >+ ERROR: missing support function 4 for attribute 1 of index "t_a_a1_idx"
> and with different explain results.
Thank you for checking that. d3a4f89 allowed that and
inet_gist_decompress is removed at the same time. Unfortunately I
didn't find a type on master that has both decompress and fetch
methods so I prefer to split the regression patch among target
versions.
master has its own one. 9.6 and 10 share the same patch. 9.5 has
a different one since the signature of consistent method has
changed as of 9.6. This is not required for 9.4 and earlier since
they don't check canreturn on attribute basis. (And they don't
try index-only on GiST.)
On the other hand the fix patch applies to all affected versions.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 0e221b3396215d67167622cbc07d04b5185d6f66 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Wed, 24 Jan 2018 15:25:48 +0900
Subject: [PATCH] Regression test for the failure of check_index_only.
check_index_only forgets the case where two or more index columns on
the same table column but with different operator class.
---
src/test/regress/expected/create_index.out | 42 ++++++++++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 27 +++++++++++++++++++
2 files changed, 69 insertions(+)
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 9c0f3e3..d95157c 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2913,6 +2913,48 @@ explain (costs off)
(2 rows)
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, integer, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 4 inet_gist_decompress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+ QUERY PLAN
+------------------------------------------
+ Index Scan using t_a_a1_idx on t
+ Index Cond: (a && '192.168.0.1'::inet)
+(2 rows)
+
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index b199c23..95a5060 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -982,6 +982,33 @@ explain (costs off)
select * from tenk1 where (thousand, tenthous) in ((1,1001), (null,null));
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, integer, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 4 inet_gist_decompress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
--
2.9.2
From 0b734a1950077aee1f43b3d3141bde8e8eeb2778 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Wed, 24 Jan 2018 14:54:56 +0900
Subject: [PATCH] Regression test for the failure of check_index_only.
check_index_only forgets the case where two or more index columns on
the same table column but with different operator class.
---
src/test/regress/expected/create_index.out | 42 ++++++++++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 27 +++++++++++++++++++
2 files changed, 69 insertions(+)
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 064adb4..483522e 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2990,6 +2990,48 @@ explain (costs off)
(4 rows)
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 4 inet_gist_decompress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+ QUERY PLAN
+------------------------------------------
+ Index Scan using t_a_a1_idx on t
+ Index Cond: (a && '192.168.0.1'::inet)
+(2 rows)
+
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 67470db..4d58ef0 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1026,6 +1026,33 @@ explain (costs off)
select * from boolindex where not b order by i limit 10;
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 4 inet_gist_decompress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
--
2.9.2
From 4bbd78bc8d3219c2be7d388553e6c9f23b242a05 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Jan 2018 20:57:13 +0900
Subject: [PATCH 1/2] Regression test for the failure of check_index_only.
check_index_only forgets the case where two or more index columns on
the same table column but with different operator class.
---
src/test/regress/expected/create_index.out | 41 ++++++++++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 26 +++++++++++++++++++
2 files changed, 67 insertions(+)
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 031a0bc..065aad4 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2990,6 +2990,47 @@ explain (costs off)
(4 rows)
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+ QUERY PLAN
+------------------------------------------
+ Index Scan using t_a_a1_idx on t
+ Index Cond: (a && '192.168.0.1'::inet)
+(2 rows)
+
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+ a
+-------------
+ 192.168.0.1
+(1 row)
+
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index a45e8eb..642ac33 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1026,6 +1026,32 @@ explain (costs off)
select * from boolindex where not b order by i limit 10;
--
+-- Check for duplicate indexkey with different opclasses
+--
+-- opclass that doesn't have function 9 (GIST_FETCH_PROC) for GiST
+CREATE OPERATOR CLASS test_inet_ops FOR TYPE inet USING gist AS
+ OPERATOR 3 &&,
+ FUNCTION 1 inet_gist_consistent (internal, inet, smallint, oid, internal),
+ FUNCTION 2 inet_gist_union (internal, internal),
+ FUNCTION 3 inet_gist_compress (internal),
+ FUNCTION 5 inet_gist_penalty (internal, internal, internal),
+ FUNCTION 6 inet_gist_picksplit (internal, internal),
+ FUNCTION 7 inet_gist_same (inet, inet, internal);
+CREATE TABLE t (a inet);
+CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
+INSERT INTO t VALUES ('192.168.0.1');
+VACUUM t;
+SELECT * FROM t WHERE a && '192.168.0.1'; -- should retuan a value
+-- enfoce GiST
+SET enable_seqscan TO false;
+SET enable_bitmapscan TO false;
+EXPLAIN (COSTS OFF) SELECT * FROM t WHERE a && '192.168.0.1'; -- shoudn't be index only scan
+SELECT * FROM t WHERE a && '192.168.0.1'; -- also should return a value
+-- cleanup
+DROP TABLE t;
+DROP OPERATOR CLASS test_inet_ops USING gist;
+
+--
-- REINDEX (VERBOSE)
--
CREATE TABLE reindex_verbose(id integer primary key);
--
2.9.2
From fec0283e4b8ea5b692c82a2abeca782b64e3d5ad Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Jan 2018 21:04:18 +0900
Subject: [PATCH 2/2] Fix check_index_only for the case of duplicate keycolumn
If there is an index that have multiple keys on the same attribute but
using different operator class, only the last key on the same
attribute affects the availability of the attribute. This causes
failure of rechecking index tuples. An attribute should be considered
as unavailable if any key on it cannot return the attribute.
---
src/backend/optimizer/path/indxpath.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 7fc7080..2fede1d 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1866,6 +1866,7 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
bool result;
Bitmapset *attrs_used = NULL;
Bitmapset *index_canreturn_attrs = NULL;
+ Bitmapset *index_cannotreturn_attrs = NULL;
ListCell *lc;
int i;
@@ -1905,7 +1906,8 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
/*
* Construct a bitmapset of columns that the index can return back in an
- * index-only scan.
+ * index-only scan. We must have a value for all occurances of the same
+ * attribute since it can be used for rechecking.
*/
for (i = 0; i < index->ncolumns; i++)
{
@@ -1922,11 +1924,19 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
index_canreturn_attrs =
bms_add_member(index_canreturn_attrs,
attno - FirstLowInvalidHeapAttributeNumber);
+ else
+ index_cannotreturn_attrs =
+ bms_add_member(index_cannotreturn_attrs,
+ attno - FirstLowInvalidHeapAttributeNumber);
}
+ index_canreturn_attrs = bms_del_members(index_canreturn_attrs,
+ index_cannotreturn_attrs);
+
/* Do we have all the necessary attributes? */
result = bms_is_subset(attrs_used, index_canreturn_attrs);
+
bms_free(attrs_used);
bms_free(index_canreturn_attrs);
--
2.9.2
Re: Index-only scan returns incorrect results when using a composite GIST index with a gist_trgm_ops column.
From
Tom Lane
Date:
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> At Wed, 24 Jan 2018 00:13:51 +0300, Sergei Kornilov <sk@zsrv.org> wrote in <348951516742031@web54j.yandex.ru>
>> Should we also make backport to older versions? I test on REL_10_STABLE - patch builds and works ok, but "make
check"fails on new testcase with error:
> CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
> + ERROR: missing support function 4 for attribute 1 of index "t_a_a1_idx"
>> and with different explain results.
> Thank you for checking that. d3a4f89 allowed that and
> inet_gist_decompress is removed at the same time. Unfortunately I
> didn't find a type on master that has both decompress and fetch
> methods so I prefer to split the regression patch among target
> versions.
I pushed this fix with minor adjustments. I did not like the proposed
regression test at all: it was overcomplicated and the need for different
versions for different back branches wasn't fun either. After some poking
around I found that the bug could be exhibited using just btree_gist's
gist_inet_ops, since the core inet_ops class indexes the same datatype and
it does have a fetch function. So I added a test case in btree_gist.
regards, tom lane
Re: Index-only scan returns incorrect results when using a composite GIST index with a gist_trgm_ops column.
From
Tom Lane
Date:
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> At Wed, 24 Jan 2018 00:13:51 +0300, Sergei Kornilov <sk@zsrv.org> wrote in <348951516742031@web54j.yandex.ru>
>> Should we also make backport to older versions? I test on REL_10_STABLE - patch builds and works ok, but "make
check"fails on new testcase with error:
> CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops);
> + ERROR: missing support function 4 for attribute 1 of index "t_a_a1_idx"
>> and with different explain results.
> Thank you for checking that. d3a4f89 allowed that and
> inet_gist_decompress is removed at the same time. Unfortunately I
> didn't find a type on master that has both decompress and fetch
> methods so I prefer to split the regression patch among target
> versions.
I pushed this fix with minor adjustments. I did not like the proposed
regression test at all: it was overcomplicated and the need for different
versions for different back branches wasn't fun either. After some poking
around I found that the bug could be exhibited using just btree_gist's
gist_inet_ops, since the core inet_ops class indexes the same datatype and
it does have a fetch function. So I added a test case in btree_gist.
regards, tom lane
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
At Thu, 01 Mar 2018 15:39:18 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in <22609.1519936758@sss.pgh.pa.us> > Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes: > > At Wed, 24 Jan 2018 00:13:51 +0300, Sergei Kornilov <sk@zsrv.org> wrote in <348951516742031@web54j.yandex.ru> > >> Should we also make backport to older versions? I test on REL_10_STABLE - patch builds and works ok, but "make check"fails on new testcase with error: > > CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops); > > + ERROR: missing support function 4 for attribute 1 of index "t_a_a1_idx" > >> and with different explain results. > > > Thank you for checking that. d3a4f89 allowed that and > > inet_gist_decompress is removed at the same time. Unfortunately I > > didn't find a type on master that has both decompress and fetch > > methods so I prefer to split the regression patch among target > > versions. > > I pushed this fix with minor adjustments. I did not like the proposed Thank you. > regression test at all: it was overcomplicated and the need for different > versions for different back branches wasn't fun either. After some poking > around I found that the bug could be exhibited using just btree_gist's > gist_inet_ops, since the core inet_ops class indexes the same datatype and > it does have a fetch function. So I added a test case in btree_gist. Ah, It wasn't in my sight to test core in contrib. Thanks for improving it. regards, -- Kyotaro Horiguchi NTT Open Source Software Center
Re: Index-only scan returns incorrect results when using acomposite GIST index with a gist_trgm_ops column.
From
Kyotaro HORIGUCHI
Date:
At Thu, 01 Mar 2018 15:39:18 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in <22609.1519936758@sss.pgh.pa.us> > Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes: > > At Wed, 24 Jan 2018 00:13:51 +0300, Sergei Kornilov <sk@zsrv.org> wrote in <348951516742031@web54j.yandex.ru> > >> Should we also make backport to older versions? I test on REL_10_STABLE - patch builds and works ok, but "make check"fails on new testcase with error: > > CREATE INDEX ON t USING gist (a test_inet_ops, a inet_ops); > > + ERROR: missing support function 4 for attribute 1 of index "t_a_a1_idx" > >> and with different explain results. > > > Thank you for checking that. d3a4f89 allowed that and > > inet_gist_decompress is removed at the same time. Unfortunately I > > didn't find a type on master that has both decompress and fetch > > methods so I prefer to split the regression patch among target > > versions. > > I pushed this fix with minor adjustments. I did not like the proposed Thank you. > regression test at all: it was overcomplicated and the need for different > versions for different back branches wasn't fun either. After some poking > around I found that the bug could be exhibited using just btree_gist's > gist_inet_ops, since the core inet_ops class indexes the same datatype and > it does have a fetch function. So I added a test case in btree_gist. Ah, It wasn't in my sight to test core in contrib. Thanks for improving it. regards, -- Kyotaro Horiguchi NTT Open Source Software Center