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

Commit 1771ec9

Browse files
committed
Avoid trying to fetch metapage of an SPGist partitioned index.
This is necessary when spgcanreturn() is invoked on a partitioned index, and the failure might be reachable in other scenarios as well. The rest of what spgGetCache() does is perfectly sensible for a partitioned index, so we should allow it to go through. I think the main takeaway from this is that we lack sufficient test coverage for non-btree partitioned indexes. Therefore, I added simple test cases for brin and gin as well as spgist (hash and gist AMs were covered already in indexing.sql). Per bug #18256 from Alexander Lakhin. Although the known test case only fails since v16 (3c56904), I've got no faith at all that there aren't other ways to reach this problem; so back-patch to all supported branches. Discussion: https://postgr.es/m/18256-0b0e1b6e4a620f1b@postgresql.org
1 parent 3cf7dfa commit 1771ec9

File tree

3 files changed

+77
-11
lines changed

3 files changed

+77
-11
lines changed

src/backend/access/spgist/spgutils.c

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,6 @@ spgGetCache(Relation index)
105105
Oid atttype;
106106
spgConfigIn in;
107107
FmgrInfo *procinfo;
108-
Buffer metabuffer;
109-
SpGistMetaPageData *metadata;
110108

111109
cache = MemoryContextAllocZero(index->rd_indexcxt,
112110
sizeof(SpGistCache));
@@ -151,19 +149,28 @@ spgGetCache(Relation index)
151149
fillTypeDesc(&cache->attPrefixType, cache->config.prefixType);
152150
fillTypeDesc(&cache->attLabelType, cache->config.labelType);
153151

154-
/* Last, get the lastUsedPages data from the metapage */
155-
metabuffer = ReadBuffer(index, SPGIST_METAPAGE_BLKNO);
156-
LockBuffer(metabuffer, BUFFER_LOCK_SHARE);
152+
/*
153+
* Finally, if it's a real index (not a partitioned one), get the
154+
* lastUsedPages data from the metapage
155+
*/
156+
if (index->rd_rel->relkind != RELKIND_PARTITIONED_INDEX)
157+
{
158+
Buffer metabuffer;
159+
SpGistMetaPageData *metadata;
160+
161+
metabuffer = ReadBuffer(index, SPGIST_METAPAGE_BLKNO);
162+
LockBuffer(metabuffer, BUFFER_LOCK_SHARE);
157163

158-
metadata = SpGistPageGetMeta(BufferGetPage(metabuffer));
164+
metadata = SpGistPageGetMeta(BufferGetPage(metabuffer));
159165

160-
if (metadata->magicNumber != SPGIST_MAGIC_NUMBER)
161-
elog(ERROR, "index \"%s\" is not an SP-GiST index",
162-
RelationGetRelationName(index));
166+
if (metadata->magicNumber != SPGIST_MAGIC_NUMBER)
167+
elog(ERROR, "index \"%s\" is not an SP-GiST index",
168+
RelationGetRelationName(index));
163169

164-
cache->lastUsedPages = metadata->lastUsedPages;
170+
cache->lastUsedPages = metadata->lastUsedPages;
165171

166-
UnlockReleaseBuffer(metabuffer);
172+
UnlockReleaseBuffer(metabuffer);
173+
}
167174

168175
index->rd_amcache = (void *) cache;
169176
}

src/test/regress/expected/indexing.out

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,6 +1279,45 @@ select tableoid::regclass, * from idxpart order by a;
12791279
idxpart2 | 857142 | six
12801280
(8 rows)
12811281

1282+
drop table idxpart;
1283+
-- Test some other non-btree index types
1284+
create table idxpart (a int, b text, c int[]) partition by range (a);
1285+
create table idxpart1 partition of idxpart for values from (0) to (100000);
1286+
set enable_seqscan to off;
1287+
create index idxpart_brin on idxpart using brin(b);
1288+
explain (costs off) select * from idxpart where b = 'abcd';
1289+
QUERY PLAN
1290+
-------------------------------------------
1291+
Bitmap Heap Scan on idxpart1
1292+
Recheck Cond: (b = 'abcd'::text)
1293+
-> Bitmap Index Scan on idxpart1_b_idx
1294+
Index Cond: (b = 'abcd'::text)
1295+
(4 rows)
1296+
1297+
drop index idxpart_brin;
1298+
create index idxpart_spgist on idxpart using spgist(b);
1299+
explain (costs off) select * from idxpart where b = 'abcd';
1300+
QUERY PLAN
1301+
-------------------------------------------
1302+
Bitmap Heap Scan on idxpart1
1303+
Recheck Cond: (b = 'abcd'::text)
1304+
-> Bitmap Index Scan on idxpart1_b_idx
1305+
Index Cond: (b = 'abcd'::text)
1306+
(4 rows)
1307+
1308+
drop index idxpart_spgist;
1309+
create index idxpart_gin on idxpart using gin(c);
1310+
explain (costs off) select * from idxpart where c @> array[42];
1311+
QUERY PLAN
1312+
----------------------------------------------
1313+
Bitmap Heap Scan on idxpart1
1314+
Recheck Cond: (c @> '{42}'::integer[])
1315+
-> Bitmap Index Scan on idxpart1_c_idx
1316+
Index Cond: (c @> '{42}'::integer[])
1317+
(4 rows)
1318+
1319+
drop index idxpart_gin;
1320+
reset enable_seqscan;
12821321
drop table idxpart;
12831322
-- intentionally leave some objects around
12841323
create table idxpart (a int) partition by range (a);

src/test/regress/sql/indexing.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,26 @@ insert into idxpart values (857142, 'six');
668668
select tableoid::regclass, * from idxpart order by a;
669669
drop table idxpart;
670670

671+
-- Test some other non-btree index types
672+
create table idxpart (a int, b text, c int[]) partition by range (a);
673+
create table idxpart1 partition of idxpart for values from (0) to (100000);
674+
set enable_seqscan to off;
675+
676+
create index idxpart_brin on idxpart using brin(b);
677+
explain (costs off) select * from idxpart where b = 'abcd';
678+
drop index idxpart_brin;
679+
680+
create index idxpart_spgist on idxpart using spgist(b);
681+
explain (costs off) select * from idxpart where b = 'abcd';
682+
drop index idxpart_spgist;
683+
684+
create index idxpart_gin on idxpart using gin(c);
685+
explain (costs off) select * from idxpart where c @> array[42];
686+
drop index idxpart_gin;
687+
688+
reset enable_seqscan;
689+
drop table idxpart;
690+
671691
-- intentionally leave some objects around
672692
create table idxpart (a int) partition by range (a);
673693
create table idxpart1 partition of idxpart for values from (0) to (100);

0 commit comments

Comments
 (0)