From 9e9acf56b793334e80d32a38c183253bdbb237c5 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Thu, 30 Jul 2026 18:30:48 -0700 Subject: [PATCH v3 11/11] When changing owner of a subscription, do not throw an error. Errors will be caught when the connection is actually used. Restore uses multiple DDL commands to restore a subscription, so checks of the intermediate state risk restore errors. In the future we could address this with a more careful restoration order, but the DDL-time errors are merely for convenience. Addresses finding 2 in the report from the linked discussion. Reported-by: Noah Misch Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com Discussion: https://postgr.es/m/e103ae8daf74485e0c0ebde297fae735d38f54d1.camel@j-davis.com Backpatch-through: 19 --- src/backend/commands/subscriptioncmds.c | 24 +++++++--------------- src/test/regress/expected/subscription.out | 5 +++++ src/test/regress/sql/subscription.sql | 4 ++++ 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 54f35d41c27..fd7c4780acc 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -3006,25 +3006,15 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId) get_database_name(MyDatabaseId)); /* - * If the subscription uses a server, check that the new owner has USAGE - * privileges on the server and that a user mapping exists. Note: does not - * re-check the resulting connection string. + * The privileges will be checked before the connection is actually used, + * so it does not need to be done here. Avoid unnecessary risk of errors + * here, which could interfere with restore. + * + * However, it is convenient to check if a user mapping exists, and raise + * a WARNING if not. */ if (OidIsValid(form->subserver)) - { - ForeignServer *server = GetForeignServer(form->subserver); - - aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, newOwnerId, ACL_USAGE); - if (aclresult != ACLCHECK_OK) - ereport(ERROR, - errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("new subscription owner \"%s\" does not have permission on foreign server \"%s\"", - GetUserNameFromId(newOwnerId, false), - server->servername)); - - /* make sure a user mapping exists */ - GetUserMapping(newOwnerId, server->serverid); - } + GetUserMappingExtended(newOwnerId, form->subserver, WARNING); form->subowner = newOwnerId; CatalogTupleUpdate(rel, &tup->t_self, tup); diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index d447cba1397..f67ece3ccfd 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -187,6 +187,11 @@ CREATE SUBSCRIPTION regress_testsub6 SERVER test_server WARNING: subscription was created, but is not connected HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and alter the subscription to refresh publications. RESET SESSION AUTHORIZATION; +-- ok, USAGE privilege on server not checked for OWNER TO, but warn +-- about user mapping +ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2; +WARNING: user mapping not found for user "regress_subscription_user2", server "test_server" +ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user3; 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 dd33b18da55..266bc6d9deb 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -133,6 +133,10 @@ CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = 'dummy', connect = false); RESET SESSION AUTHORIZATION; +-- ok, USAGE privilege on server not checked for OWNER TO, but warn +-- about user mapping +ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2; +ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user3; REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3; -- 2.43.0