Now that we've rearranged relation open to get a lock before touching

the rel, it's easy to get rid of the narrow race-condition window that
used to exist in VACUUM and CLUSTER.  Did some minor code-beautification
work in the same area, too.

1 files changed, 30 insertions, 19 deletions

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index f0cbaefd60e..bd2301dd62e 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.152 2006/07/31 20:09:00 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.153 2006/08/18 16:09:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -240,6 +240,18 @@ cluster_rel(RelToCluster *rvtc, bool recheck)
 	CHECK_FOR_INTERRUPTS();
 
 	/*
+	 * We grab exclusive access to the target rel and index for the duration
+	 * of the transaction.	(This is redundant for the single-transaction
+	 * case, since cluster() already did it.)  The index lock is taken inside
+	 * check_index_is_clusterable.
+	 */
+	OldHeap = try_relation_open(rvtc->tableOid, AccessExclusiveLock);
+
+	/* If the table has gone away, we can skip processing it */
+	if (!OldHeap)
+		return;
+
+	/*
 	 * Since we may open a new transaction for each relation, we have to check
 	 * that the relation still is what we think it is.
 	 *
@@ -252,20 +264,23 @@ cluster_rel(RelToCluster *rvtc, bool recheck)
 		HeapTuple	tuple;
 		Form_pg_index indexForm;
 
+		/* Check that the user still owns the relation */
+		if (!pg_class_ownercheck(rvtc->tableOid, GetUserId()))
+		{
+			relation_close(OldHeap, AccessExclusiveLock);
+			return;
+		}
+
 		/*
-		 * Check if the relation and index still exist before opening them
+		 * Check that the index still exists
 		 */
 		if (!SearchSysCacheExists(RELOID,
-								  ObjectIdGetDatum(rvtc->tableOid),
-								  0, 0, 0) ||
-			!SearchSysCacheExists(RELOID,
 								  ObjectIdGetDatum(rvtc->indexOid),
 								  0, 0, 0))
+		{
+			relation_close(OldHeap, AccessExclusiveLock);
 			return;
-
-		/* Check that the user still owns the relation */
-		if (!pg_class_ownercheck(rvtc->tableOid, GetUserId()))
-			return;
+		}
 
 		/*
 		 * Check that the index is still the one with indisclustered set.
@@ -273,25 +288,21 @@ cluster_rel(RelToCluster *rvtc, bool recheck)
 		tuple = SearchSysCache(INDEXRELID,
 							   ObjectIdGetDatum(rvtc->indexOid),
 							   0, 0, 0);
-		if (!HeapTupleIsValid(tuple))
-			return;				/* could have gone away... */
+		if (!HeapTupleIsValid(tuple))		/* probably can't happen */
+		{
+			relation_close(OldHeap, AccessExclusiveLock);
+			return;
+		}
 		indexForm = (Form_pg_index) GETSTRUCT(tuple);
 		if (!indexForm->indisclustered)
 		{
 			ReleaseSysCache(tuple);
+			relation_close(OldHeap, AccessExclusiveLock);
 			return;
 		}
 		ReleaseSysCache(tuple);
 	}
 
-	/*
-	 * We grab exclusive access to the target rel and index for the duration
-	 * of the transaction.	(This is redundant for the single- transaction
-	 * case, since cluster() already did it.)  The index lock is taken inside
-	 * check_index_is_clusterable.
-	 */
-	OldHeap = heap_open(rvtc->tableOid, AccessExclusiveLock);
-
 	/* Check index is valid to cluster on */
 	check_index_is_clusterable(OldHeap, rvtc->indexOid, recheck);