upstream/postgresql
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-06-21 06:29:06 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Avoid errors during DROP SUBSCRIPTION when slot_name is NONE.

Browse source 
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) <kuroda.hayato@fujitsu.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/OS9PR01MB12149B54DEA148108C6FA5667F52D2@OS9PR01MB12149.jpnprd01.prod.outlook.com
This commit is contained in:
Jeff Davis 2026-06-18 09:00:32 -07:00
parent 9228275b04
commit 702e9dfd6c
3 changed files with 56 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

85
src/backend/commands/subscriptioncmds.c
View file

@ -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);

5
src/test/regress/expected/subscription.out
View file

@ -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;

5
src/test/regress/sql/subscription.sql
View file

@ -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;