From: Jeff Davis <pgsql@j-davis.com>
To: Noah Misch <noah@leadboat.com>
Cc: pgsql-hackers@postgresql.org, Robert Haas <robertmhaas@gmail.com>
Subject: Re: CREATE SUBSCRIPTION ... SERVER vs. pg_dump, etc.
Date: Tue, 28 Jul 2026 15:36:08 -0700
Message-ID: <2487ddcd737d4fc8e408e87aa9ad4365eed3bbb3.camel@j-davis.com> (raw)
In-Reply-To: <20260710195902.4f.noahmisch@microsoft.com>
References: <20260710195902.4f.noahmisch@microsoft.com>
On Fri, 2026-07-10 at 12:59 -0700, Noah Misch wrote:
> An Opus 4.8 review of commit 8185bb5 found two pg_dump+restore
> failure
> scenarios, visible in the attached test patch. (The patch also tests
> a
> REASSIGN OWNED finding, for which I started a distinct thread
> postgr.es/m/flat/20260710192533.4f.noahmisch@microsoft.com).
>
> Opus also emitted the attached report about these findings and
> others. I
> didn't examine the others closely. Finding-19, about invalidation
> callbacks,
> stood out as perhaps most exciting if true.
Partial patch series:
0001: Finding 10 preexisting issue: Add missing lock release for
DROP OWNED BY (backport to 16)
0002: Finding 10 & 15: Improve & document DROP SERVER CASCADE
0003: Finding 3: Reject use_scram_passthrough for
subscription connections.
Robert, can you take a look at 0001, which fixes an issue introduced in
6566133c5f? I don't think it's major but it can retain the lock for
longer.
Regards,
Jeff Davis
Attachments:
[text/x-patch] v2-0001-Fix-lock-release-for-role-membership-grants-in-DR.patch (1.1K, ../2487ddcd737d4fc8e408e87aa9ad4365eed3bbb3.camel@j-davis.com/2-v2-0001-Fix-lock-release-for-role-membership-grants-in-DR.patch)
download | inline diff:
From 428d8c7c41806deeb1357a6e95e8f9a2687dbd56 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Tue, 28 Jul 2026 14:50:45 -0700
Subject: [PATCH v2 1/3] Fix lock release for role membership grants in DROP
OWNED BY.
Add ReleaseDeletionLock() to match AcquireDeletionLock(). Introduced
by commit 6566133c5f5.
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch@microsoft.com
Backpatch-through: 16
---
src/backend/catalog/dependency.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.cindex 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,
--
2.43.0
[text/x-patch] v2-0002-Improve-DROP-SERVER-handling-of-dependent-subscri.patch (5.5K, ../2487ddcd737d4fc8e408e87aa9ad4365eed3bbb3.camel@j-davis.com/3-v2-0002-Improve-DROP-SERVER-handling-of-dependent-subscri.patch)
download | inline diff:
From f40a6c256c709014b1fbe7670f68061800d504ce Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Tue, 28 Jul 2026 14:48:12 -0700
Subject: [PATCH v2 2/3] 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.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch@microsoft.com
Backpatch-through: 19
---
doc/src/sgml/ref/drop_server.sgml | 4 ++++
src/backend/catalog/dependency.c | 28 ++++++++++++----------
src/test/regress/expected/subscription.out | 4 ++++
src/test/regress/sql/subscription.sql | 2 ++
4 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/ref/drop_server.sgml b/doc/src/sgml/ref/drop_server.sgmlindex 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.cindex 52cd2caf9d4..b80949a5eeb 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,19 @@ 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))+ ereport(ERROR,+ (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),+ errmsg("cannot drop %s because %s depends on it",+ getObjectDescription(object, false),+ getObjectDescription(&otherObject, false)),+ errhint("Drop %s first.",+ getObjectDescription(&otherObject, false))));+
/*
* We do need to delete it, so identify objflags to be passed down,
* which depend on the dependency type.
@@ -1579,7 +1581,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 +1602,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.outindex 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.sqlindex 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
[text/x-patch] v2-0003-postgres_fdw-reject-use_scram_passthrough-for-sub.patch (3.5K, ../2487ddcd737d4fc8e408e87aa9ad4365eed3bbb3.camel@j-davis.com/4-v2-0003-postgres_fdw-reject-use_scram_passthrough-for-sub.patch)
download | inline diff:
From 96bc4e83b40feaf747871081a48350991bd5b5f3 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Tue, 28 Jul 2026 13:50:29 -0700
Subject: [PATCH v2 3/3] postgres_fdw: reject use_scram_passthrough for
subscriptions.
The subscription is initiated from a loical replication worker, so
SCRAM pass-through won't work.
Resolves finding 3 in report.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260710195902.4f.noahmisch@microsoft.com
Backpatch-through: 19
---
contrib/postgres_fdw/connection.c | 12 ++++++++++++
contrib/postgres_fdw/t/010_subscription.pl | 14 +++++++++++++-
doc/src/sgml/postgres-fdw.sgml | 7 +++++++
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.cindex 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.plindex c34b3d15b8d..53f1ea73ece 100644--- a/contrib/postgres_fdw/t/010_subscription.pl+++ b/contrib/postgres_fdw/t/010_subscription.pl@@ -41,7 +41,19 @@ $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 succeeds 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.sgmlindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pgsql@j-davis.com, noah@leadboat.com, robertmhaas@gmail.com
Subject: Re: CREATE SUBSCRIPTION ... SERVER vs. pg_dump, etc.
In-Reply-To: <2487ddcd737d4fc8e408e87aa9ad4365eed3bbb3.camel@j-davis.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox