Fix orphan on cancel of drop index concurrently.

simonat2ndQuadrant · simonat2ndQuadrant · commit da8572756581 · 2012-10-19T09:56:29.000+01:00
Canceling DROP INDEX CONCURRENTLY during
wait could allow an orphaned index to be
left behind which could not be dropped.

Backpatch to 9.2

Andres Freund, tested by Abhijit Menon-Sen
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
@@ -355,12 +355,7 @@ performMultipleDeletions(const ObjectAddresses *objects,
 	/* And clean up */
 	free_object_addresses(targetObjects);
 
-	/*
-	 * We closed depRel earlier in deleteOneObject if doing a drop
-	 * concurrently
-	 */
-	if ((flags & PERFORM_DELETION_CONCURRENTLY) != PERFORM_DELETION_CONCURRENTLY)
-		heap_close(depRel, RowExclusiveLock);
+	heap_close(depRel, RowExclusiveLock);
 }
 
 /*
@@ -1000,6 +995,7 @@ deleteOneObject(const ObjectAddress *object, Relation depRel, int flags)
 	int			nkeys;
 	SysScanDesc scan;
 	HeapTuple	tup;
+	Oid			depRelOid = depRel->rd_id;
 
 	/* DROP hook of the objects being removed */
 	if (object_access_hook)
@@ -1012,7 +1008,33 @@ deleteOneObject(const ObjectAddress *object, Relation depRel, int flags)
 	}
 
 	/*
-	 * First remove any pg_depend records that link from this object to
+	 * Close depRel if we are doing a drop concurrently. The individual
+	 * deletion has to commit the transaction and we don't want dangling
+	 * references.
+	 */
+	if (flags & PERFORM_DELETION_CONCURRENTLY)
+		heap_close(depRel, RowExclusiveLock);
+
+	/*
+	 * Delete the object itself, in an object-type-dependent way.
+	 *
+	 * Do this before removing outgoing dependencies as deletions can be
+	 * happening in concurrent mode. That will only happen for a single object
+	 * at once and if so the object will be invalidated inside a separate
+	 * transaction and only dropped inside a transaction thats in-progress when
+	 * doDeletion returns. This way no observer can see dangling dependency
+	 * entries.
+	 */
+	doDeletion(object, flags);
+
+	/*
+	 * Reopen depRel if we closed it before
+	 */
+	if (flags & PERFORM_DELETION_CONCURRENTLY)
+		depRel = heap_open(depRelOid, RowExclusiveLock);
+
+	/*
+	 * Then remove any pg_depend records that link from this object to
 	 * others.	(Any records linking to this object should be gone already.)
 	 *
 	 * When dropping a whole object (subId = 0), remove all pg_depend records
@@ -1054,17 +1076,6 @@ deleteOneObject(const ObjectAddress *object, Relation depRel, int flags)
 	deleteSharedDependencyRecordsFor(object->classId, object->objectId,
 									 object->objectSubId);
 
-	/*
-	 * Close depRel if we are doing a drop concurrently because it commits the
-	 * transaction, so we don't want dangling references.
-	 */
-	if ((flags & PERFORM_DELETION_CONCURRENTLY) == PERFORM_DELETION_CONCURRENTLY)
-		heap_close(depRel, RowExclusiveLock);
-
-	/*
-	 * Now delete the object itself, in an object-type-dependent way.
-	 */
-	doDeletion(object, flags);
 
 	/*
 	 * Delete any comments or security labels associated with this object.
@@ -1247,7 +1258,7 @@ AcquireDeletionLock(const ObjectAddress *object, int flags)
 {
 	if (object->classId == RelationRelationId)
 	{
-		if ((flags & PERFORM_DELETION_CONCURRENTLY) == PERFORM_DELETION_CONCURRENTLY)
+		if (flags & PERFORM_DELETION_CONCURRENTLY)
 			LockRelationOid(object->objectId, ShareUpdateExclusiveLock);
 		else
 			LockRelationOid(object->objectId, AccessExclusiveLock);
