From 251b31f6ae4a7af500b14107f55d253b72bf9bee Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 25 Sep 2012 14:58:32 +0200 Subject: [PATCH 2/2] Fix that DROP INDEX CONCURRENT could leave behind an index without dependencies in case of error DROP INDEX CONCURRENT needs to commit internal transactions to achieve its aim. It prevents problems due to that by forbidding doing any DROP that involves more than one object. It forgot about it's own dependencies to its own table though. Fix that by removing those only in the transaction the index is actually removed not in the ones invalidating it. --- src/backend/catalog/dependency.c | 49 +++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 87d6f02..b9cfee2 100644 --- 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); -- 1.7.10.4