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

Commit 903737c

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 0590480 commit 903737c

File tree

3 files changed

+77
-11
lines changed

3 files changed

+77
-11
lines changed

src/backend/access/spgist/spgutils.c

+18-11
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,6 @@ spgGetCache(Relation index)
188188
Oid atttype;
189189
spgConfigIn in;
190190
FmgrInfo *procinfo;
191-
Buffer metabuffer;
192-
SpGistMetaPageData *metadata;
193191

194192
cache = MemoryContextAllocZero(index->rd_indexcxt,
195193
sizeof(SpGistCache));
@@ -257,19 +255,28 @@ spgGetCache(Relation index)
257255
fillTypeDesc(&cache->attPrefixType, cache->config.prefixType);
258256
fillTypeDesc(&cache->attLabelType, cache->config.labelType);
259257

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

264-
metadata = SpGistPageGetMeta(BufferGetPage(metabuffer));
270+
metadata = SpGistPageGetMeta(BufferGetPage(metabuffer));
265271

266-
if (metadata->magicNumber != SPGIST_MAGIC_NUMBER)
267-
elog(ERROR, "index \"%s\" is not an SP-GiST index",
268-
RelationGetRelationName(index));
272+
if (metadata->magicNumber != SPGIST_MAGIC_NUMBER)
273+
elog(ERROR, "index \"%s\" is not an SP-GiST index",
274+
RelationGetRelationName(index));
269275

270-
cache->lastUsedPages = metadata->lastUsedPages;
276+
cache->lastUsedPages = metadata->lastUsedPages;
271277

272-
UnlockReleaseBuffer(metabuffer);
278+
UnlockReleaseBuffer(metabuffer);
279+
}
273280

274281
index->rd_amcache = (void *) cache;
275282
}

src/test/regress/expected/indexing.out

+39
Original file line numberDiff line numberDiff line change
@@ -1343,6 +1343,45 @@ select tableoid::regclass, * from idxpart order by a;
13431343
idxpart2 | 857142 | six
13441344
(8 rows)
13451345

1346+
drop table idxpart;
1347+
-- Test some other non-btree index types
1348+
create table idxpart (a int, b text, c int[]) partition by range (a);
1349+
create table idxpart1 partition of idxpart for values from (0) to (100000);
1350+
set enable_seqscan to off;
1351+
create index idxpart_brin on idxpart using brin(b);
1352+
explain (costs off) select * from idxpart where b = 'abcd';
1353+
QUERY PLAN
1354+
-------------------------------------------
1355+
Bitmap Heap Scan on idxpart1 idxpart
1356+
Recheck Cond: (b = 'abcd'::text)
1357+
-> Bitmap Index Scan on idxpart1_b_idx
1358+
Index Cond: (b = 'abcd'::text)
1359+
(4 rows)
1360+
1361+
drop index idxpart_brin;
1362+
create index idxpart_spgist on idxpart using spgist(b);
1363+
explain (costs off) select * from idxpart where b = 'abcd';
1364+
QUERY PLAN
1365+
-------------------------------------------
1366+
Bitmap Heap Scan on idxpart1 idxpart
1367+
Recheck Cond: (b = 'abcd'::text)
1368+
-> Bitmap Index Scan on idxpart1_b_idx
1369+
Index Cond: (b = 'abcd'::text)
1370+
(4 rows)
1371+
1372+
drop index idxpart_spgist;
1373+
create index idxpart_gin on idxpart using gin(c);
1374+
explain (costs off) select * from idxpart where c @> array[42];
1375+
QUERY PLAN
1376+
----------------------------------------------
1377+
Bitmap Heap Scan on idxpart1 idxpart
1378+
Recheck Cond: (c @> '{42}'::integer[])
1379+
-> Bitmap Index Scan on idxpart1_c_idx
1380+
Index Cond: (c @> '{42}'::integer[])
1381+
(4 rows)
1382+
1383+
drop index idxpart_gin;
1384+
reset enable_seqscan;
13461385
drop table idxpart;
13471386
-- intentionally leave some objects around
13481387
create table idxpart (a int) partition by range (a);

src/test/regress/sql/indexing.sql

+20
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,26 @@ insert into idxpart values (857142, 'six');
717717
select tableoid::regclass, * from idxpart order by a;
718718
drop table idxpart;
719719

720+
-- Test some other non-btree index types
721+
create table idxpart (a int, b text, c int[]) partition by range (a);
722+
create table idxpart1 partition of idxpart for values from (0) to (100000);
723+
set enable_seqscan to off;
724+
725+
create index idxpart_brin on idxpart using brin(b);
726+
explain (costs off) select * from idxpart where b = 'abcd';
727+
drop index idxpart_brin;
728+
729+
create index idxpart_spgist on idxpart using spgist(b);
730+
explain (costs off) select * from idxpart where b = 'abcd';
731+
drop index idxpart_spgist;
732+
733+
create index idxpart_gin on idxpart using gin(c);
734+
explain (costs off) select * from idxpart where c @> array[42];
735+
drop index idxpart_gin;
736+
737+
reset enable_seqscan;
738+
drop table idxpart;
739+
720740
-- intentionally leave some objects around
721741
create table idxpart (a int) partition by range (a);
722742
create table idxpart1 partition of idxpart for values from (0) to (100);

0 commit comments

Comments
 (0)