From: Tom Lane Date: Thu, 21 Dec 2023 17:43:36 +0000 (-0500) Subject: Avoid trying to fetch metapage of an SPGist partitioned index. X-Git-Tag: REL_17_BETA1~1256 X-Git-Url: http://git.postgresql.org/gitweb/?a=commitdiff_plain;h=903737c5bf9431c2f983c87ac211082418341805;p=postgresql.git 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 (3c569049b), 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 --- diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c index fd4b6157101..a8055c51a2d 100644 --- a/src/backend/access/spgist/spgutils.c +++ b/src/backend/access/spgist/spgutils.c @@ -188,8 +188,6 @@ spgGetCache(Relation index) Oid atttype; spgConfigIn in; FmgrInfo *procinfo; - Buffer metabuffer; - SpGistMetaPageData *metadata; cache = MemoryContextAllocZero(index->rd_indexcxt, sizeof(SpGistCache)); @@ -257,19 +255,28 @@ spgGetCache(Relation index) fillTypeDesc(&cache->attPrefixType, cache->config.prefixType); fillTypeDesc(&cache->attLabelType, cache->config.labelType); - /* Last, get the lastUsedPages data from the metapage */ - metabuffer = ReadBuffer(index, SPGIST_METAPAGE_BLKNO); - LockBuffer(metabuffer, BUFFER_LOCK_SHARE); + /* + * Finally, if it's a real index (not a partitioned one), get the + * lastUsedPages data from the metapage + */ + if (index->rd_rel->relkind != RELKIND_PARTITIONED_INDEX) + { + Buffer metabuffer; + SpGistMetaPageData *metadata; + + metabuffer = ReadBuffer(index, SPGIST_METAPAGE_BLKNO); + LockBuffer(metabuffer, BUFFER_LOCK_SHARE); - metadata = SpGistPageGetMeta(BufferGetPage(metabuffer)); + metadata = SpGistPageGetMeta(BufferGetPage(metabuffer)); - if (metadata->magicNumber != SPGIST_MAGIC_NUMBER) - elog(ERROR, "index \"%s\" is not an SP-GiST index", - RelationGetRelationName(index)); + if (metadata->magicNumber != SPGIST_MAGIC_NUMBER) + elog(ERROR, "index \"%s\" is not an SP-GiST index", + RelationGetRelationName(index)); - cache->lastUsedPages = metadata->lastUsedPages; + cache->lastUsedPages = metadata->lastUsedPages; - UnlockReleaseBuffer(metabuffer); + UnlockReleaseBuffer(metabuffer); + } index->rd_amcache = (void *) cache; } diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index 087f955b1e6..3de99dd9273 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -1343,6 +1343,45 @@ select tableoid::regclass, * from idxpart order by a; idxpart2 | 857142 | six (8 rows) +drop table idxpart; +-- Test some other non-btree index types +create table idxpart (a int, b text, c int[]) partition by range (a); +create table idxpart1 partition of idxpart for values from (0) to (100000); +set enable_seqscan to off; +create index idxpart_brin on idxpart using brin(b); +explain (costs off) select * from idxpart where b = 'abcd'; + QUERY PLAN +------------------------------------------- + Bitmap Heap Scan on idxpart1 idxpart + Recheck Cond: (b = 'abcd'::text) + -> Bitmap Index Scan on idxpart1_b_idx + Index Cond: (b = 'abcd'::text) +(4 rows) + +drop index idxpart_brin; +create index idxpart_spgist on idxpart using spgist(b); +explain (costs off) select * from idxpart where b = 'abcd'; + QUERY PLAN +------------------------------------------- + Bitmap Heap Scan on idxpart1 idxpart + Recheck Cond: (b = 'abcd'::text) + -> Bitmap Index Scan on idxpart1_b_idx + Index Cond: (b = 'abcd'::text) +(4 rows) + +drop index idxpart_spgist; +create index idxpart_gin on idxpart using gin(c); +explain (costs off) select * from idxpart where c @> array[42]; + QUERY PLAN +---------------------------------------------- + Bitmap Heap Scan on idxpart1 idxpart + Recheck Cond: (c @> '{42}'::integer[]) + -> Bitmap Index Scan on idxpart1_c_idx + Index Cond: (c @> '{42}'::integer[]) +(4 rows) + +drop index idxpart_gin; +reset enable_seqscan; drop table idxpart; -- intentionally leave some objects around create table idxpart (a int) partition by range (a); diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql index 44f6788915c..e5b7b18b91c 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -717,6 +717,26 @@ insert into idxpart values (857142, 'six'); select tableoid::regclass, * from idxpart order by a; drop table idxpart; +-- Test some other non-btree index types +create table idxpart (a int, b text, c int[]) partition by range (a); +create table idxpart1 partition of idxpart for values from (0) to (100000); +set enable_seqscan to off; + +create index idxpart_brin on idxpart using brin(b); +explain (costs off) select * from idxpart where b = 'abcd'; +drop index idxpart_brin; + +create index idxpart_spgist on idxpart using spgist(b); +explain (costs off) select * from idxpart where b = 'abcd'; +drop index idxpart_spgist; + +create index idxpart_gin on idxpart using gin(c); +explain (costs off) select * from idxpart where c @> array[42]; +drop index idxpart_gin; + +reset enable_seqscan; +drop table idxpart; + -- intentionally leave some objects around create table idxpart (a int) partition by range (a); create table idxpart1 partition of idxpart for values from (0) to (100);