From 39b2a13ee7904256fc100a0f92104c056386712e Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Fri, 8 May 2026 17:16:50 -0700 Subject: [PATCH v4 2/2] Avoid errors during DROP SUBSCRIPTION. Previously, an error during ForeignServerConnectionString() could prevent a subscription from being dropped, even if slot_name was NULL. ACL check errors were handled, but not other errors, such as a dropped user mapping. Change to only construct conninfo if slotname is defined. Errors (aside from ACL check failures) will not benefit from the more helpful ReportSlotConnectionError(), but at least they will not interfere with a user fixing the problem. Reported-by: Hayato Kuroda (Fujitsu) Discussion: https://postgr.es/m/OS9PR01MB12149B54DEA148108C6FA5667F52D2@OS9PR01MB12149.jpnprd01.prod.outlook.com --- src/backend/commands/subscriptioncmds.c | 71 +++++++++++++------------ 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 21b27991d3c..498a102200e 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -2193,7 +2193,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) Datum datum; bool isnull; char *subname; - char *conninfo; + char *conninfo = NULL; char *slotname; List *subworkers; ListCell *lc; @@ -2256,39 +2256,6 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) Anum_pg_subscription_subname); subname = pstrdup(NameStr(*DatumGetName(datum))); - /* Get conninfo */ - if (OidIsValid(form->subserver)) - { - AclResult aclresult; - ForeignServer *server; - - server = GetForeignServer(form->subserver); - aclresult = object_aclcheck(ForeignServerRelationId, form->subserver, - form->subowner, ACL_USAGE); - if (aclresult != ACLCHECK_OK) - { - /* - * Unable to generate connection string because permissions on the - * foreign server have been removed. Follow the same logic as an - * unusable subconninfo (which will result in an ERROR later - * unless slot_name = NONE). - */ - err = psprintf(_("subscription owner \"%s\" does not have permission on foreign server \"%s\""), - GetUserNameFromId(form->subowner, false), - server->servername); - conninfo = NULL; - } - else - conninfo = ForeignServerConnectionString(form->subowner, - server); - } - else - { - datum = SysCacheGetAttrNotNull(SUBSCRIPTIONOID, tup, - Anum_pg_subscription_subconninfo); - conninfo = TextDatumGetCString(datum); - } - /* Get slotname */ datum = SysCacheGetAttr(SUBSCRIPTIONOID, tup, Anum_pg_subscription_subslotname, &isnull); @@ -2297,6 +2264,42 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) else slotname = NULL; + /* conninfo only needed if slotname is valid */ + if (slotname) + { + if (OidIsValid(form->subserver)) + { + AclResult aclresult; + ForeignServer *server; + + server = GetForeignServer(form->subserver); + aclresult = object_aclcheck(ForeignServerRelationId, form->subserver, + form->subowner, ACL_USAGE); + if (aclresult != ACLCHECK_OK) + { + /* + * Unable to generate connection string because permissions on + * the foreign server have been removed. Follow the same logic + * as an unusable subconninfo and ReportSlotConnectionError() + * for a more helpful error. + */ + err = psprintf(_("subscription owner \"%s\" does not have permission on foreign server \"%s\""), + GetUserNameFromId(form->subowner, false), + server->servername); + conninfo = NULL; + } + else + conninfo = ForeignServerConnectionString(form->subowner, + server); + } + else + { + datum = SysCacheGetAttrNotNull(SUBSCRIPTIONOID, tup, + Anum_pg_subscription_subconninfo); + conninfo = TextDatumGetCString(datum); + } + } + /* * Since dropping a replication slot is not transactional, the replication * slot stays dropped even if the transaction rolls back. So we cannot -- 2.43.0