postgres.git / summary / log / commit / refs
commit b66ea17a3a1383a24096ba6a3fe1e83c5c059a35
Author: Jeff Davis <jdavis@postgresql.org>
Date: Mon Aug 03 20:21:46 2026 +0000
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 <noah@leadboat.com>
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 | 31 +++++++++++++++++-------------
src/test/regress/expected/subscription.out | 4 ++++
src/test/regress/sql/subscription.sql | 2 ++
4 files changed, 28 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 ] <replaceable class="parameter">name</replaceable> [, .
user mappings),
and in turn all objects that depend on those objects
(see <xref linkend="ddl-depend"/>).
+ However, a subscription that uses the server is never dropped
+ automatically; it must be dropped with
+ <link linkend="sql-dropsubscription"><command>DROP SUBSCRIPTION</command></link>
+ before the server can be dropped.
</para>
</listitem>
</varlistentry>
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 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;
[parent: b48caa7dfde8]