git.postgresql.org Git - postgresql.git/commitdiff

git projects / postgresql.git / commitdiff

Avoid trying to fetch metapage of an SPGist partitioned index.

2023年12月21日 17:43:36 +0000 (12:43 -0500)

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

src/backend/access/spgist/spgutils.c patch | blob | blame | history

src/test/regress/expected/indexing.out patch | blob | blame | history

src/test/regress/sql/indexing.sql patch | blob | blame | history

diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c

index fd4b6157101f323445d6acf9d17fa8d85681c30e..a8055c51a2d53962315e9b5b123d915fa4efc952 100644 (file)

--- 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 087f955b1e6eb70a79523ffc2dde3bc4dd105932..3de99dd92734d375b0fb4270606d35d8e757fcef 100644 (file)

--- 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 44f6788915cad5eebf57cd37406c77b6106ad5e6..e5b7b18b91c09eb05f8fb1f325e46aec535b2e60 100644 (file)

--- 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);

This is the main PostgreSQL git repository.

RSS Atom