v1-0001-Do-not-lock-tables-in-get_tables_to_repack.patch

From fa5c3073f2f41cf3c8ab7c9663b681e6140a4824 Mon Sep 17 00:00:00 2001
From: ChangAo Chen <cca5507@qq.com>
Date: Tue, 16 Jun 2026 14:49:49 +0800
Subject: [PATCH v1] Do not lock tables in get_tables_to_repack().

When doing a whole database repack, we build a list of repackable
tables and take a lock on them to prevent concurrent drops. But
concurrent drops can always happen after we build the list because
we process each table in a separate transaction. The
ConditionalLockRelationOid() also makes the default behavior like
SKIP_LOCKED, which is unexpected.

To remove the locks, we need to make repack_is_permitted_for_relation()
handles concurrent drops correctly: it should not report an error
when failing to search the syscache in pg_class_aclcheck(). Use
pg_class_aclcheck_ext() instead to detect a concurrent drop. Also
check the return value of get_rel_name().

While at it, replace relation_close() with table_close() to match
the table_open().
---
 src/backend/commands/repack.c | 67 ++++++++++-------------------------
 1 file changed, 19 insertions(+), 48 deletions(-)

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index ec100e3eef5..6f25effeed2 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -2159,22 +2159,9 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 
 			index = (Form_pg_index) GETSTRUCT(tuple);
 
-			/*
-			 * Try to obtain a light lock on the index's table, to ensure it
-			 * doesn't go away while we collect the list.  If we cannot, just
-			 * disregard it.  Be sure to release this if we ultimately decide
-			 * not to process the table!
-			 */
-			if (!ConditionalLockRelationOid(index->indrelid, AccessShareLock))
-				continue;
-
-			/* Verify that the table still exists; skip if not */
 			classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid));
 			if (!HeapTupleIsValid(classtup))
-			{
-				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
-			}
 			classForm = (Form_pg_class) GETSTRUCT(classtup);
 
 			/* Skip temp relations belonging to other sessions */
@@ -2182,7 +2169,6 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				!isTempOrTempToastNamespace(classForm->relnamespace))
 			{
 				ReleaseSysCache(classtup);
-				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
 			}
 
@@ -2191,10 +2177,7 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, index->indrelid,
 												  GetUserId()))
-			{
-				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
-			}
 
 			/* Use a permanent memory context for the result list */
 			oldcxt = MemoryContextSwitchTo(permcxt);
@@ -2218,45 +2201,20 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 
 			class = (Form_pg_class) GETSTRUCT(tuple);
 
-			/*
-			 * Try to obtain a light lock on the table, to ensure it doesn't
-			 * go away while we collect the list.  If we cannot, just
-			 * disregard the table.  Be sure to release this if we ultimately
-			 * decide not to process the table!
-			 */
-			if (!ConditionalLockRelationOid(class->oid, AccessShareLock))
-				continue;
-
-			/* Verify that the table still exists */
-			if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(class->oid)))
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
-				continue;
-			}
-
 			/* Can only process plain tables and matviews */
 			if (class->relkind != RELKIND_RELATION &&
 				class->relkind != RELKIND_MATVIEW)
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
 				continue;
-			}
 
 			/* Skip temp relations belonging to other sessions */
 			if (class->relpersistence == RELPERSISTENCE_TEMP &&
 				!isTempOrTempToastNamespace(class->relnamespace))
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
 				continue;
-			}
 
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, class->oid,
 												  GetUserId()))
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
 				continue;
-			}
 
 			/* Use a permanent memory context for the result list */
 			oldcxt = MemoryContextSwitchTo(permcxt);
@@ -2269,7 +2227,7 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 	}
 
 	table_endscan(scan);
-	relation_close(catalog, AccessShareLock);
+	table_close(catalog, AccessShareLock);
 
 	return rtcs;
 }
@@ -2347,15 +2305,28 @@ get_tables_to_repack_partitioned(RepackCommand cmd, Oid relid,
 static bool
 repack_is_permitted_for_relation(RepackCommand cmd, Oid relid, Oid userid)
 {
+	bool		is_missing = false;
+
 	Assert(cmd == REPACK_COMMAND_CLUSTER || cmd == REPACK_COMMAND_REPACK);
 
-	if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK)
+	if (pg_class_aclcheck_ext(relid, userid, ACL_MAINTAIN, &is_missing) == ACLCHECK_OK)
 		return true;
 
-	ereport(WARNING,
-			errmsg("permission denied to execute %s on \"%s\", skipping it",
-				   RepackCommandAsString(cmd),
-				   get_rel_name(relid)));
+	/* Report a warning if the relation still exists. */
+	if (!is_missing)
+	{
+		char	   *relname;
+
+		relname = get_rel_name(relid);
+		if (relname != NULL)
+		{
+			ereport(WARNING,
+					errmsg("permission denied to execute %s on \"%s\", skipping it",
+						   RepackCommandAsString(cmd), relname));
+
+			pfree(relname);
+		}
+	}
 
 	return false;
 }
-- 
2.34.1