Hi, On Fri, Dec 19, 2025 at 08:01:54AM +0900, Michael Paquier wrote: > On Wed, Dec 17, 2025 at 11:57:13AM +0000, Bertrand Drouvot wrote: > > 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). > > I have noticed that the two surrounding relation_close() calls for the > parent tables did not get the notice of the change for brin.c of what > you are doing for the indexes, while we use table_open(). I have > fixed these.
Nice catch, thanks! > It would be nicer if IS_INDEX() could be removed in the other code > paths you are suggesting to change, but the partitioned index argument > also means that we would have two code paths in charge of a relkind > check instead of one. Just using relation_*() may be cleaner. Yeah, and removing IS_INDEX() and adding a check for partitioned indexes would still mean 2 code paths. So, v2 changes the close calls (and that's consistent with what pgstatginindex_internal() is doing. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From ef7f64323d326aa54dcc4b0f25ac47242e11def9 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Fri, 19 Dec 2025 04:08:33 +0000 Subject: [PATCH v2] Use relation_close() more consistently All the code paths updated here have been using index_close() to close a relation that has already been opened with relation_open(). index_close() does the same thing as relation_close(), so there was no harm, but being inconsistent could lead to issues if the internals of these close() functions begin to introduce specific logic in the future. Author: Bertrand Drouvot <[email protected]> Discussion: https://postgr.es/m/[email protected] --- contrib/pageinspect/hashfuncs.c | 2 +- contrib/pgstattuple/pgstatindex.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 53.1% contrib/pageinspect/ 46.8% contrib/pgstattuple/ diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c index 0e898889fa5..d3066eac81f 100644 --- a/contrib/pageinspect/hashfuncs.c +++ b/contrib/pageinspect/hashfuncs.c @@ -486,7 +486,7 @@ hash_bitmap_info(PG_FUNCTION_ARGS) bit = ISSET(freep, bitmapbit) != 0; _hash_relbuf(indexRel, mapbuf); - index_close(indexRel, AccessShareLock); + relation_close(indexRel, AccessShareLock); /* Build a tuple descriptor for our result type */ if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE) diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c index 40823d54fca..f4bee79a216 100644 --- a/contrib/pgstattuple/pgstatindex.c +++ b/contrib/pgstattuple/pgstatindex.c @@ -691,7 +691,7 @@ pgstathashindex(PG_FUNCTION_ARGS) } /* Done accessing the index */ - index_close(rel, AccessShareLock); + relation_close(rel, AccessShareLock); /* Count unused pages as free space. */ stats.free_space += (uint64) stats.unused_pages * stats.space_per_page; -- 2.34.1
