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

Commit 0977bd6

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 7f07384 commit 0977bd6

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
@@ -186,8 +186,6 @@ spgGetCache(Relation index)
186186
Oid atttype;
187187
spgConfigIn in;
188188
FmgrInfo *procinfo;
189-
Buffer metabuffer;
190-
SpGistMetaPageData *metadata;
191189

192190
cache = MemoryContextAllocZero(index->rd_indexcxt,
193191
sizeof(SpGistCache));
@@ -255,19 +253,28 @@ spgGetCache(Relation index)
255253
fillTypeDesc(&cache->attPrefixType, cache->config.prefixType);
256254
fillTypeDesc(&cache->attLabelType, cache->config.labelType);
257255

258-
/* Last, get the lastUsedPages data from the metapage */
259-
metabuffer = ReadBuffer(index, SPGIST_METAPAGE_BLKNO);
260-
LockBuffer(metabuffer, BUFFER_LOCK_SHARE);
256+
/*
257+
* Finally, if it's a real index (not a partitioned one), get the
258+
* lastUsedPages data from the metapage
259+
*/
260+
if (index->rd_rel->relkind != RELKIND_PARTITIONED_INDEX)
261+
{
262+
Buffer metabuffer;
263+
SpGistMetaPageData *metadata;
264+
265+
metabuffer = ReadBuffer(index, SPGIST_METAPAGE_BLKNO);
266+
LockBuffer(metabuffer, BUFFER_LOCK_SHARE);
261267

262-
metadata = SpGistPageGetMeta(BufferGetPage(metabuffer));
268+
metadata = SpGistPageGetMeta(BufferGetPage(metabuffer));
263269

264-
if (metadata->magicNumber != SPGIST_MAGIC_NUMBER)
265-
elog(ERROR, "index \"%s\" is not an SP-GiST index",
266-
RelationGetRelationName(index));
270+
if (metadata->magicNumber != SPGIST_MAGIC_NUMBER)
271+
elog(ERROR, "index \"%s\" is not an SP-GiST index",
272+
RelationGetRelationName(index));
267273

268-
cache->lastUsedPages = metadata->lastUsedPages;
274+
cache->lastUsedPages = metadata->lastUsedPages;
269275

270-
UnlockReleaseBuffer(metabuffer);
276+
UnlockReleaseBuffer(metabuffer);
277+
}
271278

272279
index->rd_amcache = (void *) cache;
273280
}

src/test/regress/expected/indexing.out

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

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