From: Jeff Davis Date: Thu, 18 Jun 2026 16:00:32 +0000 (-0700) Subject: Avoid errors during DROP SUBSCRIPTION when slot_name is NONE. X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=702e9dfd6c5001fd64d51c3c47dca2fc953fa9cd;p=thirdparty%2Fpostgresql.git Avoid errors during DROP SUBSCRIPTION when slot_name is NONE. Previously, if the subscription used a server, ForeignServerConnectionString() could raise an error (e.g. missing user mapping) during DROP SUBSCRIPTION even if the conninfo wasn't needed at all. Construct conninfo after the early return, so that if slot_name is NONE and rstates is NIL, the DROP SUBSCRIPTION will succeed even if ForeignServerConnectionString() raises an error (e.g. missing user mapping). If slot_name is NONE and rstates is not NIL, DROP SUBSCRIPTION may still encounter an error from ForeignServerConnectionString(). Reported-by: Hayato Kuroda (Fujitsu) Reviewed-by: Chao Li Discussion: https://postgr.es/m/OS9PR01MB12149B54DEA148108C6FA5667F52D2@OS9PR01MB12149.jpnprd01.prod.outlook.com --- diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index c9faf68cbc5..ee06a726f42 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -2260,6 +2260,43 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, return myself; } +/* + * Construct conninfo from a subscription's server. Like libpqrcv_connect(), + * if an error occurs, set *err to the error message and return NULL. + * + * However, failures in ForeignServerConnectionString() may ereport(ERROR), + * and (also like libpqrcv_connect) it's not worth adding the machinery to + * pass all of those back to the caller just to cover this one case. + */ +static char * +construct_subserver_conninfo(Oid subserver, Oid subowner, char **err) +{ + AclResult aclresult; + ForeignServer *server; + + *err = NULL; + + server = GetForeignServer(subserver); + + aclresult = object_aclcheck(ForeignServerRelationId, subserver, + 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(subowner, false), + server->servername); + return NULL; + } + + return ForeignServerConnectionString(subowner, server); +} + /* * Drop a subscription */ @@ -2271,10 +2308,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) HeapTuple tup; Oid subid; Oid subowner; + Oid subserver; + char *subconninfo = NULL; Datum datum; bool isnull; char *subname; - char *conninfo; + char *conninfo = NULL; char *slotname; List *subworkers; ListCell *lc; @@ -2313,9 +2352,15 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) return; } + datum = SysCacheGetAttr(SUBSCRIPTIONOID, tup, + Anum_pg_subscription_subconninfo, &isnull); + if (!isnull) + subconninfo = TextDatumGetCString(datum); + form = (Form_pg_subscription) GETSTRUCT(tup); subid = form->oid; subowner = form->subowner; + subserver = form->subserver; must_use_password = !superuser_arg(subowner) && form->subpasswordrequired; /* must be owner */ @@ -2337,39 +2382,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); @@ -2506,6 +2518,11 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) */ load_file("libpqwalreceiver", false); + if (OidIsValid(subserver)) + conninfo = construct_subserver_conninfo(subserver, subowner, &err); + else + conninfo = subconninfo; + if (conninfo) wrconn = walrcv_connect(conninfo, true, true, must_use_password, subname, &err); diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 61236672ce4..8dbfac66326 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -213,11 +213,12 @@ DROP USER MAPPING FOR regress_subscription_user3 SERVER test_server; BEGIN; ALTER SUBSCRIPTION regress_testsub6 CONNECTION 'dbname=regress_doesnotexist password=secret'; ABORT; -CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret'); +-- fails, cannot drop slot +DROP SUBSCRIPTION regress_testsub6; +ERROR: user mapping not found for user "regress_subscription_user3", server "test_server" ALTER SUBSCRIPTION regress_testsub6 DISABLE; ALTER SUBSCRIPTION regress_testsub6 SET (slot_name = NONE); DROP SUBSCRIPTION regress_testsub6; --ok -DROP USER MAPPING FOR regress_subscription_user3 SERVER test_server; SET SESSION AUTHORIZATION regress_subscription_user; REVOKE CREATE ON DATABASE REGRESSION FROM regress_subscription_user3; DROP SERVER test_server; diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index 665b510f180..05533d66675 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -164,14 +164,13 @@ BEGIN; ALTER SUBSCRIPTION regress_testsub6 CONNECTION 'dbname=regress_doesnotexist password=secret'; ABORT; -CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret'); +-- fails, cannot drop slot +DROP SUBSCRIPTION regress_testsub6; ALTER SUBSCRIPTION regress_testsub6 DISABLE; ALTER SUBSCRIPTION regress_testsub6 SET (slot_name = NONE); DROP SUBSCRIPTION regress_testsub6; --ok -DROP USER MAPPING FOR regress_subscription_user3 SERVER test_server; - SET SESSION AUTHORIZATION regress_subscription_user; REVOKE CREATE ON DATABASE REGRESSION FROM regress_subscription_user3;