On Wed, 2026-07-29 at 14:30 -0700, Jeff Davis wrote:
> Proposal:
>
> * Form a conninfo if and only if a connection is immediately
> required. That is, it's one of the DDL commands above, or a
> logical worker.
>
> * Check USAGE on the server when a connection is formed or when
> ALTER sets the server or when the subscription owner changes
> (unless superuser changes the owner, in which case it may be part
> of a multi-command DDL sequence during restore).
>
> * Check that the server's FDW supports a connection function when
> DDL sets the subscription's server.
>
> * Check that a user mapping exists during DDL when the server or
> owner changes, but demote the message to a WARNING, because it
> may be part of a multi-command DDL sequence during restore.
> - CREATE SUBSCRIPTION already issues WARNINGS during restore.
>
> * Ensure that none of the commands during restore need a connection.
> - check_pub_rdt should happen at connection time, and only
> opportunistically at DDL time if already forming a connection
>
> * Check walrcv_check_conninfo() before connecting, or during
> CREATE/ALTER SUBSCRIPTION ... CONNECTION.
> - If a connection is not needed for DDL, and it's a server-based
> subscription, walrcv_check_conninfo() will be called only by
> the logical worker when a connection is needed.
> - That loses some convenience for interactive DDL, but avoids
> false positive failures during restore.
Here's a consolidated series of commits.
Fujii, note that this includes a revert of your commit 1c9c358904.
Doing too much validation can cause problems for restore.
Amit, this series does not include the check_pub_rdt change to move it
to the worker.
Shlok Kyal, the change to detect when a refresh is happening is in my
patch 0006, the ACL check fix is in my patch 0007.
Hayato Kuroda, Amit already pushed the --no-subscriptions fix
(02decf9a9a). I didn't change the dump/restore order in this series,
because reducing the errors seems right for v19. We can reconsider it
for v20.
Regards,
Jeff Davis
From a0150ddf088a622d38eac247ebd7ff862e7fb7a8 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Tue, 28 Jul 2026 14:50:45 -0700
Subject: [PATCH v3 01/11] Fix lock release for role membership grants in DROP
OWNED BY.
Commit 6566133c5f5 added a case for AuthMemRelationId in
AcquireDeletionLock(), but not ReleaseDeletionLock(). The fall-through
case would go to UnlockDatabaseObject(), which would raise a WARNING;
and the lock would be retained until the end of the transaction.
Add the missing branch.
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 16
---
src/backend/catalog/dependency.c | 3 ++
.../isolation/expected/drop-owned-grant.out | 8 +++++
src/test/isolation/isolation_schedule | 1 +
.../isolation/specs/drop-owned-grant.spec | 30 +++++++++++++++++++
4 files changed, 42 insertions(+)
create mode 100644 src/test/isolation/expected/drop-owned-grant.out
create mode 100644 src/test/isolation/specs/drop-owned-grant.spec
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index c54774b3275..52cd2caf9d4 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1600,6 +1600,9 @@ ReleaseDeletionLock(const ObjectAddress *object)
{
if (object->classId == RelationRelationId)
UnlockRelationOid(object->objectId, AccessExclusiveLock);
+ else if (object->classId == AuthMemRelationId)
+ UnlockSharedObject(object->classId, object->objectId, 0,
+ AccessExclusiveLock);
else
/* assume we should lock the whole object not a sub-object */
UnlockDatabaseObject(object->classId, object->objectId, 0,
diff --git a/src/test/isolation/expected/drop-owned-grant.out b/src/test/isolation/expected/drop-owned-grant.out
new file mode 100644
index 00000000000..ea6cca277b6
--- /dev/null
+++ b/src/test/isolation/expected/drop-owned-grant.out
@@ -0,0 +1,8 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1b s1d s2d s1c
+step s1b: BEGIN;
+step s1d: DROP OWNED BY regress_dropowned_grantor;
+step s2d: DROP OWNED BY regress_dropowned_grantor; <waiting ...>
+step s1c: COMMIT;
+step s2d: <... completed>
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 26abed9f9f0..df8ce44ede6 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -129,3 +129,4 @@ test: lock-nowait
test: for-portion-of
test: ddl-dependency-locking
test: pub-concurrent-drop
+test: drop-owned-grant
diff --git a/src/test/isolation/specs/drop-owned-grant.spec b/src/test/isolation/specs/drop-owned-grant.spec
new file mode 100644
index 00000000000..636cc213a6b
--- /dev/null
+++ b/src/test/isolation/specs/drop-owned-grant.spec
@@ -0,0 +1,30 @@
+# Test locking of role membership grants during concurrent DROP OWNED BY.
+
+setup
+{
+ CREATE ROLE regress_dropowned_role;
+ CREATE ROLE regress_dropowned_member;
+ CREATE ROLE regress_dropowned_grantor;
+ GRANT regress_dropowned_role TO regress_dropowned_grantor
+ WITH ADMIN OPTION;
+ SET ROLE regress_dropowned_grantor;
+ GRANT regress_dropowned_role TO regress_dropowned_member;
+ RESET ROLE;
+}
+
+teardown
+{
+ DROP ROLE regress_dropowned_member;
+ DROP ROLE regress_dropowned_grantor;
+ DROP ROLE regress_dropowned_role;
+}
+
+session s1
+step s1b { BEGIN; }
+step s1d { DROP OWNED BY regress_dropowned_grantor; }
+step s1c { COMMIT; }
+
+session s2
+step s2d { DROP OWNED BY regress_dropowned_grantor; }
+
+permutation s1b s1d s2d s1c
--
2.43.0
From deea81eb5b65343a6635acdaeb897ea91cd16e04 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Tue, 28 Jul 2026 14:48:12 -0700
Subject: [PATCH v3 02/11] Improve DROP SERVER handling of dependent
subscriptions.
Acquire a lock on the subscription to avoid unnecessary errors. Also
issue a HINT and document the restriction that CASCADE won't cascade
to the subscription object.
Addresses finding 10 & 15 in report from linked discussion.
Reported-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 19
---
doc/src/sgml/ref/drop_server.sgml | 4 +++
src/backend/catalog/dependency.c | 31 +++++++++++++---------
src/test/regress/expected/subscription.out | 4 +++
src/test/regress/sql/subscription.sql | 2 ++
4 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/ref/drop_server.sgml b/doc/src/sgml/ref/drop_server.sgml
index f83a661b3eb..5fa0b763f36 100644
--- a/doc/src/sgml/ref/drop_server.sgml
+++ b/doc/src/sgml/ref/drop_server.sgml
@@ -66,6 +66,10 @@ DROP SERVER [ IF EXISTS ] <replaceable class="parameter">name</replaceable> [, .
user mappings),
and in turn all objects that depend on those objects
(see <xref linkend="ddl-depend"/>).
+ However, a subscription that uses the server is never dropped
+ automatically; it must be dropped with
+ <link linkend="sql-dropsubscription"><command>DROP SUBSCRIPTION</command></link>
+ before the server can be dropped.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 52cd2caf9d4..c8dd78341eb 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -900,17 +900,6 @@ findDependentObjects(const ObjectAddress *object,
object->objectSubId == 0)
continue;
- /*
- * Check that the dependent object is not in a shared catalog, which
- * is not supported by doDeletion().
- */
- if (IsSharedRelation(otherObject.classId))
- ereport(ERROR,
- (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
- errmsg("cannot drop %s because %s depends on it",
- getObjectDescription(object, false),
- getObjectDescription(&otherObject, false))));
-
/*
* Must lock the dependent object before recursing to it.
*/
@@ -931,6 +920,22 @@ findDependentObjects(const ObjectAddress *object,
continue;
}
+ /*
+ * Check that the dependent object is not in a shared catalog, which
+ * is not supported by doDeletion().
+ */
+ if (IsSharedRelation(otherObject.classId))
+ {
+ char *otherObjDesc = getObjectDescription(&otherObject,
+ false);
+
+ ereport(ERROR,
+ (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+ errmsg("cannot drop %s because %s depends on it",
+ getObjectDescription(object, false), otherObjDesc),
+ errhint("Drop %s first.", otherObjDesc)));
+ }
+
/*
* We do need to delete it, so identify objflags to be passed down,
* which depend on the dependency type.
@@ -1579,7 +1584,7 @@ AcquireDeletionLock(const ObjectAddress *object, int flags)
else
LockRelationOid(object->objectId, AccessExclusiveLock);
}
- else if (object->classId == AuthMemRelationId)
+ else if (IsSharedRelation(object->classId))
LockSharedObject(object->classId, object->objectId, 0,
AccessExclusiveLock);
else
@@ -1600,7 +1605,7 @@ ReleaseDeletionLock(const ObjectAddress *object)
{
if (object->classId == RelationRelationId)
UnlockRelationOid(object->objectId, AccessExclusiveLock);
- else if (object->classId == AuthMemRelationId)
+ else if (IsSharedRelation(object->classId))
UnlockSharedObject(object->classId, object->objectId, 0,
AccessExclusiveLock);
else
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 1bb785f4f9f..259db747334 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -205,6 +205,10 @@ ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection;
WARNING: changing the foreign-data wrapper connection function can cause the options for dependent objects to become invalid
DROP USER MAPPING FOR regress_subscription_user2 SERVER test_server;
REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user2;
+-- fail, subscription depends on the server and cannot be dropped by CASCADE
+DROP SERVER test_server CASCADE;
+ERROR: cannot drop server test_server because subscription regress_testsub6 depends on it
+HINT: Drop subscription regress_testsub6 first.
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 f19740fdfb8..7718c742974 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -150,6 +150,8 @@ ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2;
ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection;
DROP USER MAPPING FOR regress_subscription_user2 SERVER test_server;
REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user2;
+-- fail, subscription depends on the server and cannot be dropped by CASCADE
+DROP SERVER test_server CASCADE;
REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3;
SET SESSION AUTHORIZATION regress_subscription_user3;
--
2.43.0
From da32e6ad7fc6d2a006340a2640bc2880caf27b97 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Tue, 28 Jul 2026 13:50:29 -0700
Subject: [PATCH v3 03/11] postgres_fdw: reject use_scram_passthrough for
subscriptions.
The subscription is initiated from a logical replication worker, so
SCRAM pass-through won't work.
Partially addresses finding 3 in report from linked discussion.
Reported-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 19
---
contrib/postgres_fdw/connection.c | 12 ++++++++++++
contrib/postgres_fdw/t/010_subscription.pl | 16 +++++++++++++++-
doc/src/sgml/postgres-fdw.sgml | 7 +++++++
3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index aab21695979..094eac2f343 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -2479,6 +2479,18 @@ postgres_fdw_connection(PG_FUNCTION_ARGS)
char *appname;
char *sep = "";
+ /*
+ * SCRAM pass-through cannot work for subscriptions because the connection
+ * happens in a worker process.
+ */
+ if (UseScramPassthrough(server, user))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("SCRAM pass-through authentication is not supported for subscription connections"),
+ errdetail("The foreign server or user mapping for user \"%s\" has \"use_scram_passthrough\" enabled.",
+ GetUserNameFromId(userid, false)),
+ errhint("Store a password in the user mapping instead.")));
+
construct_connection_params(server, user, &keywords, &values, &appname);
initStringInfo(&str);
diff --git a/contrib/postgres_fdw/t/010_subscription.pl b/contrib/postgres_fdw/t/010_subscription.pl
index c34b3d15b8d..449fa35ce31 100644
--- a/contrib/postgres_fdw/t/010_subscription.pl
+++ b/contrib/postgres_fdw/t/010_subscription.pl
@@ -41,7 +41,21 @@ $node_subscriber->safe_psql('postgres',
);
$node_subscriber->safe_psql('postgres',
- "CREATE USER MAPPING FOR PUBLIC SERVER tap_server");
+ "CREATE USER MAPPING FOR PUBLIC SERVER tap_server OPTIONS (use_scram_passthrough 'true')"
+);
+
+my ($ret, $stdout, $stderr) = $node_subscriber->psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub SERVER tap_server PUBLICATION tap_pub WITH (password_required=false)"
+);
+isnt($ret, 0, 'CREATE SUBSCRIPTION fails with use_scram_passthrough');
+like(
+ $stderr,
+ qr/ERROR.*SCRAM pass-through authentication is not supported for subscription connections/,
+ 'CREATE SUBSCRIPTION gives correct connection error');
+
+$node_subscriber->safe_psql('postgres',
+ "ALTER USER MAPPING FOR PUBLIC SERVER tap_server OPTIONS (DROP use_scram_passthrough)"
+);
$node_subscriber->safe_psql('postgres',
"CREATE SUBSCRIPTION tap_sub SERVER tap_server PUBLICATION tap_pub WITH (password_required=false)"
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index b9e1b04463e..8b0669f672d 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -861,6 +861,13 @@ OPTIONS (ADD password_required 'false');
This is a technical requirement of the SCRAM protocol.
</para>
</listitem>
+
+ <listitem>
+ <para>
+ The foreign server must not be used for subscription connections
+ (see <xref linkend="postgres-fdw-server-subscription"/>).
+ </para>
+ </listitem>
</itemizedlist>
</para>
</listitem>
--
2.43.0
From c7eb3abedf60408d776f0906398c896a815c44a5 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Thu, 30 Jul 2026 12:34:03 -0700
Subject: [PATCH v3 04/11] Remove Subscription conninfo field; generate in
caller.
After server-based subscriptions, conninfo became more than just a
catalog field. It has its own error paths, and it's important that
callers that don't need conninfo don't encounter errors related to it.
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com
Backpatch-through: 19
---
src/backend/catalog/pg_subscription.c | 100 ++++++++++--------
src/backend/commands/subscriptioncmds.c | 43 ++++++--
.../replication/logical/sequencesync.c | 2 +-
src/backend/replication/logical/tablesync.c | 2 +-
src/backend/replication/logical/worker.c | 24 ++++-
src/include/catalog/pg_subscription.h | 6 +-
src/include/replication/worker_internal.h | 1 +
7 files changed, 112 insertions(+), 66 deletions(-)
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index 5ff61edb989..8fa2a460a25 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -79,14 +79,10 @@ GetPublicationsStr(List *publications, StringInfo dest, bool quote_literal)
/*
* Fetch the subscription from the syscache.
*
- * If conninfo_needed is true, conninfo will be constructed, possibly
- * encountering errors in ForeignServerConnectionString(). Callers not
- * expecting such errors should pass false, in which case conninfo will be
- * NULL.
+ * Callers that need conninfo must call SubscriptionConninfo().
*/
Subscription *
-GetSubscription(Oid subid, bool missing_ok, bool conninfo_needed,
- bool conninfo_aclcheck)
+GetSubscription(Oid subid, bool missing_ok)
{
HeapTuple tup;
Subscription *sub;
@@ -96,8 +92,6 @@ GetSubscription(Oid subid, bool missing_ok, bool conninfo_needed,
MemoryContext cxt;
MemoryContext oldcxt;
- Assert(conninfo_needed || !conninfo_aclcheck);
-
tup = SearchSysCache1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid));
if (!HeapTupleIsValid(tup))
@@ -140,42 +134,6 @@ GetSubscription(Oid subid, bool missing_ok, bool conninfo_needed,
sub->retentionactive = subform->subretentionactive;
sub->conflictlogrelid = subform->subconflictlogrelid;
- if (conninfo_needed)
- {
- if (OidIsValid(subform->subserver))
- {
- AclResult aclresult;
- ForeignServer *server;
-
- server = GetForeignServer(subform->subserver);
-
- if (conninfo_aclcheck)
- {
- /* recheck ACL if requested */
- aclresult = object_aclcheck(ForeignServerRelationId,
- subform->subserver,
- subform->subowner, ACL_USAGE);
-
- if (aclresult != ACLCHECK_OK)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("subscription owner \"%s\" does not have permission on foreign server \"%s\"",
- GetUserNameFromId(subform->subowner, false),
- server->servername)));
- }
-
- sub->conninfo = ForeignServerConnectionString(subform->subowner,
- server);
- }
- else
- {
- datum = SysCacheGetAttrNotNull(SUBSCRIPTIONOID,
- tup,
- Anum_pg_subscription_subconninfo);
- sub->conninfo = TextDatumGetCString(datum);
- }
- }
-
/* Get slotname */
datum = SysCacheGetAttr(SUBSCRIPTIONOID,
tup,
@@ -226,6 +184,60 @@ GetSubscription(Oid subid, bool missing_ok, bool conninfo_needed,
return sub;
}
+/*
+ * Generate conninfo string for subscription.
+ *
+ * For server-based subscriptions this may raise an error (e.g. due to a
+ * missing user mapping).
+ */
+char *
+SubscriptionConninfo(Subscription *sub, bool aclcheck)
+{
+ HeapTuple tup;
+ Form_pg_subscription subform;
+ Datum datum;
+ char *conninfo;
+
+ tup = SearchSysCache1(SUBSCRIPTIONOID, ObjectIdGetDatum(sub->oid));
+ if (!HeapTupleIsValid(tup))
+ elog(ERROR, "cache lookup failed for subscription %u", sub->oid);
+
+ subform = (Form_pg_subscription) GETSTRUCT(tup);
+
+ if (OidIsValid(subform->subserver))
+ {
+ ForeignServer *server;
+ AclResult aclresult;
+
+ server = GetForeignServer(subform->subserver);
+
+ if (aclcheck)
+ {
+ aclresult = object_aclcheck(ForeignServerRelationId,
+ subform->subserver,
+ sub->owner, ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("subscription owner \"%s\" does not have permission on foreign server \"%s\"",
+ GetUserNameFromId(sub->owner, false),
+ server->servername)));
+ }
+
+ conninfo = ForeignServerConnectionString(sub->owner, server);
+ }
+ else
+ {
+ datum = SysCacheGetAttrNotNull(SUBSCRIPTIONOID, tup,
+ Anum_pg_subscription_subconninfo);
+ conninfo = TextDatumGetCString(datum);
+ }
+
+ ReleaseSysCache(tup);
+
+ return conninfo;
+}
+
/*
* Return number of subscriptions defined in given database.
* Used by dropdb() to check if database can indeed be dropped.
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 013ac46db07..6ca215f8bf3 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1088,7 +1088,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
static void
AlterSubscription_refresh(Subscription *sub, bool copy_data,
- List *validate_publications)
+ List *validate_publications, char *conninfo)
{
char *err;
List *pubrels = NIL;
@@ -1112,12 +1112,19 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data,
WalReceiverConn *wrconn;
bool must_use_password;
+ /*
+ * Should not happen: CREATE/ALTER/DROP SUBSCRIPTION did not call
+ * SubscriptionConninfo() in a path where it's required.
+ */
+ if (!conninfo)
+ elog(ERROR, "no connection string provided for subscription");
+
/* Load the library providing us libpq calls. */
load_file("libpqwalreceiver", false);
/* Try to connect to the publisher. */
must_use_password = sub->passwordrequired && !sub->ownersuperuser;
- wrconn = walrcv_connect(sub->conninfo, true, true, must_use_password,
+ wrconn = walrcv_connect(conninfo, true, true, must_use_password,
sub->name, &err);
if (!wrconn)
ereport(ERROR,
@@ -1358,19 +1365,26 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data,
* Marks all sequences with INIT state.
*/
static void
-AlterSubscription_refresh_seq(Subscription *sub)
+AlterSubscription_refresh_seq(Subscription *sub, char *conninfo)
{
char *err = NULL;
WalReceiverConn *wrconn;
bool must_use_password;
List *subrel_states;
+ /*
+ * Should not happen: CREATE/ALTER/DROP SUBSCRIPTION did not call
+ * SubscriptionConninfo() in a path where it's required.
+ */
+ if (!conninfo)
+ elog(ERROR, "no connection string provided for subscription");
+
/* Load the library providing us libpq calls. */
load_file("libpqwalreceiver", false);
/* Try to connect to the publisher. */
must_use_password = sub->passwordrequired && !sub->ownersuperuser;
- wrconn = walrcv_connect(sub->conninfo, true, true, must_use_password,
+ wrconn = walrcv_connect(conninfo, true, true, must_use_password,
sub->name, &err);
if (!wrconn)
ereport(ERROR,
@@ -1627,6 +1641,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
int max_retention;
bool retention_active;
char *new_conninfo = NULL;
+ char *orig_conninfo = NULL;
char *origin;
Subscription *sub;
Form_pg_subscription form;
@@ -1729,6 +1744,8 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
orig_conninfo_needed = false;
}
+ sub = GetSubscription(subid, false);
+
/*
* Skip ACL checks on the subscription's foreign server, if any. If
* changing the server (or replacing it with a raw connection), then the
@@ -1736,7 +1753,8 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
* there's no need to do an additional ACL check here; that will be done
* by the subscription worker.
*/
- sub = GetSubscription(subid, false, orig_conninfo_needed, false);
+ if (orig_conninfo_needed)
+ orig_conninfo = SubscriptionConninfo(sub, false);
retain_dead_tuples = sub->retaindeadtuples;
origin = sub->origin;
@@ -2227,7 +2245,8 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
sub->publications = stmt->publication;
AlterSubscription_refresh(sub, opts.copy_data,
- stmt->publication);
+ stmt->publication,
+ orig_conninfo);
}
break;
@@ -2282,7 +2301,8 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
sub->publications = publist;
AlterSubscription_refresh(sub, opts.copy_data,
- validate_publications);
+ validate_publications,
+ orig_conninfo);
}
break;
@@ -2321,7 +2341,8 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
PreventInTransactionBlock(isTopLevel, "ALTER SUBSCRIPTION ... REFRESH PUBLICATION");
- AlterSubscription_refresh(sub, opts.copy_data, NULL);
+ AlterSubscription_refresh(sub, opts.copy_data, NULL,
+ orig_conninfo);
break;
}
@@ -2334,7 +2355,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
errmsg("%s is not allowed for disabled subscriptions",
"ALTER SUBSCRIPTION ... REFRESH SEQUENCES"));
- AlterSubscription_refresh_seq(sub);
+ AlterSubscription_refresh_seq(sub, orig_conninfo);
break;
}
@@ -2406,7 +2427,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
char *err;
WalReceiverConn *wrconn;
- Assert(new_conninfo || orig_conninfo_needed);
+ Assert(new_conninfo || orig_conninfo);
/* Load the library providing us libpq calls. */
load_file("libpqwalreceiver", false);
@@ -2416,7 +2437,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
* available.
*/
must_use_password = sub->passwordrequired && !sub->ownersuperuser;
- wrconn = walrcv_connect(new_conninfo ? new_conninfo : sub->conninfo,
+ wrconn = walrcv_connect(new_conninfo ? new_conninfo : orig_conninfo,
true, true, must_use_password, sub->name,
&err);
if (!wrconn)
diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c
index 0423745a428..8b187d15822 100644
--- a/src/backend/replication/logical/sequencesync.c
+++ b/src/backend/replication/logical/sequencesync.c
@@ -815,7 +815,7 @@ LogicalRepSyncSequences(void)
* Establish the connection to the publisher for sequence synchronization.
*/
LogRepWorkerWalRcvConn =
- walrcv_connect(MySubscription->conninfo, true, true,
+ walrcv_connect(MySubscriptionConninfo, true, true,
must_use_password,
app_name.data, &err);
if (LogRepWorkerWalRcvConn == NULL)
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index a04b84ebc1d..e5101997cd3 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -1305,7 +1305,7 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos)
* so that synchronous replication can distinguish them.
*/
LogRepWorkerWalRcvConn =
- walrcv_connect(MySubscription->conninfo, true, true,
+ walrcv_connect(MySubscriptionConninfo, true, true,
must_use_password,
slotname, &err);
if (LogRepWorkerWalRcvConn == NULL)
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 1ca19c1a7a8..74409fc9202 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -482,6 +482,7 @@ static MemoryContext LogicalStreamingContext = NULL;
WalReceiverConn *LogRepWorkerWalRcvConn = NULL;
Subscription *MySubscription = NULL;
+char *MySubscriptionConninfo = NULL;
static bool MySubscriptionValid = false;
static List *on_commit_wakeup_workers_subids = NIL;
@@ -5061,6 +5062,7 @@ void
maybe_reread_subscription(void)
{
Subscription *newsub;
+ char *new_conninfo;
bool started_tx = false;
/* When cache state is valid there is nothing to do here. */
@@ -5074,7 +5076,7 @@ maybe_reread_subscription(void)
started_tx = true;
}
- newsub = GetSubscription(MyLogicalRepWorker->subid, true, true, true);
+ newsub = GetSubscription(MyLogicalRepWorker->subid, true);
if (newsub)
{
@@ -5097,6 +5099,9 @@ maybe_reread_subscription(void)
proc_exit(0);
}
+ /* allocated in transaction context */
+ new_conninfo = SubscriptionConninfo(newsub, true);
+
/* Exit if the subscription was disabled. */
if (!newsub->enabled)
{
@@ -5120,7 +5125,7 @@ maybe_reread_subscription(void)
* 'parallel' to any other value or the server decides not to stream the
* in-progress transaction.
*/
- if (strcmp(newsub->conninfo, MySubscription->conninfo) != 0 ||
+ if (strcmp(new_conninfo, MySubscriptionConninfo) != 0 ||
strcmp(newsub->name, MySubscription->name) != 0 ||
strcmp(newsub->slotname, MySubscription->slotname) != 0 ||
newsub->binary != MySubscription->binary ||
@@ -5171,6 +5176,10 @@ maybe_reread_subscription(void)
MemoryContextDelete(MySubscription->cxt);
MySubscription = newsub;
+ /* Owned by ApplyContext */
+ pfree(MySubscriptionConninfo);
+ MySubscriptionConninfo = MemoryContextStrdup(ApplyContext, new_conninfo);
+
/* Change synchronous commit according to the user's wishes */
SetConfigOption("synchronous_commit", MySubscription->synccommit,
PGC_BACKEND, PGC_S_OVERRIDE);
@@ -5718,7 +5727,7 @@ run_apply_worker(void)
must_use_password = MySubscription->passwordrequired &&
!MySubscription->ownersuperuser;
- LogRepWorkerWalRcvConn = walrcv_connect(MySubscription->conninfo, true,
+ LogRepWorkerWalRcvConn = walrcv_connect(MySubscriptionConninfo, true,
true, must_use_password,
MySubscription->name, &err);
@@ -5831,7 +5840,7 @@ InitializeLogRepWorker(void)
LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
AccessShareLock);
- MySubscription = GetSubscription(MyLogicalRepWorker->subid, true, true, true);
+ MySubscription = GetSubscription(MyLogicalRepWorker->subid, true);
if (MySubscription)
{
@@ -5850,6 +5859,11 @@ InitializeLogRepWorker(void)
proc_exit(0);
}
+ /* build conninfo in transaction context and copy to ApplyContext */
+ MySubscriptionConninfo =
+ MemoryContextStrdup(ApplyContext,
+ SubscriptionConninfo(MySubscription, true));
+
MySubscriptionValid = true;
if (!MySubscription->enabled)
@@ -6005,7 +6019,7 @@ SetupApplyOrSyncWorker(int worker_slot)
/* Connect to the origin and start the replication. */
elog(DEBUG1, "connecting to publisher using connection string \"%s\"",
- MySubscription->conninfo);
+ MySubscriptionConninfo);
/*
* Setup callback for syscache so that we know when something changes in
diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index 65ce8e145fb..5a9c07fe8d6 100644
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -173,7 +173,6 @@ typedef struct Subscription
* exceeded max_retention_duration, when
* defined */
Oid conflictlogrelid; /* conflict log table Oid */
- char *conninfo; /* Connection string to the publisher */
char *slotname; /* Name of the replication slot */
char *synccommit; /* Synchronous commit setting for worker */
char *walrcvtimeout; /* wal_receiver_timeout setting for worker */
@@ -222,9 +221,8 @@ typedef struct Subscription
#endif /* EXPOSE_TO_CLIENT_CODE */
-extern Subscription *GetSubscription(Oid subid, bool missing_ok,
- bool conninfo_needed,
- bool conninfo_aclcheck);
+extern Subscription *GetSubscription(Oid subid, bool missing_ok);
+extern char *SubscriptionConninfo(Subscription *sub, bool aclcheck);
extern void DisableSubscription(Oid subid);
extern int CountDBSubscriptions(Oid dbid);
diff --git a/src/include/replication/worker_internal.h b/src/include/replication/worker_internal.h
index 745b7d9e969..88cb7c1e252 100644
--- a/src/include/replication/worker_internal.h
+++ b/src/include/replication/worker_internal.h
@@ -247,6 +247,7 @@ extern PGDLLIMPORT struct WalReceiverConn *LogRepWorkerWalRcvConn;
/* Worker and subscription objects. */
extern PGDLLIMPORT Subscription *MySubscription;
+extern PGDLLIMPORT char *MySubscriptionConninfo;
extern PGDLLIMPORT LogicalRepWorker *MyLogicalRepWorker;
extern PGDLLIMPORT bool in_remote_transaction;
--
2.43.0
From 37e6fcdf9e10aecb6aab08952c449aca98b35e10 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Thu, 30 Jul 2026 13:38:17 -0700
Subject: [PATCH v3 05/11] Build subscription conninfo after checking that it's
enabled.
If a subscription is disabled, don't try to build conninfo because
that may generate a confusing error and try to disable an
already-disabled subscription.
Partially addresses finding 5 in report from linked discussion.
Reported-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com
Backpatch-through: 19
---
src/backend/replication/logical/worker.c | 28 +++++++++++++++---------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 74409fc9202..1cdd28f5049 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5099,9 +5099,6 @@ maybe_reread_subscription(void)
proc_exit(0);
}
- /* allocated in transaction context */
- new_conninfo = SubscriptionConninfo(newsub, true);
-
/* Exit if the subscription was disabled. */
if (!newsub->enabled)
{
@@ -5112,6 +5109,13 @@ maybe_reread_subscription(void)
apply_worker_exit();
}
+ /*
+ * May raise error, so build conninfo after checking that the subscription
+ * is enabled. Allocated in transaction context; must be copied to
+ * ApplyContext when we set MySubscriptionConninfo.
+ */
+ new_conninfo = SubscriptionConninfo(newsub, true);
+
/* !slotname should never happen when enabled is true. */
Assert(newsub->slotname);
@@ -5859,13 +5863,6 @@ InitializeLogRepWorker(void)
proc_exit(0);
}
- /* build conninfo in transaction context and copy to ApplyContext */
- MySubscriptionConninfo =
- MemoryContextStrdup(ApplyContext,
- SubscriptionConninfo(MySubscription, true));
-
- MySubscriptionValid = true;
-
if (!MySubscription->enabled)
{
ereport(LOG,
@@ -5875,6 +5872,17 @@ InitializeLogRepWorker(void)
apply_worker_exit();
}
+ /*
+ * May raise error for server-based subscriptions, so build conninfo after
+ * checking that the subscription is enabled. Build in transaction context
+ * and copy to ApplyContext.
+ */
+ MySubscriptionConninfo =
+ MemoryContextStrdup(ApplyContext,
+ SubscriptionConninfo(MySubscription, true));
+
+ MySubscriptionValid = true;
+
/*
* Restart the worker if retain_dead_tuples was enabled during startup.
*
--
2.43.0
From 1797d4b1a7eb1450c3611e3ba039e2bf977fb6e4 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Thu, 30 Jul 2026 13:40:02 -0700
Subject: [PATCH v3 06/11] Be precise about when ALTER SUBSCRIPTION needs
conninfo.
Decide early whether the original conninfo is needed so that errors
happen consistently.
Addresses finding 12 in report from linked discussion.
Co-authored-by: Shlok Kyal <[email protected]>
Reported-by: Noah Misch <[email protected]>
Reviewed-by: Hayato Kuroda (Fujitsu) <[email protected]>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com
Backpatch-through: 19
---
src/backend/commands/subscriptioncmds.c | 83 ++++++++++++++--------
src/test/regress/expected/subscription.out | 6 ++
src/test/regress/sql/subscription.sql | 7 ++
3 files changed, 68 insertions(+), 28 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 6ca215f8bf3..d05ef3fb4b2 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1632,7 +1632,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
Datum values[Natts_pg_subscription];
HeapTuple tup;
Oid subid;
- bool orig_conninfo_needed = true;
+ bool orig_conninfo_needed = false;
bool update_tuple = false;
bool update_failover = false;
bool update_two_phase = false;
@@ -1714,37 +1714,64 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
if (supported_opts > 0)
parse_subscription_options(pstate, stmt->options, supported_opts, &opts);
+ sub = GetSubscription(subid, false);
+
/*
- * Ensure that ALTER SUBSCRIPTION commands that could be used to fix a
- * broken connection or prepare to drop a broken subscription don't
- * attempt to construct the conninfo. Otherwise, we might encounter the
- * error the user is trying to fix.
- *
- * Specifically, ALTER SUBSCRIPTION DISABLE, ALTER SUBSCRIPTION SERVER,
- * ALTER SUBSCRIPTION CONNECTION, or ALTER SUBSCRIPTION SET
- * (slot_name=NONE).
- *
- * NB: if the user specifies multiple SET options, then we may still need
- * to construct conninfo even if slot_name is set to NONE.
+ * Determine in advance whether we need the original conninfo or not, so
+ * that errors are generated consistently in cases where we do need it;
+ * and not generated at all if we don't.
*/
- if (stmt->kind == ALTER_SUBSCRIPTION_ENABLED)
- {
- if (opts.specified_opts == SUBOPT_ENABLED && !opts.enabled)
- orig_conninfo_needed = false;
- }
- else if (stmt->kind == ALTER_SUBSCRIPTION_SERVER ||
- stmt->kind == ALTER_SUBSCRIPTION_CONNECTION)
- {
- orig_conninfo_needed = false;
- }
- else if (stmt->kind == ALTER_SUBSCRIPTION_OPTIONS)
+
+ /* conninfo needed when refreshing */
+ switch (stmt->kind)
{
- /* ... SET (slot_name = NONE) with no other options */
- if (opts.specified_opts == SUBOPT_SLOT_NAME && !opts.slot_name)
- orig_conninfo_needed = false;
- }
+ case ALTER_SUBSCRIPTION_REFRESH_PUBLICATION:
+ case ALTER_SUBSCRIPTION_REFRESH_SEQUENCES:
+ orig_conninfo_needed = true;
+ break;
- sub = GetSubscription(subid, false);
+ case ALTER_SUBSCRIPTION_SET_PUBLICATION:
+ case ALTER_SUBSCRIPTION_ADD_PUBLICATION:
+ case ALTER_SUBSCRIPTION_DROP_PUBLICATION:
+ /* opts.refresh defaults to true when the option is supported */
+ orig_conninfo_needed = opts.refresh;
+ break;
+
+ case ALTER_SUBSCRIPTION_ENABLED:
+ orig_conninfo_needed = opts.enabled && sub->retaindeadtuples;
+ break;
+
+ case ALTER_SUBSCRIPTION_OPTIONS:
+ {
+ if (sub->slotname)
+ {
+ if (IsSet(opts.specified_opts, SUBOPT_FAILOVER))
+ orig_conninfo_needed = true;
+ if (IsSet(opts.specified_opts, SUBOPT_TWOPHASE_COMMIT) &&
+ !opts.twophase)
+ orig_conninfo_needed = true;
+ }
+
+ if (IsSet(opts.specified_opts, SUBOPT_RETAIN_DEAD_TUPLES) &&
+ opts.retaindeadtuples)
+ orig_conninfo_needed = true;
+
+ if (IsSet(opts.specified_opts, SUBOPT_ORIGIN))
+ {
+ bool rdt;
+
+ rdt = IsSet(opts.specified_opts, SUBOPT_RETAIN_DEAD_TUPLES) ?
+ opts.retaindeadtuples : sub->retaindeadtuples;
+
+ if (rdt && pg_strcasecmp(opts.origin, LOGICALREP_ORIGIN_ANY) == 0)
+ orig_conninfo_needed = true;
+ }
+ }
+ break;
+
+ default:
+ break;
+ }
/*
* Skip ACL checks on the subscription's foreign server, if any. If
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 259db747334..d0955ca1159 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -229,6 +229,12 @@ 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.
DROP USER MAPPING FOR regress_subscription_user3 SERVER test_server;
+-- ok, catalog-only forms don't construct conninfo
+ALTER SUBSCRIPTION regress_testsub6 SET (synchronous_commit = local);
+ALTER SUBSCRIPTION regress_testsub6 SET (synchronous_commit = off);
+ALTER SUBSCRIPTION regress_testsub6 SET (disable_on_error = true);
+ALTER SUBSCRIPTION regress_testsub6 SET (disable_on_error = false);
+ALTER SUBSCRIPTION regress_testsub6 SET PUBLICATION testpub WITH (refresh = false);
-- ok, test_server lacks user mapping, but replacing connection anyway
BEGIN;
ALTER SUBSCRIPTION regress_testsub6 CONNECTION 'dbname=regress_doesnotexist password=secret';
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 7718c742974..98304737adc 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -176,6 +176,13 @@ CREATE SUBSCRIPTION regress_testsub6 SERVER test_server
DROP USER MAPPING FOR regress_subscription_user3 SERVER test_server;
+-- ok, catalog-only forms don't construct conninfo
+ALTER SUBSCRIPTION regress_testsub6 SET (synchronous_commit = local);
+ALTER SUBSCRIPTION regress_testsub6 SET (synchronous_commit = off);
+ALTER SUBSCRIPTION regress_testsub6 SET (disable_on_error = true);
+ALTER SUBSCRIPTION regress_testsub6 SET (disable_on_error = false);
+ALTER SUBSCRIPTION regress_testsub6 SET PUBLICATION testpub WITH (refresh = false);
+
-- ok, test_server lacks user mapping, but replacing connection anyway
BEGIN;
ALTER SUBSCRIPTION regress_testsub6 CONNECTION 'dbname=regress_doesnotexist password=secret';
--
2.43.0
From 331840bce6463b99416fe7614464e10498972232 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Thu, 30 Jul 2026 13:47:29 -0700
Subject: [PATCH v3 07/11] Always check foreign-server USAGE when resolving
subscription conninfo.
Previously, this was skipped in some cases to avoid raising errors
when conninfo wasn't even needed. That was wrong in cases where
conninfo was needed.
Now that we only build conninfo when needed, always perform the USAGE
check.
Addresses finding 7 in report from linked discussion.
Co-authored-by: Shlok Kyal <[email protected]>
Reported-by: Noah Misch <[email protected]>
Reviewed-by: Hayato Kuroda (Fujitsu) <[email protected]>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com
Backpatch-through: 19
---
src/backend/catalog/pg_subscription.c | 23 ++++++++++------------
src/backend/commands/subscriptioncmds.c | 9 +--------
src/backend/replication/logical/worker.c | 4 ++--
src/include/catalog/pg_subscription.h | 2 +-
src/test/regress/expected/subscription.out | 3 +++
src/test/regress/sql/subscription.sql | 3 +++
6 files changed, 20 insertions(+), 24 deletions(-)
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index 8fa2a460a25..76f09836bee 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -191,7 +191,7 @@ GetSubscription(Oid subid, bool missing_ok)
* missing user mapping).
*/
char *
-SubscriptionConninfo(Subscription *sub, bool aclcheck)
+SubscriptionConninfo(Subscription *sub)
{
HeapTuple tup;
Form_pg_subscription subform;
@@ -211,18 +211,15 @@ SubscriptionConninfo(Subscription *sub, bool aclcheck)
server = GetForeignServer(subform->subserver);
- if (aclcheck)
- {
- aclresult = object_aclcheck(ForeignServerRelationId,
- subform->subserver,
- sub->owner, ACL_USAGE);
- if (aclresult != ACLCHECK_OK)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("subscription owner \"%s\" does not have permission on foreign server \"%s\"",
- GetUserNameFromId(sub->owner, false),
- server->servername)));
- }
+ aclresult = object_aclcheck(ForeignServerRelationId,
+ subform->subserver,
+ sub->owner, ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("subscription owner \"%s\" does not have permission on foreign server \"%s\"",
+ GetUserNameFromId(sub->owner, false),
+ server->servername)));
conninfo = ForeignServerConnectionString(sub->owner, server);
}
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index d05ef3fb4b2..77e10d78e64 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1773,15 +1773,8 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
break;
}
- /*
- * Skip ACL checks on the subscription's foreign server, if any. If
- * changing the server (or replacing it with a raw connection), then the
- * old one will be removed anyway. If changing something unrelated,
- * there's no need to do an additional ACL check here; that will be done
- * by the subscription worker.
- */
if (orig_conninfo_needed)
- orig_conninfo = SubscriptionConninfo(sub, false);
+ orig_conninfo = SubscriptionConninfo(sub);
retain_dead_tuples = sub->retaindeadtuples;
origin = sub->origin;
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 1cdd28f5049..9c4c31a5cbc 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5114,7 +5114,7 @@ maybe_reread_subscription(void)
* is enabled. Allocated in transaction context; must be copied to
* ApplyContext when we set MySubscriptionConninfo.
*/
- new_conninfo = SubscriptionConninfo(newsub, true);
+ new_conninfo = SubscriptionConninfo(newsub);
/* !slotname should never happen when enabled is true. */
Assert(newsub->slotname);
@@ -5879,7 +5879,7 @@ InitializeLogRepWorker(void)
*/
MySubscriptionConninfo =
MemoryContextStrdup(ApplyContext,
- SubscriptionConninfo(MySubscription, true));
+ SubscriptionConninfo(MySubscription));
MySubscriptionValid = true;
diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index 5a9c07fe8d6..d2781a0b837 100644
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -222,7 +222,7 @@ typedef struct Subscription
#endif /* EXPOSE_TO_CLIENT_CODE */
extern Subscription *GetSubscription(Oid subid, bool missing_ok);
-extern char *SubscriptionConninfo(Subscription *sub, bool aclcheck);
+extern char *SubscriptionConninfo(Subscription *sub);
extern void DisableSubscription(Oid subid);
extern int CountDBSubscriptions(Oid dbid);
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index d0955ca1159..f67ffab1f54 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -215,6 +215,9 @@ SET SESSION AUTHORIZATION regress_subscription_user3;
BEGIN;
ALTER SUBSCRIPTION regress_testsub6 CONNECTION 'dbname=regress_doesnotexist password=secret';
ABORT;
+-- fail, connecting forms recheck USAGE on the foreign server
+ALTER SUBSCRIPTION regress_testsub6 REFRESH PUBLICATION;
+ERROR: subscription owner "regress_subscription_user3" does not have permission on foreign server "test_server"
-- fails, cannot drop slot
DROP SUBSCRIPTION regress_testsub6;
ERROR: could not connect to publisher when attempting to drop replication slot "dummy": subscription owner "regress_subscription_user3" does not have permission on foreign server "test_server"
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 98304737adc..47e2b6ef09c 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -161,6 +161,9 @@ BEGIN;
ALTER SUBSCRIPTION regress_testsub6 CONNECTION 'dbname=regress_doesnotexist password=secret';
ABORT;
+-- fail, connecting forms recheck USAGE on the foreign server
+ALTER SUBSCRIPTION regress_testsub6 REFRESH PUBLICATION;
+
-- fails, cannot drop slot
DROP SUBSCRIPTION regress_testsub6;
--
2.43.0
From 243475c3e7887c19db73720887dd741f0ceeb532 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Thu, 30 Jul 2026 18:18:25 -0700
Subject: [PATCH v3 08/11] For subscription DDL, demote user mapping checks to
WARNING.
The checks are useful to report to the user, but there's no reason to
raise an error. If needed while constructing conninfo, fdwconnection
will raise an error then.
Partially addresses finding 1, and addresses finding 13 in report from
the linked discussion.
Reported-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 19
---
src/backend/commands/subscriptioncmds.c | 8 ++++----
src/backend/foreign/foreign.c | 14 +++++++++++++-
src/include/foreign/foreign.h | 1 +
src/test/regress/expected/subscription.out | 8 +++-----
src/test/regress/sql/subscription.sql | 5 +----
5 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 77e10d78e64..b91c07d208e 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -806,8 +806,8 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, server->servername);
- /* make sure a user mapping exists */
- GetUserMapping(owner, server->serverid);
+ /* check user mapping */
+ GetUserMappingExtended(owner, server->serverid, WARNING);
serverid = server->serverid;
conninfo = ForeignServerConnectionString(owner, server);
@@ -2170,8 +2170,8 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
GetUserNameFromId(form->subowner, false),
new_server->servername));
- /* make sure a user mapping exists */
- GetUserMapping(form->subowner, new_server->serverid);
+ /* check user mapping */
+ GetUserMappingExtended(form->subowner, new_server->serverid, WARNING);
new_conninfo = ForeignServerConnectionString(form->subowner,
new_server);
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index 821d45c1e11..73343f017b3 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -230,6 +230,16 @@ ForeignServerConnectionString(Oid userid, ForeignServer *server)
*/
UserMapping *
GetUserMapping(Oid userid, Oid serverid)
+{
+ return GetUserMappingExtended(userid, serverid, ERROR);
+}
+
+/*
+ * Like GetUserMapping(), but allows caller to specify an elevel. If elevel is
+ * less than ERROR, returns NULL if the user mapping doesn't exist.
+ */
+UserMapping *
+GetUserMappingExtended(Oid userid, Oid serverid, int elevel)
{
Datum datum;
HeapTuple tp;
@@ -252,10 +262,12 @@ GetUserMapping(Oid userid, Oid serverid)
{
ForeignServer *server = GetForeignServer(serverid);
- ereport(ERROR,
+ ereport(elevel,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("user mapping not found for user \"%s\", server \"%s\"",
MappingUserName(userid), server->servername)));
+
+ return NULL;
}
um = palloc_object(UserMapping);
diff --git a/src/include/foreign/foreign.h b/src/include/foreign/foreign.h
index 92a55214fee..9b4532895a4 100644
--- a/src/include/foreign/foreign.h
+++ b/src/include/foreign/foreign.h
@@ -73,6 +73,7 @@ extern ForeignServer *GetForeignServerByName(const char *srvname,
extern char *ForeignServerConnectionString(Oid userid,
ForeignServer *server);
extern UserMapping *GetUserMapping(Oid userid, Oid serverid);
+extern UserMapping *GetUserMappingExtended(Oid userid, Oid serverid, int elevel);
extern ForeignDataWrapper *GetForeignDataWrapper(Oid fdwid);
extern ForeignDataWrapper *GetForeignDataWrapperExtended(Oid fdwid,
uint16 flags);
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index f67ffab1f54..e36f227129b 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -177,14 +177,12 @@ ERROR: permission denied for foreign server test_server
RESET SESSION AUTHORIZATION;
GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user3;
SET SESSION AUTHORIZATION regress_subscription_user3;
--- fail, need user mapping
-CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = NONE, connect = false);
-ERROR: user mapping not found for user "regress_subscription_user3", server "test_server"
-CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret');
--- fail, need CONNECTION clause
+-- warn, need user mapping, then fail, FDW doesn't support connections
CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = NONE, connect = false);
+WARNING: user mapping not found for user "regress_subscription_user3", server "test_server"
ERROR: foreign data wrapper "test_fdw" does not support subscription connections
DETAIL: Foreign data wrapper must be defined with CONNECTION specified.
+CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret');
RESET SESSION AUTHORIZATION;
ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection;
SET SESSION AUTHORIZATION regress_subscription_user3;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 47e2b6ef09c..5ee13df6653 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -124,14 +124,11 @@ RESET SESSION AUTHORIZATION;
GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user3;
SET SESSION AUTHORIZATION regress_subscription_user3;
--- fail, need user mapping
+-- warn, need user mapping, then fail, FDW doesn't support connections
CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = NONE, connect = false);
CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret');
--- fail, need CONNECTION clause
-CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = NONE, connect = false);
-
RESET SESSION AUTHORIZATION;
ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection;
SET SESSION AUTHORIZATION regress_subscription_user3;
--
2.43.0
From b55b408bf6a8351d468ff06bd92617aa3de6e277 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Thu, 30 Jul 2026 18:26:23 -0700
Subject: [PATCH v3 09/11] CREATE SUBSCRIPTION: do not construct conninfo
unnecessarily.
Still check that the creating user has USAGE privileges on the server,
and that the FDW supports subscription connections.
Addresses finding 1 in the report from the linked discussion.
Reported-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 19
---
src/backend/commands/subscriptioncmds.c | 37 ++++++++++++++++------
src/backend/foreign/foreign.c | 4 +--
src/test/regress/expected/subscription.out | 4 +--
3 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index b91c07d208e..d52050282a5 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -677,8 +677,8 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
Datum values[Natts_pg_subscription];
Oid owner = GetUserId();
HeapTuple tup;
- Oid serverid;
- char *conninfo;
+ Oid serverid = InvalidOid;
+ char *conninfo = NULL;
char originname[NAMEDATALEN];
List *publications;
uint32 supported_opts;
@@ -799,30 +799,47 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
ForeignServer *server;
Assert(!stmt->conninfo);
- conninfo = NULL;
server = GetForeignServerByName(stmt->servername, false);
- aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, owner, ACL_USAGE);
+ serverid = server->serverid;
+
+ /* check USAGE privileges on server */
+ aclresult = object_aclcheck(ForeignServerRelationId, serverid, owner, ACL_USAGE);
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, server->servername);
/* check user mapping */
GetUserMappingExtended(owner, server->serverid, WARNING);
- serverid = server->serverid;
- conninfo = ForeignServerConnectionString(owner, server);
+ /*
+ * Check conninfo if connecting; otherwise only check that the
+ * server's FDW supports connections.
+ */
+ if (opts.connect)
+ {
+ conninfo = ForeignServerConnectionString(owner, server);
+ walrcv_check_conninfo(conninfo, opts.passwordrequired && !superuser());
+ }
+ else
+ {
+ ForeignDataWrapper *fdw = GetForeignDataWrapper(server->fdwid);
+
+ if (!OidIsValid(fdw->fdwconnection))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("foreign-data wrapper \"%s\" does not support subscription connections",
+ fdw->fdwname),
+ errdetail("Foreign-data wrapper must be defined with CONNECTION specified.")));
+ }
}
else
{
Assert(stmt->conninfo);
- serverid = InvalidOid;
conninfo = stmt->conninfo;
+ walrcv_check_conninfo(conninfo, opts.passwordrequired && !superuser());
}
- /* Check the connection info string. */
- walrcv_check_conninfo(conninfo, opts.passwordrequired && !superuser());
-
publications = stmt->publication;
/* Everything ok, form a new tuple. */
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index 73343f017b3..7ad8e8ee56b 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -209,9 +209,9 @@ ForeignServerConnectionString(Oid userid, ForeignServer *server)
if (!OidIsValid(fdw->fdwconnection))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("foreign data wrapper \"%s\" does not support subscription connections",
+ errmsg("foreign-data wrapper \"%s\" does not support subscription connections",
fdw->fdwname),
- errdetail("Foreign data wrapper must be defined with CONNECTION specified.")));
+ errdetail("Foreign-data wrapper must be defined with CONNECTION specified.")));
connection_datum = OidFunctionCall3(fdw->fdwconnection,
ObjectIdGetDatum(userid),
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index e36f227129b..715c84afaa9 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -180,8 +180,8 @@ SET SESSION AUTHORIZATION regress_subscription_user3;
-- warn, need user mapping, then fail, FDW doesn't support connections
CREATE SUBSCRIPTION regress_testsub6 SERVER test_server PUBLICATION testpub WITH (slot_name = NONE, connect = false);
WARNING: user mapping not found for user "regress_subscription_user3", server "test_server"
-ERROR: foreign data wrapper "test_fdw" does not support subscription connections
-DETAIL: Foreign data wrapper must be defined with CONNECTION specified.
+ERROR: foreign-data wrapper "test_fdw" does not support subscription connections
+DETAIL: Foreign-data wrapper must be defined with CONNECTION specified.
CREATE USER MAPPING FOR regress_subscription_user3 SERVER test_server OPTIONS(user 'foo', password 'secret');
RESET SESSION AUTHORIZATION;
ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection;
--
2.43.0
From 04d22977acd8ada50a767b5717ee5d29cba09c3f Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Thu, 30 Jul 2026 20:28:47 -0700
Subject: [PATCH v3 10/11] Revert "Validate subscription conninfo on owner
change"
This reverts commit 1c9c35890421e96a91129b51f2c6446a6d95af95.
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 19
---
doc/src/sgml/ref/alter_subscription.sgml | 7 -------
src/backend/commands/subscriptioncmds.c | 14 ++------------
src/test/regress/expected/subscription.out | 21 ---------------------
src/test/regress/regress.c | 9 ---------
src/test/regress/sql/subscription.sql | 18 ------------------
5 files changed, 2 insertions(+), 67 deletions(-)
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 0f81af5608b..6fc3e07a2d5 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -53,13 +53,6 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
to alter the owner, you must be able to <literal>SET ROLE</literal> to the
new owning role. If the subscription has
<literal>password_required=false</literal>, only superusers can modify it.
- If the subscription uses a foreign server, the new owner must have
- <literal>USAGE</literal> privilege on the foreign server, a user mapping
- for the new owner or for <literal>PUBLIC</literal> must exist, and the
- connection string generated for the new owner must be valid. If the new
- owner is not a superuser and the subscription has
- <literal>password_required=true</literal>, the generated connection string
- must include a password.
</para>
<para>
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index d52050282a5..54f35d41c27 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -3007,12 +3007,11 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
/*
* If the subscription uses a server, check that the new owner has USAGE
- * privileges on the server, that a user mapping exists, and that the
- * resulting connection string is valid for the new owner.
+ * privileges on the server and that a user mapping exists. Note: does not
+ * re-check the resulting connection string.
*/
if (OidIsValid(form->subserver))
{
- char *conninfo;
ForeignServer *server = GetForeignServer(form->subserver);
aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, newOwnerId, ACL_USAGE);
@@ -3025,15 +3024,6 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
/* make sure a user mapping exists */
GetUserMapping(newOwnerId, server->serverid);
-
- conninfo = ForeignServerConnectionString(newOwnerId, server);
-
- /* Load the library providing us libpq calls. */
- load_file("libpqwalreceiver", false);
- /* Check the connection info string. */
- walrcv_check_conninfo(conninfo,
- form->subpasswordrequired &&
- !superuser_arg(newOwnerId));
}
form->subowner = newOwnerId;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 715c84afaa9..d447cba1397 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -9,10 +9,6 @@ CREATE FUNCTION test_fdw_connection(oid, oid, internal)
RETURNS text
AS :'regresslib', 'test_fdw_connection'
LANGUAGE C;
-CREATE FUNCTION test_fdw_connection_no_password(oid, oid, internal)
- RETURNS text
- AS :'regresslib', 'test_fdw_connection_no_password'
- LANGUAGE C;
CREATE ROLE regress_subscription_user LOGIN SUPERUSER;
CREATE ROLE regress_subscription_user2;
CREATE ROLE regress_subscription_user3 IN ROLE pg_create_subscription;
@@ -191,22 +187,6 @@ 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;
-GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user2;
-CREATE USER MAPPING FOR regress_subscription_user2 SERVER test_server OPTIONS(user 'foo');
-ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection_no_password;
-WARNING: changing the foreign-data wrapper connection function can cause the options for dependent objects to become invalid
--- fail, new owner's generated conninfo must satisfy password_required
-ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2;
-ERROR: password is required
-DETAIL: Non-superusers must provide a password in the connection string.
-ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection;
-WARNING: changing the foreign-data wrapper connection function can cause the options for dependent objects to become invalid
-DROP USER MAPPING FOR regress_subscription_user2 SERVER test_server;
-REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user2;
--- fail, subscription depends on the server and cannot be dropped by CASCADE
-DROP SERVER test_server CASCADE;
-ERROR: cannot drop server test_server because subscription regress_testsub6 depends on it
-HINT: Drop subscription regress_testsub6 first.
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
@@ -258,7 +238,6 @@ HINT: Use DROP ... CASCADE to drop the dependent objects too.
ALTER FOREIGN DATA WRAPPER test_fdw NO CONNECTION;
WARNING: removing the foreign-data wrapper connection function will cause dependent subscriptions to fail
DROP FUNCTION test_fdw_connection(oid, oid, internal);
-DROP FUNCTION test_fdw_connection_no_password(oid, oid, internal);
DROP FOREIGN DATA WRAPPER test_fdw;
-- fail - invalid connection string during ALTER
ALTER SUBSCRIPTION regress_testsub CONNECTION 'foobar';
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 14d301b3499..9801cdd1d8c 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -742,15 +742,6 @@ test_fdw_connection(PG_FUNCTION_ARGS)
PG_RETURN_TEXT_P(cstring_to_text("dbname=regress_doesnotexist user=doesnotexist password=secret"));
}
-PG_FUNCTION_INFO_V1(test_fdw_connection_no_password);
-Datum
-test_fdw_connection_no_password(PG_FUNCTION_ARGS)
-{
- /* Ensure the test fails if no valid user mapping exists. */
- GetUserMapping(PG_GETARG_OID(0), PG_GETARG_OID(1));
- PG_RETURN_TEXT_P(cstring_to_text("dbname=regress_doesnotexist user=doesnotexist"));
-}
-
PG_FUNCTION_INFO_V1(is_catalog_text_unique_index_oid);
Datum
is_catalog_text_unique_index_oid(PG_FUNCTION_ARGS)
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 5ee13df6653..dd33b18da55 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -12,10 +12,6 @@ CREATE FUNCTION test_fdw_connection(oid, oid, internal)
RETURNS text
AS :'regresslib', 'test_fdw_connection'
LANGUAGE C;
-CREATE FUNCTION test_fdw_connection_no_password(oid, oid, internal)
- RETURNS text
- AS :'regresslib', 'test_fdw_connection_no_password'
- LANGUAGE C;
CREATE ROLE regress_subscription_user LOGIN SUPERUSER;
CREATE ROLE regress_subscription_user2;
@@ -137,19 +133,6 @@ CREATE SUBSCRIPTION regress_testsub6 SERVER test_server
PUBLICATION testpub WITH (slot_name = 'dummy', connect = false);
RESET SESSION AUTHORIZATION;
-GRANT USAGE ON FOREIGN SERVER test_server TO regress_subscription_user2;
-CREATE USER MAPPING FOR regress_subscription_user2 SERVER test_server OPTIONS(user 'foo');
-ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection_no_password;
-
--- fail, new owner's generated conninfo must satisfy password_required
-ALTER SUBSCRIPTION regress_testsub6 OWNER TO regress_subscription_user2;
-
-ALTER FOREIGN DATA WRAPPER test_fdw CONNECTION test_fdw_connection;
-DROP USER MAPPING FOR regress_subscription_user2 SERVER test_server;
-REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user2;
--- fail, subscription depends on the server and cannot be dropped by CASCADE
-DROP SERVER test_server CASCADE;
-
REVOKE USAGE ON FOREIGN SERVER test_server FROM regress_subscription_user3;
SET SESSION AUTHORIZATION regress_subscription_user3;
@@ -206,7 +189,6 @@ DROP FUNCTION test_fdw_connection(oid, oid, internal);
ALTER FOREIGN DATA WRAPPER test_fdw NO CONNECTION;
DROP FUNCTION test_fdw_connection(oid, oid, internal);
-DROP FUNCTION test_fdw_connection_no_password(oid, oid, internal);
DROP FOREIGN DATA WRAPPER test_fdw;
--
2.43.0
From 9e9acf56b793334e80d32a38c183253bdbb237c5 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
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 <[email protected]>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch%40microsoft.com
Discussion: https://postgr.es/m/[email protected]
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
