Fix intermittent crash in DROP INDEX CONCURRENTLY.

When deleteOneObject closes and reopens the pg_depend relation,
we must see to it that the relcache pointer held by the calling function
(typically performMultipleDeletions) is updated.  Usually the relcache
entry is retained so that the pointer value doesn't change, which is why
the problem had escaped notice ... but after a cache flush event there's
no guarantee that the same memory will be reassigned.  To fix, change
the recursive functions' APIs so that we pass around a "Relation *"
not just "Relation".

Per investigation of occasional buildfarm failures.  This is trivial
to reproduce with -DCLOBBER_CACHE_ALWAYS, which points up the sad
lack of any buildfarm member running that way on a regular basis.

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 0b61c97eb4da391e66b7094115983094b3da2409..cefa82c3d54818d8e4da28d4efd4bbeb9dd17576 100644 (file)
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -171,13 +171,13 @@ static void findDependentObjects(const ObjectAddress *object,
                                         ObjectAddressStack *stack,
                                         ObjectAddresses *targetObjects,
                                         const ObjectAddresses *pendingObjects,
-                                        Relation depRel);
+                                        Relation *depRel);
 static void reportDependentObjects(const ObjectAddresses *targetObjects,
                                           DropBehavior behavior,
                                           int msglevel,
                                           const ObjectAddress *origObject);
 static void deleteOneObject(const ObjectAddress *object,
-                               Relation depRel, int32 flags);
+                               Relation *depRel, int32 flags);
 static void doDeletion(const ObjectAddress *object, int flags);
 static void AcquireDeletionLock(const ObjectAddress *object, int flags);
 static void ReleaseDeletionLock(const ObjectAddress *object);
@@ -250,7 +250,7 @@ performDeletion(const ObjectAddress *object,
                                                 NULL,  /* empty stack */
                                                 targetObjects,
                                                 NULL,  /* no pendingObjects */
-                                                depRel);
+                                                &depRel);
 
        /*
         * Check if deletion is allowed, and report about cascaded deletes.
@@ -267,7 +267,7 @@ performDeletion(const ObjectAddress *object,
        {
                ObjectAddress *thisobj = targetObjects->refs + i;
 
-               deleteOneObject(thisobj, depRel, flags);
+               deleteOneObject(thisobj, &depRel, flags);
        }
 
        /* And clean up */
@@ -328,7 +328,7 @@ performMultipleDeletions(const ObjectAddresses *objects,
                                                         NULL,          /* empty stack */
                                                         targetObjects,
                                                         objects,
-                                                        depRel);
+                                                        &depRel);
        }
 
        /*
@@ -349,7 +349,7 @@ performMultipleDeletions(const ObjectAddresses *objects,
        {
                ObjectAddress *thisobj = targetObjects->refs + i;
 
-               deleteOneObject(thisobj, depRel, flags);
+               deleteOneObject(thisobj, &depRel, flags);
        }
 
        /* And clean up */
@@ -398,7 +398,7 @@ deleteWhatDependsOn(const ObjectAddress *object,
                                                 NULL,  /* empty stack */
                                                 targetObjects,
                                                 NULL,  /* no pendingObjects */
-                                                depRel);
+                                                &depRel);
 
        /*
         * Check if deletion is allowed, and report about cascaded deletes.
@@ -427,7 +427,7 @@ deleteWhatDependsOn(const ObjectAddress *object,
                 * action.      If, in the future, this function is used for other
                 * purposes, we might need to revisit this.
                 */
-               deleteOneObject(thisobj, depRel, PERFORM_DELETION_INTERNAL);
+               deleteOneObject(thisobj, &depRel, PERFORM_DELETION_INTERNAL);
        }
 
        /* And clean up */
@@ -462,7 +462,7 @@ deleteWhatDependsOn(const ObjectAddress *object,
  *     targetObjects: list of objects that are scheduled to be deleted
  *     pendingObjects: list of other objects slated for destruction, but
  *                     not necessarily in targetObjects yet (can be NULL if none)
- *     depRel: already opened pg_depend relation
+ *     *depRel: already opened pg_depend relation
  */
 static void
 findDependentObjects(const ObjectAddress *object,
@@ -470,7 +470,7 @@ findDependentObjects(const ObjectAddress *object,
                                         ObjectAddressStack *stack,
                                         ObjectAddresses *targetObjects,
                                         const ObjectAddresses *pendingObjects,
-                                        Relation depRel)
+                                        Relation *depRel)
 {
        ScanKeyData key[3];
        int                     nkeys;
@@ -540,7 +540,7 @@ findDependentObjects(const ObjectAddress *object,
        else
                nkeys = 2;
 
-       scan = systable_beginscan(depRel, DependDependerIndexId, true,
+       scan = systable_beginscan(*depRel, DependDependerIndexId, true,
                                                          SnapshotNow, nkeys, key);
 
        while (HeapTupleIsValid(tup = systable_getnext(scan)))
@@ -715,7 +715,7 @@ findDependentObjects(const ObjectAddress *object,
        else
                nkeys = 2;
 
-       scan = systable_beginscan(depRel, DependReferenceIndexId, true,
+       scan = systable_beginscan(*depRel, DependReferenceIndexId, true,
                                                          SnapshotNow, nkeys, key);
 
        while (HeapTupleIsValid(tup = systable_getnext(scan)))
@@ -986,10 +986,10 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
 /*
  * deleteOneObject: delete a single object for performDeletion.
  *
- * depRel is the already-open pg_depend relation.
+ * *depRel is the already-open pg_depend relation.
  */
 static void
-deleteOneObject(const ObjectAddress *object, Relation depRel, int flags)
+deleteOneObject(const ObjectAddress *object, Relation *depRel, int flags)
 {
        ScanKeyData key[3];
        int                     nkeys;
@@ -1012,7 +1012,7 @@ deleteOneObject(const ObjectAddress *object, Relation depRel, int flags)
         * relation open across doDeletion().
         */
        if (flags & PERFORM_DELETION_CONCURRENTLY)
-               heap_close(depRel, RowExclusiveLock);
+               heap_close(*depRel, RowExclusiveLock);
 
        /*
         * Delete the object itself, in an object-type-dependent way.
@@ -1029,7 +1029,7 @@ deleteOneObject(const ObjectAddress *object, Relation depRel, int flags)
         * Reopen depRel if we closed it above
         */
        if (flags & PERFORM_DELETION_CONCURRENTLY)
-               depRel = heap_open(DependRelationId, RowExclusiveLock);
+               *depRel = heap_open(DependRelationId, RowExclusiveLock);
 
        /*
         * Now remove any pg_depend records that link from this object to others.
@@ -1057,12 +1057,12 @@ deleteOneObject(const ObjectAddress *object, Relation depRel, int flags)
        else
                nkeys = 2;
 
-       scan = systable_beginscan(depRel, DependDependerIndexId, true,
+       scan = systable_beginscan(*depRel, DependDependerIndexId, true,
                                                          SnapshotNow, nkeys, key);
 
        while (HeapTupleIsValid(tup = systable_getnext(scan)))
        {
-               simple_heap_delete(depRel, &tup->t_self);
+               simple_heap_delete(*depRel, &tup->t_self);
        }
 
        systable_endscan(scan);
@@ -1125,10 +1125,6 @@ doDeletion(const ObjectAddress *object, int flags)
                                break;
                        }
 
-               case OCLASS_EVENT_TRIGGER:
-                       RemoveEventTriggerById(object->objectId);
-                       break;
-
                case OCLASS_PROC:
                        RemoveFunctionById(object->objectId);
                        break;
@@ -1238,6 +1234,10 @@ doDeletion(const ObjectAddress *object, int flags)
                        RemoveExtensionById(object->objectId);
                        break;
 
+               case OCLASS_EVENT_TRIGGER:
+                       RemoveEventTriggerById(object->objectId);
+                       break;
+
                default:
                        elog(ERROR, "unrecognized object class: %u",
                                 object->classId);