From b19e661d5244582752a5fa61b2cc9e60a7825fc1 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 14 Oct 2025 14:42:37 -0500
Subject: [PATCH v7 1/3] Fix lookups in
 pg_{clear,restore}_{attribute,relation}_stats().

Reviewed-by: Jeff Davis <pgsql@j-davis.com>
Discussion: https://postgr.es/m/Z8zwVmGzXyDdkAXj%40nathan
Backpatch-through: 18
---
 src/backend/statistics/attribute_stats.c   |  16 ++-
 src/backend/statistics/relation_stats.c    |   8 +-
 src/backend/statistics/stat_utils.c        | 152 +++++++++++----------
 src/include/statistics/stat_utils.h        |   8 +-
 src/test/regress/expected/stats_import.out |   6 +-
 5 files changed, 103 insertions(+), 87 deletions(-)

diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c
index 1db6a7f784c..c5df83282e0 100644
--- a/src/backend/statistics/attribute_stats.c
+++ b/src/backend/statistics/attribute_stats.c
@@ -19,8 +19,10 @@
 
 #include "access/heapam.h"
 #include "catalog/indexing.h"
+#include "catalog/namespace.h"
 #include "catalog/pg_collation.h"
 #include "catalog/pg_operator.h"
+#include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
 #include "statistics/statistics.h"
 #include "statistics/stat_utils.h"
@@ -143,6 +145,7 @@ attribute_statistics_update(FunctionCallInfo fcinfo)
 	char	   *attname;
 	AttrNumber	attnum;
 	bool		inherited;
+	Oid			locked_table = InvalidOid;
 
 	Relation	starel;
 	HeapTuple	statup;
@@ -182,8 +185,6 @@ attribute_statistics_update(FunctionCallInfo fcinfo)
 	nspname = TextDatumGetCString(PG_GETARG_DATUM(ATTRELSCHEMA_ARG));
 	relname = TextDatumGetCString(PG_GETARG_DATUM(ATTRELNAME_ARG));
 
-	reloid = stats_lookup_relid(nspname, relname);
-
 	if (RecoveryInProgress())
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
@@ -191,7 +192,9 @@ attribute_statistics_update(FunctionCallInfo fcinfo)
 				 errhint("Statistics cannot be modified during recovery.")));
 
 	/* lock before looking up attribute */
-	stats_lock_check_privileges(reloid);
+	reloid = RangeVarGetRelidExtended(makeRangeVar(nspname, relname, -1),
+									  ShareUpdateExclusiveLock, 0,
+									  RangeVarCallbackForStats, &locked_table);
 
 	/* user can specify either attname or attnum, but not both */
 	if (!PG_ARGISNULL(ATTNAME_ARG))
@@ -917,6 +920,7 @@ pg_clear_attribute_stats(PG_FUNCTION_ARGS)
 	char	   *attname;
 	AttrNumber	attnum;
 	bool		inherited;
+	Oid			locked_table = InvalidOid;
 
 	stats_check_required_arg(fcinfo, cleararginfo, C_ATTRELSCHEMA_ARG);
 	stats_check_required_arg(fcinfo, cleararginfo, C_ATTRELNAME_ARG);
@@ -926,15 +930,15 @@ pg_clear_attribute_stats(PG_FUNCTION_ARGS)
 	nspname = TextDatumGetCString(PG_GETARG_DATUM(C_ATTRELSCHEMA_ARG));
 	relname = TextDatumGetCString(PG_GETARG_DATUM(C_ATTRELNAME_ARG));
 
-	reloid = stats_lookup_relid(nspname, relname);
-
 	if (RecoveryInProgress())
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("recovery is in progress"),
 				 errhint("Statistics cannot be modified during recovery.")));
 
-	stats_lock_check_privileges(reloid);
+	reloid = RangeVarGetRelidExtended(makeRangeVar(nspname, relname, -1),
+									  ShareUpdateExclusiveLock, 0,
+									  RangeVarCallbackForStats, &locked_table);
 
 	attname = TextDatumGetCString(PG_GETARG_DATUM(C_ATTNAME_ARG));
 	attnum = get_attnum(reloid, attname);
diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index a59f0c519a4..174da7d93a5 100644
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -20,6 +20,7 @@
 #include "access/heapam.h"
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
+#include "nodes/makefuncs.h"
 #include "statistics/stat_utils.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
@@ -82,6 +83,7 @@ relation_statistics_update(FunctionCallInfo fcinfo)
 	Datum		values[4] = {0};
 	bool		nulls[4] = {0};
 	int			nreplaces = 0;
+	Oid			locked_table = InvalidOid;
 
 	stats_check_required_arg(fcinfo, relarginfo, RELSCHEMA_ARG);
 	stats_check_required_arg(fcinfo, relarginfo, RELNAME_ARG);
@@ -89,15 +91,15 @@ relation_statistics_update(FunctionCallInfo fcinfo)
 	nspname = TextDatumGetCString(PG_GETARG_DATUM(RELSCHEMA_ARG));
 	relname = TextDatumGetCString(PG_GETARG_DATUM(RELNAME_ARG));
 
-	reloid = stats_lookup_relid(nspname, relname);
-
 	if (RecoveryInProgress())
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("recovery is in progress"),
 				 errhint("Statistics cannot be modified during recovery.")));
 
-	stats_lock_check_privileges(reloid);
+	reloid = RangeVarGetRelidExtended(makeRangeVar(nspname, relname, -1),
+									  ShareUpdateExclusiveLock, 0,
+									  RangeVarCallbackForStats, &locked_table);
 
 	if (!PG_ARGISNULL(RELPAGES_ARG))
 	{
diff --git a/src/backend/statistics/stat_utils.c b/src/backend/statistics/stat_utils.c
index ef7e5168bed..5fd49e26132 100644
--- a/src/backend/statistics/stat_utils.c
+++ b/src/backend/statistics/stat_utils.c
@@ -16,9 +16,11 @@
 
 #include "postgres.h"
 
+#include "access/htup_details.h"
 #include "access/relation.h"
 #include "catalog/index.h"
 #include "catalog/namespace.h"
+#include "catalog/pg_class.h"
 #include "catalog/pg_database.h"
 #include "funcapi.h"
 #include "miscadmin.h"
@@ -29,6 +31,7 @@
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
+#include "utils/syscache.h"
 
 /*
  * Ensure that a given argument is not null.
@@ -119,53 +122,84 @@ stats_check_arg_pair(FunctionCallInfo fcinfo,
 }
 
 /*
- * Lock relation in ShareUpdateExclusive mode, check privileges, and close the
- * relation (but retain the lock).
- *
  * A role has privileges to set statistics on the relation if any of the
  * following are true:
  *   - the role owns the current database and the relation is not shared
  *   - the role has the MAINTAIN privilege on the relation
  */
 void
-stats_lock_check_privileges(Oid reloid)
+RangeVarCallbackForStats(const RangeVar *relation,
+						 Oid relId, Oid oldRelId, void *arg)
 {
-	Relation	table;
-	Oid			table_oid = reloid;
-	Oid			index_oid = InvalidOid;
-	LOCKMODE	index_lockmode = NoLock;
+	Oid		   *locked_oid = (Oid *) arg;
+	Oid			table_oid = relId;
+	HeapTuple	tuple;
+	Form_pg_class form;
+	char		relkind;
 
 	/*
-	 * For indexes, we follow the locking behavior in do_analyze_rel() and
-	 * check_lock_if_inplace_updateable_rel(), which is to lock the table
-	 * first in ShareUpdateExclusive mode and then the index in AccessShare
-	 * mode.
-	 *
-	 * Partitioned indexes are treated differently than normal indexes in
-	 * check_lock_if_inplace_updateable_rel(), so we take a
-	 * ShareUpdateExclusive lock on both the partitioned table and the
-	 * partitioned index.
+	 * If we previously locked some other index's heap, and the name we're
+	 * looking up no longer refers to that relation, release the now-useless
+	 * lock.
 	 */
-	switch (get_rel_relkind(reloid))
+	if (relId != oldRelId && OidIsValid(*locked_oid))
 	{
-		case RELKIND_INDEX:
-			index_oid = reloid;
-			table_oid = IndexGetRelation(index_oid, false);
-			index_lockmode = AccessShareLock;
-			break;
-		case RELKIND_PARTITIONED_INDEX:
-			index_oid = reloid;
-			table_oid = IndexGetRelation(index_oid, false);
-			index_lockmode = ShareUpdateExclusiveLock;
-			break;
-		default:
-			break;
+		UnlockRelationOid(*locked_oid, ShareUpdateExclusiveLock);
+		*locked_oid = InvalidOid;
+	}
+
+	/* If the relation does not exist, there's nothing more to do. */
+	if (!OidIsValid(relId))
+		return;
+
+	/* If the relation does exist, check whether it's an index. */
+	relkind = get_rel_relkind(relId);
+	if (relkind == RELKIND_INDEX ||
+		relkind == RELKIND_PARTITIONED_INDEX)
+		table_oid = IndexGetRelation(relId, false);
+
+	/*
+	 * If retrying yields the same OID, there are a couple of extremely
+	 * unlikely scenarios we need to handle.
+	 */
+	if (relId == oldRelId)
+	{
+		/*
+		 * If a previous lookup found an index, but the current lookup did
+		 * not, the index was dropped and the OID was reused for something
+		 * else between lookups.  In theory, we could simply drop our lock on
+		 * the index's relation and proceed, but in the interest of avoiding
+		 * complexity, we just error.
+		 */
+		if (table_oid == relId && OidIsValid(*locked_oid))
+			ereport(ERROR,
+					(errcode(ERRCODE_UNDEFINED_OBJECT),
+					 errmsg("index \"%s\" was concurrently dropped",
+							relation->relname)));
+
+		/*
+		 * If the current lookup found an index but a previous lookup either
+		 * did not find an index or found one with a different parent
+		 * relation, the relation was dropped and the OID was reused for an
+		 * index between lookups.  RangeVarGetRelidExtended() will have
+		 * already locked the index at this point, so we can't just lock the
+		 * newly discovered parent table OID without risking deadlock.  As
+		 * above, we just error in this case.
+		 */
+		if (table_oid != relId && table_oid != *locked_oid)
+			ereport(ERROR,
+					(errcode(ERRCODE_UNDEFINED_OBJECT),
+					 errmsg("index \"%s\" was concurrently created",
+							relation->relname)));
 	}
 
-	table = relation_open(table_oid, ShareUpdateExclusiveLock);
+	tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(table_oid));
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "cache lookup failed for OID %u", table_oid);
+	form = (Form_pg_class) GETSTRUCT(tuple);
 
 	/* the relkinds that can be used with ANALYZE */
-	switch (table->rd_rel->relkind)
+	switch (form->relkind)
 	{
 		case RELKIND_RELATION:
 		case RELKIND_MATVIEW:
@@ -176,62 +210,36 @@ stats_lock_check_privileges(Oid reloid)
 			ereport(ERROR,
 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 					 errmsg("cannot modify statistics for relation \"%s\"",
-							RelationGetRelationName(table)),
-					 errdetail_relkind_not_supported(table->rd_rel->relkind)));
+							NameStr(form->relname)),
+					 errdetail_relkind_not_supported(form->relkind)));
 	}
 
-	if (OidIsValid(index_oid))
-	{
-		Relation	index;
-
-		Assert(index_lockmode != NoLock);
-		index = relation_open(index_oid, index_lockmode);
-
-		Assert(index->rd_index && index->rd_index->indrelid == table_oid);
-
-		/* retain lock on index */
-		relation_close(index, NoLock);
-	}
-
-	if (table->rd_rel->relisshared)
+	if (form->relisshared)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot modify statistics for shared relation")));
 
+	/* Check permissions */
 	if (!object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()))
 	{
-		AclResult	aclresult = pg_class_aclcheck(RelationGetRelid(table),
+		AclResult	aclresult = pg_class_aclcheck(table_oid,
 												  GetUserId(),
 												  ACL_MAINTAIN);
 
 		if (aclresult != ACLCHECK_OK)
 			aclcheck_error(aclresult,
-						   get_relkind_objtype(table->rd_rel->relkind),
-						   NameStr(table->rd_rel->relname));
+						   get_relkind_objtype(form->relkind),
+						   NameStr(form->relname));
 	}
 
-	/* retain lock on table */
-	relation_close(table, NoLock);
-}
+	ReleaseSysCache(tuple);
 
-/*
- * Lookup relation oid from schema and relation name.
- */
-Oid
-stats_lookup_relid(const char *nspname, const char *relname)
-{
-	Oid			nspoid;
-	Oid			reloid;
-
-	nspoid = LookupExplicitNamespace(nspname, false);
-	reloid = get_relname_relid(relname, nspoid);
-	if (!OidIsValid(reloid))
-		ereport(ERROR,
-				(errcode(ERRCODE_UNDEFINED_TABLE),
-				 errmsg("relation \"%s.%s\" does not exist",
-						nspname, relname)));
-
-	return reloid;
+	/* Lock heap before index to avoid deadlock. */
+	if (relId != oldRelId && table_oid != relId)
+	{
+		LockRelationOid(table_oid, ShareUpdateExclusiveLock);
+		*locked_oid = table_oid;
+	}
 }
 
 
diff --git a/src/include/statistics/stat_utils.h b/src/include/statistics/stat_utils.h
index 512eb776e0e..f41b181d4d3 100644
--- a/src/include/statistics/stat_utils.h
+++ b/src/include/statistics/stat_utils.h
@@ -15,6 +15,9 @@
 
 #include "fmgr.h"
 
+/* avoid including primnodes.h here */
+typedef struct RangeVar RangeVar;
+
 struct StatsArgInfo
 {
 	const char *argname;
@@ -30,9 +33,8 @@ extern bool stats_check_arg_pair(FunctionCallInfo fcinfo,
 								 struct StatsArgInfo *arginfo,
 								 int argnum1, int argnum2);
 
-extern void stats_lock_check_privileges(Oid reloid);
-
-extern Oid	stats_lookup_relid(const char *nspname, const char *relname);
+extern void RangeVarCallbackForStats(const RangeVar *relation,
+									 Oid relId, Oid oldRelid, void *arg);
 
 extern bool stats_fill_fcinfo_from_arg_pairs(FunctionCallInfo pairs_fcinfo,
 											 FunctionCallInfo positional_fcinfo,
diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out
index 9e615ccd0af..98ce7dc2841 100644
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -120,9 +120,9 @@ WHERE relation = 'stats_import.test'::regclass AND
 SELECT mode FROM pg_locks
 WHERE relation = 'stats_import.test_i'::regclass AND
       pid = pg_backend_pid() AND granted;
-      mode       
------------------
- AccessShareLock
+           mode           
+--------------------------
+ ShareUpdateExclusiveLock
 (1 row)
 
 COMMIT;
-- 
2.39.5 (Apple Git-154)

