postgres.git / summary / log / commit / refs
commit a1e39c501d7a89a01c2738fecf7b66927e4e8bb7
Author: Jeff Davis <jdavis@postgresql.org>
Date: Wed Aug 05 20:19:21 2026 +0000
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 <noah@leadboat.com>
Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
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 50a5e279252..fd28c978c6e 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -2790,25 +2790,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 c034b25f3d5..7163b756787 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -191,6 +191,11 @@ RESET SESSION AUTHORIZATION;
DROP SERVER test_server CASCADE;
ERROR: cannot drop server test_server because subscription regress_testsub6 depends on it
HINT: Drop subscription regress_testsub6 first.
+-- 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 cbdaaa9294d..1a6fd89854a 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -136,6 +136,10 @@ RESET SESSION AUTHORIZATION;
-- fail, subscription depends on the server and cannot be dropped by CASCADE
DROP SERVER test_server CASCADE;
+-- 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;
[parent: 45b479a836ec]