Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit44c763f

Browse files
committed
Fix bugs in gin_fuzzy_search_limit processing.
entryGetItem()'s three code paths each contained bugs associatedwith filtering the entries for gin_fuzzy_search_limit.The posting-tree path failed to advance "advancePast" after havingdecided to filter an item. If we ran out of items on the currentpage and needed to advance to the next, what would actually happenis that entryLoadMoreItems() would re-load the same page. Eventually,the random dropItem() test would accept one of the same items it'dpreviously rejected, and we'd move on --- but it could take awhilewith small gin_fuzzy_search_limit. To add insult to injury, thiscase would inevitably cause entryLoadMoreItems() to decide it neededto re-descend from the root, making things even slower.The posting-list path failed to implement gin_fuzzy_search_limitfiltering at all, so that all entries in the posting list wouldbe returned.The bitmap-result path used a "gotitem" variable that it failed toupdate in the one place where it'd actually make a difference, ieat the one "continue" statement. I think this was unreachable inpractice, because if we'd looped around then it shouldn't be thecase that the entries on the new page are before advancePast.Still, the "gotitem" variable was contributing nothing to eitherclarity or correctness, so get rid of it.Refactor all three loops so that the termination conditions aremore alike and less unreadable.The code coverage report showed that we had no coverage at all forthe re-descend-from-root code path in entryLoadMoreItems(), whichseems like a very bad thing, so add a test case that exercises it.We also had exactly no coverage for gin_fuzzy_search_limit, so add asimplistic test case that at least hits those code paths a little bit.Back-patch to all supported branches.Adé Heyward and Tom LaneDiscussion:https://postgr.es/m/CAEknJCdS-dE1Heddptm7ay2xTbSeADbkaQ8bU2AXRCVC2LdtKQ@mail.gmail.com
1 parent85f0d47 commit44c763f

File tree

3 files changed

+88
-14
lines changed

3 files changed

+88
-14
lines changed

‎src/backend/access/gin/ginget.c

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -753,9 +753,8 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
753753
/* A bitmap result */
754754
BlockNumberadvancePastBlk=GinItemPointerGetBlockNumber(&advancePast);
755755
OffsetNumberadvancePastOff=GinItemPointerGetOffsetNumber(&advancePast);
756-
boolgotitem= false;
757756

758-
do
757+
for (;;)
759758
{
760759
/*
761760
* If we've exhausted all items on this block, move to next block
@@ -804,7 +803,6 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
804803
* estimate number of results on this page to support correct
805804
* reducing of result even if it's enabled.
806805
*/
807-
gotitem= true;
808806
break;
809807
}
810808

@@ -817,7 +815,7 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
817815
/*
818816
* First, do a quick check against the last offset on the
819817
* page. If that's > advancePast, so are all the other
820-
* offsets.
818+
* offsets, so just go back to the top to get the next page.
821819
*/
822820
if (entry->matchResult->offsets[entry->matchResult->ntuples-1] <=advancePastOff)
823821
{
@@ -834,16 +832,19 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
834832
entry->matchResult->blockno,
835833
entry->matchResult->offsets[entry->offset]);
836834
entry->offset++;
837-
gotitem= true;
838-
}while (!gotitem|| (entry->reduceResult== TRUE&&dropItem(entry)));
835+
836+
/* Done unless we need to reduce the result */
837+
if (!entry->reduceResult|| !dropItem(entry))
838+
break;
839+
}
839840
}
840841
elseif (!BufferIsValid(entry->buffer))
841842
{
842843
/*
843844
* A posting list from an entry tuple, or the last page of a posting
844845
* tree.
845846
*/
846-
do
847+
for (;;)
847848
{
848849
if (entry->offset >=entry->nlist)
849850
{
@@ -853,13 +854,20 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
853854
}
854855

855856
entry->curItem=entry->list[entry->offset++];
856-
}while (ginCompareItemPointers(&entry->curItem,&advancePast) <=0);
857-
/* XXX: shouldn't we apply the fuzzy search limit here? */
857+
858+
/* If we're not past advancePast, keep scanning */
859+
if (ginCompareItemPointers(&entry->curItem,&advancePast) <=0)
860+
continue;
861+
862+
/* Done unless we need to reduce the result */
863+
if (!entry->reduceResult|| !dropItem(entry))
864+
break;
865+
}
858866
}
859867
else
860868
{
861869
/* A posting tree */
862-
do
870+
for (;;)
863871
{
864872
/* If we've processed the current batch, load more items */
865873
while (entry->offset >=entry->nlist)
@@ -875,8 +883,20 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
875883

876884
entry->curItem=entry->list[entry->offset++];
877885

878-
}while (ginCompareItemPointers(&entry->curItem,&advancePast) <=0||
879-
(entry->reduceResult== TRUE&&dropItem(entry)));
886+
/* If we're not past advancePast, keep scanning */
887+
if (ginCompareItemPointers(&entry->curItem,&advancePast) <=0)
888+
continue;
889+
890+
/* Done unless we need to reduce the result */
891+
if (!entry->reduceResult|| !dropItem(entry))
892+
break;
893+
894+
/*
895+
* Advance advancePast (so that entryLoadMoreItems will load the
896+
* right data), and keep scanning
897+
*/
898+
advancePast=entry->curItem;
899+
}
880900
}
881901
}
882902

‎src/test/regress/expected/gin.out

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
--
22
-- Test GIN indexes.
33
--
4-
-- There are other tests to test different GINopclassed. This is for testing
4+
-- There are other tests to test different GINopclasses. This is for testing
55
-- GIN itself.
66
-- Create and populate a test table with a GIN index.
77
create table gin_test_tbl(i int4[]) with (autovacuum_enabled = off);
@@ -35,3 +35,41 @@ insert into gin_test_tbl select array[1, 2, g] from generate_series(1, 1000) g;
3535
insert into gin_test_tbl select array[1, 3, g] from generate_series(1, 1000) g;
3636
delete from gin_test_tbl where i @> array[2];
3737
vacuum gin_test_tbl;
38+
-- Test for "rare && frequent" searches
39+
explain (costs off)
40+
select count(*) from gin_test_tbl where i @> array[1, 999];
41+
QUERY PLAN
42+
-------------------------------------------------------
43+
Aggregate
44+
-> Bitmap Heap Scan on gin_test_tbl
45+
Recheck Cond: (i @> '{1,999}'::integer[])
46+
-> Bitmap Index Scan on gin_test_idx
47+
Index Cond: (i @> '{1,999}'::integer[])
48+
(5 rows)
49+
50+
select count(*) from gin_test_tbl where i @> array[1, 999];
51+
count
52+
-------
53+
3
54+
(1 row)
55+
56+
-- Very weak test for gin_fuzzy_search_limit
57+
set gin_fuzzy_search_limit = 1000;
58+
explain (costs off)
59+
select count(*) > 0 as ok from gin_test_tbl where i @> array[1];
60+
QUERY PLAN
61+
---------------------------------------------------
62+
Aggregate
63+
-> Bitmap Heap Scan on gin_test_tbl
64+
Recheck Cond: (i @> '{1}'::integer[])
65+
-> Bitmap Index Scan on gin_test_idx
66+
Index Cond: (i @> '{1}'::integer[])
67+
(5 rows)
68+
69+
select count(*) > 0 as ok from gin_test_tbl where i @> array[1];
70+
ok
71+
----
72+
t
73+
(1 row)
74+
75+
reset gin_fuzzy_search_limit;

‎src/test/regress/sql/gin.sql

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
--
22
-- Test GIN indexes.
33
--
4-
-- There are other tests to test different GINopclassed. This is for testing
4+
-- There are other tests to test different GINopclasses. This is for testing
55
-- GIN itself.
66

77
-- Create and populate a test table with a GIN index.
@@ -34,3 +34,19 @@ insert into gin_test_tbl select array[1, 3, g] from generate_series(1, 1000) g;
3434

3535
deletefrom gin_test_tblwhere i @> array[2];
3636
vacuum gin_test_tbl;
37+
38+
-- Test for "rare && frequent" searches
39+
explain (costs off)
40+
selectcount(*)from gin_test_tblwhere i @> array[1,999];
41+
42+
selectcount(*)from gin_test_tblwhere i @> array[1,999];
43+
44+
-- Very weak test for gin_fuzzy_search_limit
45+
set gin_fuzzy_search_limit=1000;
46+
47+
explain (costs off)
48+
selectcount(*)>0as okfrom gin_test_tblwhere i @> array[1];
49+
50+
selectcount(*)>0as okfrom gin_test_tblwhere i @> array[1];
51+
52+
reset gin_fuzzy_search_limit;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp