[Jeff Davis <pgsql@j-davis.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:

v2-0001-Fix-lock-release-for-role-membership-grants-in-DR.patch (text/x-patch, 1.1K) download
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.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



v2-0002-Improve-DROP-SERVER-handling-of-dependent-subscri.patch (text/x-patch, 5.5K) download
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.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



v2-0003-postgres_fdw-reject-use_scram_passthrough-for-sub.patch (text/x-patch, 3.5K) download
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.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