Hi, On Fri, Dec 19, 2025 at 05:36:51PM +0800, Japin Li wrote: > On Fri, 19 Dec 2025 at 08:23, Bertrand Drouvot <[email protected]> > wrote: > > Yeah that would not hurt. What about before the relation_open() calls? > > > > " > > Use relation_open() and not index_open() to avoid the > > validate_relation_kind() > > check as we handle relation validation separately below. > > " > > > > LGTM.
Thanks! Done that way in the attached. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From 1a83cc26b23b6a720839dae06c09e06d00c67750 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Fri, 19 Dec 2025 04:08:33 +0000 Subject: [PATCH v3] 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. In passing, explain why we are using relation_open() instead of index_open() in a few places. Author: Bertrand Drouvot <[email protected]> Reviewed-by: Japin Li <[email protected]> Discussion: https://postgr.es/m/[email protected] --- contrib/pageinspect/hashfuncs.c | 7 ++++++- contrib/pgstattuple/pgstatindex.c | 12 +++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) 38.3% contrib/pageinspect/ 61.6% contrib/pgstattuple/ diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c index 0e898889fa5..03e926931fd 100644 --- a/contrib/pageinspect/hashfuncs.c +++ b/contrib/pageinspect/hashfuncs.c @@ -415,6 +415,11 @@ hash_bitmap_info(PG_FUNCTION_ARGS) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must be superuser to use raw page functions"))); + /* + * Use relation_open() and not index_open() to avoid the + * validate_relation_kind() check as we handle relation validation + * separately below. + */ indexRel = relation_open(indexRelid, AccessShareLock); if (!IS_INDEX(indexRel) || !IS_HASH(indexRel)) @@ -486,7 +491,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..93e191bceee 100644 --- a/contrib/pgstattuple/pgstatindex.c +++ b/contrib/pgstattuple/pgstatindex.c @@ -514,6 +514,11 @@ pgstatginindex_internal(Oid relid, FunctionCallInfo fcinfo) bool nulls[3] = {false, false, false}; Datum result; + /* + * Use relation_open() and not index_open() to avoid the + * validate_relation_kind() check as we handle relation validation + * separately below. + */ rel = relation_open(relid, AccessShareLock); if (!IS_INDEX(rel) || !IS_GIN(rel)) @@ -597,6 +602,11 @@ pgstathashindex(PG_FUNCTION_ARGS) float8 free_percent; uint64 total_space; + /* + * Use relation_open() and not index_open() to avoid the + * validate_relation_kind() check as we handle relation validation + * separately below. + */ rel = relation_open(relid, AccessShareLock); if (!IS_INDEX(rel) || !IS_HASH(rel)) @@ -691,7 +701,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
