Hi, On Mon, Dec 15, 2025 at 01:08:44PM +0900, Michael Paquier wrote: > While passing through, I have applied 0001.
Out of curiosity, I searched for other mismatched index_open/relation_close pairs in the tree and found a few more with the help of [1]. They are fixed in the attached. Please note that for hash_bitmap_info() and pgstathashindex() the open calls are changed instead. For those we keep the IS_INDEX() checks to reject partitioned indexes (which index_open() accepts via validate_relation_kind()). So, that also changes the error messages in some tests. If we do prefer the previous error messages we could change the close calls instead (I prefer the way it's done in the attached though). Thoughts? [1]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/mismatched_open_close_pairs.cocci Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From 208ed2ec063b3a11ec490928f06b1ade0eb424d7 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Wed, 17 Dec 2025 10:31:06 +0000 Subject: [PATCH v1] Fix mismatched index_open/relation_close pairs Several functions were using mismatched pairs of open/close functions: - Opening with index_open() but closing with relation_close() - Opening with relation_open() but closing with index_close() The index_close() and relation_close() functions are currently identical in implementation. However, the open functions differ: index_open() validates that the relation is an index, while relation_open() accepts any relation type. Using matching pairs improves code clarity and ensures proper validation. Same idea as in 171198ff2a5. Please note that for hash_bitmap_info() and pgstathashindex() the open calls are changed instead. For those we keep the IS_INDEX() checks to reject partitioned indexes (which index_open() accepts via validate_relation_kind()). So, that also changes the error messages in some tests. --- contrib/pageinspect/hashfuncs.c | 2 +- contrib/pgstattuple/expected/pgstattuple.out | 10 +++++----- contrib/pgstattuple/pgstatindex.c | 2 +- src/backend/access/brin/brin.c | 4 ++-- src/backend/parser/parse_utilcmd.c | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) 11.4% contrib/pageinspect/ 50.0% contrib/pgstattuple/expected/ 9.3% contrib/pgstattuple/ 21.6% src/backend/access/brin/ 7.4% src/backend/parser/ diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c index 0e898889fa5..e086d0be4d1 100644 --- a/contrib/pageinspect/hashfuncs.c +++ b/contrib/pageinspect/hashfuncs.c @@ -415,7 +415,7 @@ hash_bitmap_info(PG_FUNCTION_ARGS) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must be superuser to use raw page functions"))); - indexRel = relation_open(indexRelid, AccessShareLock); + indexRel = index_open(indexRelid, AccessShareLock); if (!IS_INDEX(indexRel) || !IS_HASH(indexRel)) ereport(ERROR, diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out index 9176dc98b6a..18d0b8ae327 100644 --- a/contrib/pgstattuple/expected/pgstattuple.out +++ b/contrib/pgstattuple/expected/pgstattuple.out @@ -172,7 +172,7 @@ ERROR: relation "test_partitioned" is not a btree index select pgstatginindex('test_partitioned'); ERROR: relation "test_partitioned" is not a GIN index select pgstathashindex('test_partitioned'); -ERROR: relation "test_partitioned" is not a hash index +ERROR: "test_partitioned" is not an index select pgstathashindex('test_partitioned_hash_index'); ERROR: relation "test_partitioned_hash_index" is not a hash index create view test_view as select 1; @@ -191,7 +191,7 @@ ERROR: relation "test_view" is not a btree index select pgstatginindex('test_view'); ERROR: relation "test_view" is not a GIN index select pgstathashindex('test_view'); -ERROR: relation "test_view" is not a hash index +ERROR: "test_view" is not an index create foreign data wrapper dummy; create server dummy_server foreign data wrapper dummy; create foreign table test_foreign_table () server dummy_server; @@ -210,7 +210,7 @@ ERROR: relation "test_foreign_table" is not a btree index select pgstatginindex('test_foreign_table'); ERROR: relation "test_foreign_table" is not a GIN index select pgstathashindex('test_foreign_table'); -ERROR: relation "test_foreign_table" is not a hash index +ERROR: "test_foreign_table" is not an index -- a partition of a partitioned table should work though create table test_partition partition of test_partitioned for values from (1) to (100); select pgstattuple('test_partition'); @@ -256,7 +256,7 @@ ERROR: relation "test_partition" is not a btree index select pgstatginindex('test_partition'); ERROR: relation "test_partition" is not a GIN index select pgstathashindex('test_partition'); -ERROR: relation "test_partition" is not a hash index +ERROR: "test_partition" is not an index -- an actual index of a partitioned table should work though create index test_partition_idx on test_partition(a); create index test_partition_hash_idx on test_partition using hash (a); @@ -293,7 +293,7 @@ ERROR: relation "test_sequence" is not a btree index select pgstatginindex('test_sequence'); ERROR: relation "test_sequence" is not a GIN index select pgstathashindex('test_sequence'); -ERROR: relation "test_sequence" is not a hash index +ERROR: "test_sequence" is not an index select pgstattuple_approx('test_sequence'); ERROR: relation "test_sequence" is of wrong relation kind DETAIL: This operation is not supported for sequences. diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c index 40823d54fca..0dfcb3960ae 100644 --- a/contrib/pgstattuple/pgstatindex.c +++ b/contrib/pgstattuple/pgstatindex.c @@ -597,7 +597,7 @@ pgstathashindex(PG_FUNCTION_ARGS) float8 free_percent; uint64 total_space; - rel = relation_open(relid, AccessShareLock); + rel = index_open(relid, AccessShareLock); if (!IS_INDEX(rel) || !IS_HASH(rel)) ereport(ERROR, diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 26cb75058d1..a4c46ef3291 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -1478,7 +1478,7 @@ brin_summarize_range(PG_FUNCTION_ARGS) /* Restore userid and security context */ SetUserIdAndSecContext(save_userid, save_sec_context); - relation_close(indexRel, ShareUpdateExclusiveLock); + index_close(indexRel, ShareUpdateExclusiveLock); relation_close(heapRel, ShareUpdateExclusiveLock); PG_RETURN_INT32((int32) numSummarized); @@ -1568,7 +1568,7 @@ brin_desummarize_range(PG_FUNCTION_ARGS) errmsg("index \"%s\" is not valid", RelationGetRelationName(indexRel)))); - relation_close(indexRel, ShareUpdateExclusiveLock); + index_close(indexRel, ShareUpdateExclusiveLock); relation_close(heapRel, ShareUpdateExclusiveLock); PG_RETURN_VOID(); diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 375b40b29af..2b7b084f216 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -2572,7 +2572,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) } /* Close the index relation but keep the lock */ - relation_close(index_rel, NoLock); + index_close(index_rel, NoLock); index->indexOid = index_oid; } -- 2.34.1
