From: Jeff Davis Date: Mon, 3 Aug 2026 20:21:46 +0000 (-0700) Subject: Improve DROP SERVER handling of dependent subscriptions. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=608704adee75fd4316a10f4aa9a93aa8c6dc7eeb;p=thirdparty%2Fpostgresql.git Improve DROP SERVER handling of dependent subscriptions. We do not allow a DROP SERVER ... CASCADE to implicitly drop a subscription, because it's in a shared catalog and dropping a subscription has side effects. Instead we throw an error and the user must drop the subscription explicitly. Document this behavior and add a HINT to the error message. Generalize AcquireDeletionLock()/ReleaseDeletionLock() to use shared object locks for all shared catalogs, which includes AuthMemRelationId and now SubscriptionRelationId. Move error message after AcquireDeletionLock() to avoid an unnecessary error if there's a concurrent DROP SUBSCRIPTION. Addresses finding 10 & 15 in report from linked discussion. Reported-by: Noah Misch Discussion: https://postgr.es/m/20260710195902.4f.noahmisch@microsoft.com Backpatch-through: 19 --- diff --git a/doc/src/sgml/ref/drop_server.sgml b/doc/src/sgml/ref/drop_server.sgml index f83a661b3eb..5fa0b763f36 100644 --- a/doc/src/sgml/ref/drop_server.sgml +++ b/doc/src/sgml/ref/drop_server.sgml @@ -66,6 +66,10 @@ DROP SERVER [ IF EXISTS ] name [, . user mappings), and in turn all objects that depend on those objects (see ). + However, a subscription that uses the server is never dropped + automatically; it must be dropped with + DROP SUBSCRIPTION + before the server can be dropped. diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 52cd2caf9d4..c8dd78341eb 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -900,17 +900,6 @@ findDependentObjects(const ObjectAddress *object, object->objectSubId == 0) continue; - /* - * Check that the dependent object is not in a shared catalog, which - * is not supported by doDeletion(). - */ - if (IsSharedRelation(otherObject.classId)) - ereport(ERROR, - (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST), - errmsg("cannot drop %s because %s depends on it", - getObjectDescription(object, false), - getObjectDescription(&otherObject, false)))); - /* * Must lock the dependent object before recursing to it. */ @@ -931,6 +920,22 @@ findDependentObjects(const ObjectAddress *object, continue; } + /* + * Check that the dependent object is not in a shared catalog, which + * is not supported by doDeletion(). + */ + if (IsSharedRelation(otherObject.classId)) + { + char *otherObjDesc = getObjectDescription(&otherObject, + false); + + ereport(ERROR, + (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST), + errmsg("cannot drop %s because %s depends on it", + getObjectDescription(object, false), otherObjDesc), + errhint("Drop %s first.", otherObjDesc))); + } + /* * We do need to delete it, so identify objflags to be passed down, * which depend on the dependency type. @@ -1579,7 +1584,7 @@ AcquireDeletionLock(const ObjectAddress *object, int flags) else LockRelationOid(object->objectId, AccessExclusiveLock); } - else if (object->classId == AuthMemRelationId) + else if (IsSharedRelation(object->classId)) LockSharedObject(object->classId, object->objectId, 0, AccessExclusiveLock); else @@ -1600,7 +1605,7 @@ ReleaseDeletionLock(const ObjectAddress *object) { if (object->classId == RelationRelationId) UnlockRelationOid(object->objectId, AccessExclusiveLock); - else if (object->classId == AuthMemRelationId) + else if (IsSharedRelation(object->classId)) UnlockSharedObject(object->classId, object->objectId, 0, AccessExclusiveLock); else diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 6d89cec1503..229402826eb 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -205,6 +205,10 @@ ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection; WARNING: changing the foreign-data wrapper connection function can cause the options for dependent objects to become invalid DROP USER MAPPING FOR regress_subscription_user2 SERVER test_server; REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user2; +-- fail, subscription depends on the server and cannot be dropped by CASCADE +DROP SERVER test_server CASCADE; +ERROR: cannot drop server test_server because subscription regress_testsub6 depends on it +HINT: Drop subscription regress_testsub6 first. REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3; -- ok, lacks USAGE on test_server, but replacing connection anyway diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index cfee0b41224..03e047b8ce0 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -150,6 +150,8 @@ ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2; ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection; DROP USER MAPPING FOR regress_subscription_user2 SERVER test_server; REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user2; +-- fail, subscription depends on the server and cannot be dropped by CASCADE +DROP SERVER test_server CASCADE; REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3;