Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 1df14a5

Browse files
committed
Fix code for re-finding scan position in a multicolumn GIN index.
collectMatchBitmap() needs to re-find the index tuple it was previously looking at, after transiently dropping lock on the index page it's on. The tuple should still exist and be at its prior position or somewhere to the right of that, since ginvacuum never removes tuples but concurrent insertions could add one. However, there was a thinko in that logic, to the effect of expecting any inserted tuples to have the same index "attnum" as what we'd been scanning. Since there's no physical separation of tuples with different attnums, it's not terribly hard to devise scenarios where this fails, leading to transient "lost saved point in index" errors. (While I've duplicated this with manual testing, it seems impossible to make a reproducible test case with our available testing technology.) Fix by just continuing the scan when the attnum doesn't match. While here, improve the error message used if we do fail, so that it matches the wording used in btree for a similar case. collectMatchBitmap()'s posting-tree code path was previously not exercised at all by our regression tests. While I can't make a regression test that exhibits the bug, I can at least improve the code coverage here, so do that. The test case I made for this is an extension of one added by 4b754d6, so it only works in HEAD and v13; didn't seem worth trying hard to back-patch it. Per bug #16595 from Jesse Kinkead. This has been broken since multicolumn capability was added to GIN (commit 27cb66f), so back-patch to all supported branches. Discussion: https://postgr.es/m/16595-633118be8eef9ce2@postgresql.org
1 parent 32c42b8 commit 1df14a5

File tree

3 files changed

+105
-12
lines changed

3 files changed

+105
-12
lines changed

src/backend/access/gin/ginget.c

+16-12
Original file line numberDiff line numberDiff line change
@@ -264,24 +264,28 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack *stack,
264264
/* Search forward to re-find idatum */
265265
for (;;)
266266
{
267-
Datum newDatum;
268-
GinNullCategory newCategory;
269-
270267
if (moveRightIfItNeeded(btree, stack, snapshot) == false)
271-
elog(ERROR, "lost saved point in index"); /* must not happen !!! */
268+
ereport(ERROR,
269+
(errcode(ERRCODE_INTERNAL_ERROR),
270+
errmsg("failed to re-find tuple within index \"%s\"",
271+
RelationGetRelationName(btree->index))));
272272

273273
page = BufferGetPage(stack->buffer);
274274
itup = (IndexTuple) PageGetItem(page, PageGetItemId(page, stack->off));
275275

276-
if (gintuple_get_attrnum(btree->ginstate, itup) != attnum)
277-
elog(ERROR, "lost saved point in index"); /* must not happen !!! */
278-
newDatum = gintuple_get_key(btree->ginstate, itup,
279-
&newCategory);
276+
if (gintuple_get_attrnum(btree->ginstate, itup) == attnum)
277+
{
278+
Datum newDatum;
279+
GinNullCategory newCategory;
280+
281+
newDatum = gintuple_get_key(btree->ginstate, itup,
282+
&newCategory);
280283

281-
if (ginCompareEntries(btree->ginstate, attnum,
282-
newDatum, newCategory,
283-
idatum, icategory) == 0)
284-
break; /* Found! */
284+
if (ginCompareEntries(btree->ginstate, attnum,
285+
newDatum, newCategory,
286+
idatum, icategory) == 0)
287+
break; /* Found! */
288+
}
285289

286290
stack->off++;
287291
}

src/test/regress/expected/gin.out

+65
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,71 @@ from
199199
i @> '{1}' and j @> '{10}' | 2 | 0 | t
200200
(10 rows)
201201

202+
reset enable_seqscan;
203+
reset enable_bitmapscan;
204+
-- re-purpose t_gin_test_tbl to test scans involving posting trees
205+
insert into t_gin_test_tbl select array[1, g, g/10], array[2, g, g/10]
206+
from generate_series(1, 20000) g;
207+
select gin_clean_pending_list('t_gin_test_tbl_i_j_idx') is not null;
208+
?column?
209+
----------
210+
t
211+
(1 row)
212+
213+
analyze t_gin_test_tbl;
214+
set enable_seqscan = off;
215+
set enable_bitmapscan = on;
216+
explain (costs off)
217+
select count(*) from t_gin_test_tbl where j @> array[50];
218+
QUERY PLAN
219+
---------------------------------------------------------
220+
Aggregate
221+
-> Bitmap Heap Scan on t_gin_test_tbl
222+
Recheck Cond: (j @> '{50}'::integer[])
223+
-> Bitmap Index Scan on t_gin_test_tbl_i_j_idx
224+
Index Cond: (j @> '{50}'::integer[])
225+
(5 rows)
226+
227+
select count(*) from t_gin_test_tbl where j @> array[50];
228+
count
229+
-------
230+
11
231+
(1 row)
232+
233+
explain (costs off)
234+
select count(*) from t_gin_test_tbl where j @> array[2];
235+
QUERY PLAN
236+
---------------------------------------------------------
237+
Aggregate
238+
-> Bitmap Heap Scan on t_gin_test_tbl
239+
Recheck Cond: (j @> '{2}'::integer[])
240+
-> Bitmap Index Scan on t_gin_test_tbl_i_j_idx
241+
Index Cond: (j @> '{2}'::integer[])
242+
(5 rows)
243+
244+
select count(*) from t_gin_test_tbl where j @> array[2];
245+
count
246+
-------
247+
20000
248+
(1 row)
249+
250+
explain (costs off)
251+
select count(*) from t_gin_test_tbl where j @> '{}'::int[];
252+
QUERY PLAN
253+
---------------------------------------------------------
254+
Aggregate
255+
-> Bitmap Heap Scan on t_gin_test_tbl
256+
Recheck Cond: (j @> '{}'::integer[])
257+
-> Bitmap Index Scan on t_gin_test_tbl_i_j_idx
258+
Index Cond: (j @> '{}'::integer[])
259+
(5 rows)
260+
261+
select count(*) from t_gin_test_tbl where j @> '{}'::int[];
262+
count
263+
-------
264+
20006
265+
(1 row)
266+
202267
reset enable_seqscan;
203268
reset enable_bitmapscan;
204269
drop table t_gin_test_tbl;

src/test/regress/sql/gin.sql

+24
Original file line numberDiff line numberDiff line change
@@ -138,4 +138,28 @@ from
138138
reset enable_seqscan;
139139
reset enable_bitmapscan;
140140

141+
-- re-purpose t_gin_test_tbl to test scans involving posting trees
142+
insert into t_gin_test_tbl select array[1, g, g/10], array[2, g, g/10]
143+
from generate_series(1, 20000) g;
144+
145+
select gin_clean_pending_list('t_gin_test_tbl_i_j_idx') is not null;
146+
147+
analyze t_gin_test_tbl;
148+
149+
set enable_seqscan = off;
150+
set enable_bitmapscan = on;
151+
152+
explain (costs off)
153+
select count(*) from t_gin_test_tbl where j @> array[50];
154+
select count(*) from t_gin_test_tbl where j @> array[50];
155+
explain (costs off)
156+
select count(*) from t_gin_test_tbl where j @> array[2];
157+
select count(*) from t_gin_test_tbl where j @> array[2];
158+
explain (costs off)
159+
select count(*) from t_gin_test_tbl where j @> '{}'::int[];
160+
select count(*) from t_gin_test_tbl where j @> '{}'::int[];
161+
162+
reset enable_seqscan;
163+
reset enable_bitmapscan;
164+
141165
drop table t_gin_test_tbl;

0 commit comments

Comments
 (0)