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

Commit0912fb3

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 parent7b15522 commit0912fb3

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
@@ -801,9 +801,8 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
801801
/* A bitmap result */
802802
BlockNumberadvancePastBlk=GinItemPointerGetBlockNumber(&advancePast);
803803
OffsetNumberadvancePastOff=GinItemPointerGetOffsetNumber(&advancePast);
804-
boolgotitem= false;
805804

806-
do
805+
for (;;)
807806
{
808807
/*
809808
* If we've exhausted all items on this block, move to next block
@@ -852,7 +851,6 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
852851
* estimate number of results on this page to support correct
853852
* reducing of result even if it's enabled.
854853
*/
855-
gotitem= true;
856854
break;
857855
}
858856

@@ -865,7 +863,7 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
865863
/*
866864
* First, do a quick check against the last offset on the
867865
* page. If that's > advancePast, so are all the other
868-
* offsets.
866+
* offsets, so just go back to the top to get the next page.
869867
*/
870868
if (entry->matchResult->offsets[entry->matchResult->ntuples-1] <=advancePastOff)
871869
{
@@ -882,16 +880,19 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
882880
entry->matchResult->blockno,
883881
entry->matchResult->offsets[entry->offset]);
884882
entry->offset++;
885-
gotitem= true;
886-
}while (!gotitem|| (entry->reduceResult== true&&dropItem(entry)));
883+
884+
/* Done unless we need to reduce the result */
885+
if (!entry->reduceResult|| !dropItem(entry))
886+
break;
887+
}
887888
}
888889
elseif (!BufferIsValid(entry->buffer))
889890
{
890891
/*
891892
* A posting list from an entry tuple, or the last page of a posting
892893
* tree.
893894
*/
894-
do
895+
for (;;)
895896
{
896897
if (entry->offset >=entry->nlist)
897898
{
@@ -901,13 +902,20 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
901902
}
902903

903904
entry->curItem=entry->list[entry->offset++];
904-
}while (ginCompareItemPointers(&entry->curItem,&advancePast) <=0);
905-
/* XXX: shouldn't we apply the fuzzy search limit here? */
905+
906+
/* If we're not past advancePast, keep scanning */
907+
if (ginCompareItemPointers(&entry->curItem,&advancePast) <=0)
908+
continue;
909+
910+
/* Done unless we need to reduce the result */
911+
if (!entry->reduceResult|| !dropItem(entry))
912+
break;
913+
}
906914
}
907915
else
908916
{
909917
/* A posting tree */
910-
do
918+
for (;;)
911919
{
912920
/* If we've processed the current batch, load more items */
913921
while (entry->offset >=entry->nlist)
@@ -923,8 +931,20 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
923931

924932
entry->curItem=entry->list[entry->offset++];
925933

926-
}while (ginCompareItemPointers(&entry->curItem,&advancePast) <=0||
927-
(entry->reduceResult== true&&dropItem(entry)));
934+
/* If we're not past advancePast, keep scanning */
935+
if (ginCompareItemPointers(&entry->curItem,&advancePast) <=0)
936+
continue;
937+
938+
/* Done unless we need to reduce the result */
939+
if (!entry->reduceResult|| !dropItem(entry))
940+
break;
941+
942+
/*
943+
* Advance advancePast (so that entryLoadMoreItems will load the
944+
* right data), and keep scanning
945+
*/
946+
advancePast=entry->curItem;
947+
}
928948
}
929949
}
930950

‎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