From f40a6c256c709014b1fbe7670f68061800d504ce Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Tue, 28 Jul 2026 14:48:12 -0700 Subject: [PATCH v2 2/3] Improve DROP SERVER handling of dependent subscriptions. Acquire a lock on the subscription to avoid unnecessary errors. Also issue a HINT and document the restriction that CASCADE won't cascade to the subscription object. Reported-by: Noah Misch Discussion: https://postgr.es/m/20260710195902.4f.noahmisch@microsoft.com Backpatch-through: 19 --- doc/src/sgml/ref/drop_server.sgml | 4 ++++ src/backend/catalog/dependency.c | 28 ++++++++++++---------- src/test/regress/expected/subscription.out | 4 ++++ src/test/regress/sql/subscription.sql | 2 ++ 4 files changed, 25 insertions(+), 13 deletions(-) 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..b80949a5eeb 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,19 @@ 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)) + ereport(ERROR, + (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST), + errmsg("cannot drop %s because %s depends on it", + getObjectDescription(object, false), + getObjectDescription(&otherObject, false)), + errhint("Drop %s first.", + getObjectDescription(&otherObject, false)))); + /* * We do need to delete it, so identify objflags to be passed down, * which depend on the dependency type. @@ -1579,7 +1581,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 +1602,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 1bb785f4f9f..259db747334 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 f19740fdfb8..7718c742974 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; -- 2.43.0