commit ada4f72 (subscription-server-defect-tests)
Author:     Noah Misch <nm@cqla.c.codeql-202606a.internal>
AuthorDate: Wed Jul 8 03:26:04 2026 +0000
Commit:     Noah Misch <nm@cqla.c.codeql-202606a.internal>
CommitDate: Wed Jul 8 03:26:04 2026 +0000

    Add TAP tests demonstrating CREATE SUBSCRIPTION ... SERVER defects
    
    These tests reproduce three user-visible defects introduced by commit
    8185bb5 (CREATE SUBSCRIPTION ... SERVER) and still present in the tree.
    Each check asserts the current, buggy behavior and documents the
    expected correct behavior, so a future fix flips the relevant assertion.
    
    - pg_dump/restore of a server-based subscription fails because
      CreateSubscription() resolves the user mapping for the restoring role
      rather than the subscription owner, even with connect = false.
    
    - Restoring such a subscription leaves it owned by the restoring
      superuser, because ALTER SUBSCRIPTION ... OWNER TO is restored in the
      main pass while the owner's GRANT USAGE on the foreign server is
      deferred to the ACL pass.
    
    - REASSIGN OWNED BY, run from a database other than the one holding the
      subscription's foreign server, fails with "cache lookup failed for
      foreign server", breaking multi-database role removal.
    
    The scenarios use connect = false, so no publisher is required.
    
    Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_01TePr48d48d89GukfGsNw9b
---
 contrib/postgres_fdw/meson.build                   |   1 +
 contrib/postgres_fdw/t/011_subscription_defects.pl | 233 +++++++++++++++++++++
 2 files changed, 234 insertions(+)

diff --git a/contrib/postgres_fdw/meson.build b/contrib/postgres_fdw/meson.build
index 3e2ed06..6cb204b 100644
--- a/contrib/postgres_fdw/meson.build
+++ b/contrib/postgres_fdw/meson.build
@@ -52,6 +52,7 @@ tests += {
     'tests': [
       't/001_auth_scram.pl',
       't/010_subscription.pl',
+      't/011_subscription_defects.pl',
     ],
   },
 }
diff --git a/contrib/postgres_fdw/t/011_subscription_defects.pl b/contrib/postgres_fdw/t/011_subscription_defects.pl
new file mode 100644
index 0000000..34ea757
--- /dev/null
+++ b/contrib/postgres_fdw/t/011_subscription_defects.pl
@@ -0,0 +1,233 @@
+# Copyright (c) 2021-2026, PostgreSQL Global Development Group
+
+# Demonstrations of user-visible defects in CREATE SUBSCRIPTION ... SERVER
+# (commit 8185bb5) that are still present in the tree.
+#
+# IMPORTANT: each block below asserts the *current, buggy* behavior so the
+# test passes and serves as an executable reproduction.  Every block also
+# states, in comments, what the correct behavior should be.  When a defect
+# is fixed, the corresponding assertion will start to fail and must be
+# flipped to the "correct behavior" noted alongside it.
+#
+# All scenarios use connect => false, so no publisher is required: the
+# defects are in the local CREATE/ALTER/REASSIGN and dump/restore paths,
+# not in replication itself.
+#
+# pg_subscription is a shared catalog, so a distinct subscription-owner role
+# is used per finding (and every cross-database catalog query is filtered by
+# subdbid) to keep the scenarios isolated from one another.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+$node->start;
+
+my $tempdir = PostgreSQL::Test::Utils::tempdir;
+
+# The cluster superuser is the OS user under TAP, not necessarily "postgres";
+# capture it so the assertions below are independent of that name.
+my $super = $node->safe_psql('postgres', 'SELECT current_user');
+
+# One subscription-owner role per finding, plus a REASSIGN OWNED target.
+$node->safe_psql(
+	'postgres', q{
+	CREATE ROLE owner1 LOGIN;
+	CREATE ROLE owner2 LOGIN;
+	CREATE ROLE owner4 LOGIN;
+	CREATE ROLE bob LOGIN;
+	GRANT pg_create_subscription TO owner1, owner2, owner4;
+});
+
+# Common foreign-server options.  postgres_fdw's connection function builds a
+# libpq conninfo from these; with connect => false it is validated but never
+# used to connect, so the host/port need not be reachable.
+my $srv_opts = "OPTIONS (host 'localhost', port '1', dbname 'nx')";
+
+#############################################################################
+# Finding 1: pg_dump/pg_restore (and hence pg_upgrade) of a SERVER-based
+# subscription fails, because CreateSubscription() resolves the user mapping
+# for the role *executing the restore* (GetUserId()), not the subscription's
+# eventual owner -- even with connect => false.
+#
+# The dump is emitted "CREATE SUBSCRIPTION ... SERVER ... WITH (connect =
+# false ...)" followed later by "ALTER SUBSCRIPTION ... OWNER TO owner1", so
+# the CREATE runs as the restoring superuser.  If that superuser has no user
+# mapping (and there is no PUBLIC mapping) on the server, the restore of a
+# perfectly valid dump fails.
+#
+# CORRECT BEHAVIOR: the dump should restore cleanly (a connect=false restore
+# should not require the restoring role to have a user mapping on the
+# server).
+#############################################################################
+{
+	$node->safe_psql('postgres', 'CREATE DATABASE defect1_src');
+	$node->safe_psql(
+		'defect1_src', qq{
+		CREATE EXTENSION postgres_fdw;
+		CREATE SERVER s FOREIGN DATA WRAPPER postgres_fdw $srv_opts;
+		-- Only owner1 has a mapping; the restoring superuser will not.
+		CREATE USER MAPPING FOR owner1 SERVER s OPTIONS (user 'repl', password 'secret');
+		GRANT USAGE ON FOREIGN SERVER s TO owner1;
+		GRANT CREATE ON DATABASE defect1_src TO owner1;
+	});
+	$node->safe_psql(
+		'defect1_src', q{
+		SET SESSION AUTHORIZATION owner1;
+		CREATE SUBSCRIPTION defect1_sub SERVER s PUBLICATION p
+			WITH (connect = false, slot_name = NONE);
+	});
+
+	my $dump = "$tempdir/defect1.sql";
+	command_ok(
+		[ 'pg_dump', '-f', $dump, $node->connstr('defect1_src') ],
+		'finding 1: pg_dump of a SERVER-based subscription succeeds');
+
+	$node->safe_psql('postgres', 'CREATE DATABASE defect1_dst');
+
+	# BUG: restoring as the superuser (which has no mapping) fails at the
+	# CREATE SUBSCRIPTION step.  When fixed, this should become command_ok().
+	command_fails_like(
+		[
+			'psql', '--no-psqlrc', '-v', 'ON_ERROR_STOP=1',
+			'-f', $dump, '-d', $node->connstr('defect1_dst')
+		],
+		qr/user mapping not found for user "\Q$super\E", server "s"/,
+		'finding 1: restore fails - CREATE resolves the restorer\'s mapping, not the owner\'s'
+	);
+
+	# The subscription was therefore not restored into defect1_dst at all.
+	my $got = $node->safe_psql(
+		'defect1_dst', q{
+		SELECT count(*) FROM pg_subscription
+		WHERE subname = 'defect1_sub'
+		  AND subdbid = (SELECT oid FROM pg_database WHERE datname = 'defect1_dst')
+	});
+	is($got, '0', 'finding 1: subscription is missing after the failed restore');
+}
+
+#############################################################################
+# Finding 2: restoring a SERVER-based subscription whose owner derives its
+# foreign-server USAGE from a GRANT (rather than ownership) leaves the
+# subscription owned by the wrong role.  AlterSubscriptionOwner_internal()
+# requires the new owner to hold USAGE on the server, but pg_dump emits
+# "ALTER SUBSCRIPTION ... OWNER TO owner2" in the main restore pass while
+# "GRANT USAGE ON FOREIGN SERVER" is deferred to the later ACL pass -- so at
+# the moment of the OWNER TO, owner2 does not yet have USAGE.
+#
+# CORRECT BEHAVIOR: the OWNER TO should succeed during restore and the
+# subscription should end up owned by owner2.
+#############################################################################
+{
+	$node->safe_psql('postgres', 'CREATE DATABASE defect2_src');
+	$node->safe_psql(
+		'defect2_src', qq{
+		CREATE EXTENSION postgres_fdw;
+		CREATE SERVER s FOREIGN DATA WRAPPER postgres_fdw $srv_opts;
+		-- PUBLIC mapping, so finding 1 does not mask this one: the
+		-- restoring superuser can resolve a connection.
+		CREATE USER MAPPING FOR PUBLIC SERVER s OPTIONS (user 'repl', password 'secret');
+		-- owner2's USAGE comes from a GRANT, not from owning the server.
+		GRANT USAGE ON FOREIGN SERVER s TO owner2;
+		GRANT CREATE ON DATABASE defect2_src TO owner2;
+	});
+	$node->safe_psql(
+		'defect2_src', q{
+		SET SESSION AUTHORIZATION owner2;
+		CREATE SUBSCRIPTION defect2_sub SERVER s PUBLICATION p
+			WITH (connect = false, slot_name = NONE);
+	});
+
+	my $dump = "$tempdir/defect2.sql";
+	command_ok(
+		[ 'pg_dump', '-f', $dump, $node->connstr('defect2_src') ],
+		'finding 2: pg_dump succeeds');
+
+	$node->safe_psql('postgres', 'CREATE DATABASE defect2_dst');
+
+	# BUG: the CREATE SUBSCRIPTION succeeds (PUBLIC mapping), but the
+	# subsequent ALTER SUBSCRIPTION ... OWNER TO owner2 fails because the
+	# GRANT USAGE has not been restored yet.  When fixed: command_ok().
+	command_fails_like(
+		[
+			'psql', '--no-psqlrc', '-v', 'ON_ERROR_STOP=1',
+			'-f', $dump, '-d', $node->connstr('defect2_dst')
+		],
+		qr/new subscription owner "owner2" does not have permission on foreign server "s"/,
+		'finding 2: restore fails at OWNER TO because GRANT USAGE is in a later pass'
+	);
+
+	# BUG: the subscription is left owned by the restoring superuser instead
+	# of owner2.  When fixed, the expected value is 'owner2'.
+	my $owner = $node->safe_psql(
+		'defect2_dst', q{
+		SELECT subowner::regrole FROM pg_subscription
+		WHERE subname = 'defect2_sub'
+		  AND subdbid = (SELECT oid FROM pg_database WHERE datname = 'defect2_dst')
+	});
+	is($owner, $super,
+		'finding 2: subscription is left owned by the restoring superuser, not owner2'
+	);
+}
+
+#############################################################################
+# Finding 4: REASSIGN OWNED BY, run from any database other than the one
+# holding the subscription's foreign server, fails with an internal error.
+# pg_subscription is a shared catalog, so shdepReassignOwned() processes the
+# subscription from every database; AlterSubscriptionOwner_internal() then
+# looks the server up in the *current* database's (per-database)
+# pg_foreign_server and raises "cache lookup failed for foreign server".
+# This breaks the documented multi-database role-removal workflow (REASSIGN
+# OWNED / DROP OWNED in each database, then DROP ROLE).
+#
+# CORRECT BEHAVIOR: REASSIGN OWNED from another database should reassign the
+# subscription without an internal error (either succeeding, or failing with
+# a clean, user-facing permission error like the in-database case below).
+#############################################################################
+{
+	$node->safe_psql('postgres', 'CREATE DATABASE defect4_src');
+	$node->safe_psql(
+		'defect4_src', qq{
+		CREATE EXTENSION postgres_fdw;
+		CREATE SERVER s FOREIGN DATA WRAPPER postgres_fdw $srv_opts;
+		CREATE USER MAPPING FOR owner4 SERVER s OPTIONS (user 'repl', password 'secret');
+		GRANT USAGE ON FOREIGN SERVER s TO owner4;
+		GRANT CREATE ON DATABASE defect4_src TO owner4;
+	});
+	$node->safe_psql(
+		'defect4_src', q{
+		SET SESSION AUTHORIZATION owner4;
+		CREATE SUBSCRIPTION defect4_sub SERVER s PUBLICATION p
+			WITH (connect = false, slot_name = NONE);
+	});
+
+	# BUG: from the "postgres" database (which has no such foreign server),
+	# REASSIGN OWNED aborts with an internal (XX000) cache-lookup error.
+	my ($ret, $out, $err) =
+	  $node->psql('postgres', 'REASSIGN OWNED BY owner4 TO bob');
+	isnt($ret, 0, 'finding 4: cross-database REASSIGN OWNED fails');
+	like(
+		$err,
+		qr/cache lookup failed for foreign server \d+/,
+		'finding 4: cross-database REASSIGN OWNED raises an internal cache-lookup error'
+	);
+
+	# Contrast: run from the subscription's own database, the same command
+	# fails with a clean, user-facing permission error (bob lacks USAGE on
+	# the server) -- showing the cross-database internal error is the defect,
+	# not the reassignment being disallowed.
+	($ret, $out, $err) =
+	  $node->psql('defect4_src', 'REASSIGN OWNED BY owner4 TO bob');
+	isnt($ret, 0, 'finding 4 (contrast): in-database REASSIGN OWNED also fails here');
+	like(
+		$err,
+		qr/new subscription owner "bob" does not have permission on foreign server "s"/,
+		'finding 4 (contrast): in-database REASSIGN OWNED gives a clean permission error'
+	);
+}
+
+done_testing();
