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/[email protected]).
>
> 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
From 428d8c7c41806deeb1357a6e95e8f9a2687dbd56 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
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/[email protected]
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.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,
--
2.43.0
From f40a6c256c709014b1fbe7670f68061800d504ce Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
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 <[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 | 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.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..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.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 96bc4e83b40feaf747871081a48350991bd5b5f3 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
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 <[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 | 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.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..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.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
