mirror of
https://github.com/postgres/postgres.git
synced 2026-10-23 13:29:09 +03:00
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
This commit is contained in:
@@ -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>
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user