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.
commit ada4f72 (subscription-server-defect-tests) Author: Noah Misch <[email protected]> AuthorDate: Wed Jul 8 03:26:04 2026 +0000 Commit: Noah Misch <[email protected]> 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 <[email protected]> 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();
# User-visible defects in commit 8185bb5 still present in master **Commit:** 8185bb5 "CREATE SUBSCRIPTION ... SERVER." (Jeff Davis, 2026-03-06) **Master at time of audit:** 3c9a38b **Method:** 30-agent workflow — 16 area-focused finders (subscription DDL, foreign.c resolution, FDW DDL, dependencies, workers, postgres_fdw, extension packaging, pg_dump, psql, parser/nodes, security, NULL-sweep, docs, concurrency, end-to-end semantics) produced 40 raw findings; deduplicated to 19; the top 13 each adversarially verified by an independent skeptic instructed to refute, checking three gates: still in master, attributable to 8185bb5 (not pre-existing), and user-visible. All 13 were **confirmed** with code citations; the remaining 6 lower-priority findings are listed unverified. Line numbers refer to current master. --- ## Live-cluster verification (findings 1, 2, 4, 6) Reproduced on a running PostgreSQL 20devel cluster built from this tree (meson, `build/tmp_install`), using the regression suite's `test_fdw_connection` (returns a static conninfo, so no publisher is needed) and `connect=false` subscriptions. | # | Static verdict | Live result | Observed | |---|---|---|---| | 1 | confirmed | **CONFIRMED** | Restore as postgres fails at `CREATE SUBSCRIPTION`: `ERROR 42704: user mapping not found for user "postgres", server "test_server"` at `GetUserMapping, foreign.c:255`. Control: identical CREATE run as the mapped owner (alice) succeeds. | | 2 | confirmed | **CONFIRMED** | Restore's `CREATE` succeeds (PUBLIC mapping), then `ALTER SUBSCRIPTION ... OWNER TO alice` fails: `ERROR 42501: new subscription owner "alice" does not have permission on foreign server "test_server"` at `AlterSubscriptionOwner_internal, subscriptioncmds.c:2712`; subscription left owned by postgres. Control: `GRANT USAGE` first → OWNER TO succeeds. | | 4 | confirmed | **CONFIRMED** | `REASSIGN OWNED BY alice TO bob` from the `postgres` database: `ERROR XX000: cache lookup failed for foreign server 16393`. Contrast: same command from the subscription's own database gives a clean permission error. | | 6 | confirmed | **REFUTED** | Concurrent `DROP SERVER` does **not** race in — it blocks on `AccessExclusiveLock on object ... of class 1417` (server) and times out; no dangling reference results. See the rewritten entry below. | **Bottom line:** three of the four requested findings reproduce exactly as described; **Finding 6 does not reproduce and is withdrawn.** --- ## Confirmed — major ### 1. pg_dump/pg_restore/pg_upgrade of SERVER subscriptions fail: CREATE resolves the connection for the creating user, not the eventual owner, even with connect=false — LIVE-VERIFIED ✓ **Where:** `src/backend/commands/subscriptioncmds.c:784` **Symptom:** Restoring a dump of a database containing a server-based subscription fails with `ERROR: user mapping not found for user "postgres", server "..."` whenever the restoring role has no user mapping (and no PUBLIC mapping) on that server — even though the dump emits `connect = false`. pg_restore errors out; pg_upgrade aborts mid-restore on a perfectly valid source cluster. **Repro:** As superuser: `CREATE SERVER s ...; CREATE USER MAPPING FOR alice SERVER s ...; GRANT USAGE ON FOREIGN SERVER s TO alice;` As alice: `CREATE SUBSCRIPTION sub SERVER s PUBLICATION pub WITH (connect=false, slot_name=NONE);` Then `pg_dump db | psql newdb` as postgres, or pg_upgrade the cluster: the emitted CREATE SUBSCRIPTION fails. **Mechanism:** `CreateSubscription()` sets `owner = GetUserId()` (line 654) and in the servername branch (771–798) unconditionally — regardless of `connect=false` — calls `GetUserMapping(owner, serverid)` (line 784), which errors with no superuser bypass (foreign.c:251–258), then `ForeignServerConnectionString()` and `walrcv_check_conninfo()`. `dumpSubscription()` emits `CREATE SUBSCRIPTION ... SERVER ... WITH (connect = false, ...)` executed as the restore role; ownership is applied only afterwards via `ALTER SUBSCRIPTION ... OWNER TO`. pg_upgrade runs `pg_restore --exit-on-error` and aborts; binary-upgrade mode additionally emits `ALTER SUBSCRIPTION ... ENABLE`, hitting the same path via `GetSubscription(conninfo_needed=true)`. The docs' recommended setup (mapping only FOR the subscribing user) triggers this; pg_dump.sgml:1735–1748 promises subscription dumps restore without remote access. Follow-ups e5c4058/702e9df fixed only ALTER/DROP paths. **Verifier notes:** Confirmed on all gates. Restore succeeds only if the restoring role happens to have a role-specific or PUBLIC mapping, or with `--use-set-session-authorization`; pg_upgrade uses neither escape. pg_dump TAP tests cover only CONNECTION-based subscriptions. ### 2. Restore ordering: ALTER SUBSCRIPTION ... OWNER TO fails because GRANT USAGE ON FOREIGN SERVER restores later, in the ACL pass — LIVE-VERIFIED ✓ **Where:** `src/backend/commands/subscriptioncmds.c:2710` **Symptom:** Restore errors at the emitted `ALTER SUBSCRIPTION ... OWNER TO`: `new subscription owner "alice" does not have permission on foreign server ...`, because the GRANT has not been restored yet. Under psql the subscription silently stays owned by the bootstrap role (wrong catalog state); `pg_restore --exit-on-error` and pg_upgrade abort. **Repro:** Grant-based (non-owner) subscription owner; dump; restore into a fresh database as superuser: CREATE SUBSCRIPTION succeeds, the following `ALTER SUBSCRIPTION sub OWNER TO alice` errors. **Mechanism:** `AlterSubscriptionOwner_internal()` (2706–2720, added by 8185bb5) requires `object_aclcheck(ForeignServerRelationId, ..., ACL_USAGE)` plus `GetUserMapping(newOwnerId, ...)` before changing owner. pg_restore emits `ALTER ... OWNER TO` inside the subscription's own TOC entry in RESTORE_PASS_MAIN (`_printTocEntry`; SUBSCRIPTION is in `_getObjectDescription`, pg_backup_archiver.c:3862), while `GRANT USAGE ON FOREIGN SERVER` is an ACL TOC entry deferred by `_tocEntryRestorePass` (3366–3372) to RESTORE_PASS_ACL, strictly after all main-pass items even in serial restore. Plain-script dumps have the same order. Only new owners who own the server or are superuser restore cleanly. CONNECTION-based subscriptions have no such check — a new regression. **Verifier notes:** Confirmed. Current master reads `GetForeignServer(form->subserver)` after the 11f8018 refactor; behavior identical. ### 3. postgres_fdw connection function embeds session-only credentials (SCRAM passthrough / GSS delegation), so CREATE succeeds but apply workers can never authenticate **Where:** `contrib/postgres_fdw/connection.c:580` **Symptom:** `CREATE SUBSCRIPTION ... SERVER` over a `use_scram_passthrough` server succeeds (connects, creates the remote slot) when the creator logged in via SCRAM — but apply/tablesync workers can never connect to a SCRAM-authenticated publisher. The subscription never replicates; the log fills with connection errors as the launcher restarts the worker. Non-superuser owners instead get a misleading `password is required` at CREATE. Undocumented. **Mechanism:** `construct_connection_params()` (580–614) appends `scram_client_key`/`scram_server_key`/`require_auth` only when `MyProcPort != NULL && MyProcPort->has_scram_keys`; `MyProcPort` is assigned solely in backend_startup.c:177, so it is NULL in logical-replication background workers. The DDL session has keys, so CreateSubscription's `walrcv_check_conninfo`/`walrcv_connect` succeed; workers rebuild the conninfo via `GetSubscription → ForeignServerConnectionString → postgres_fdw_connection` (worker.c:5826) with no keys and no password. Superuser-owned subscriptions loop on publisher auth failure; non-superuser owners abort in `check_conn_params()`/`libpqrcv_check_conninfo`. GSSAPI delegated credentials have the same session-vs-worker asymmetry (`be_gssapi_get_delegation(MyProcPort)`, connection.c:769). postgres-fdw.sgml's Subscription Management section claims option parity with no caveat. No DDL-time rejection or warning exists. **Verifier notes:** Confirmed. The MyProcPort-conditional SCRAM code pre-existed but was only reachable from client backends; 8185bb5 exposed it to bgworkers via `postgres_fdw_connection`, so the omission is attributable. ### 4. REASSIGN OWNED BY fails with XX000 "cache lookup failed for foreign server" when the role owns a server-based subscription in another database — LIVE-VERIFIED ✓ **Where:** `src/backend/commands/subscriptioncmds.c:2708` **Symptom:** `REASSIGN OWNED BY old TO new`, run in any database other than the subscription's, aborts with the internal error `cache lookup failed for foreign server NNN` (XX000). The documented multi-database role-removal workflow (REASSIGN OWNED in each database, then DROP ROLE) breaks. **Mechanism:** pg_subscription is a shared catalog, so its pg_shdepend row has `dbid = InvalidOid`, and `shdepReassignOwned` processes it from every database, calling `AlterSubscriptionOwner_internal`. 8185bb5 added a lookup of the subscription's foreign server there — but pg_foreign_server is per-database, so from any other database `GetForeignServerExtended` raises `elog(ERROR, "cache lookup failed for foreign server %u")`. Pre-commit code had no server lookup and succeeded. In the freak case of an OID collision, the ACL/user-mapping checks would be evaluated against an unrelated server of the current database. **Verifier notes:** Confirmed. The literal `GetForeignServer()` call shape is from refactor 11f8018, but 8185bb5's original `object_aclcheck` + `GetUserMapping` fails identically cross-database (same XX000 for non-superusers via `object_aclmask_ext`; superusers fail in `GetUserMapping`). Broken since 8185bb5 either way. ### 5. Worker-side conninfo resolution errors preempt clean disabled-subscription handling and can permanently disable a live SERVER subscription via disable_on_error **Where:** `src/backend/replication/logical/worker.c:5077` **Symptom:** Disabling a SERVER subscription and dropping its user mapping in one transaction makes the running worker exit with `ERROR: user mapping not found ...` plus a bumped `apply_error_count` and (with `disable_on_error`) a misleading "disabled because of an error" LOG, instead of the clean "will stop because the subscription was disabled". Rotating a live mapping via DROP+CREATE (separate commits) can permanently disable the subscription if the worker rereads in the gap. **Mechanism:** `maybe_reread_subscription()` (worker.c:5077) calls `GetSubscription(subid, true, true, true)`, which runs `object_aclcheck` and `ForeignServerConnectionString → postgres_fdw_connection → GetUserMapping` — erroring before the `newsub->enabled` clean-exit check at worker.c:5101. The error unwinds to `start_apply`'s PG_CATCH (5644–5673): with `disableonerr` → `DisableSubscriptionAndExit` (apply_error_count++, misleading LOG); else pgstat error + rethrow. Pre-8185bb5, `GetSubscription` could not fail for an existing subscription. Follow-ups e5c4058/702e9df fixed this class only in ALTER/DROP DDL paths. **Verifier notes:** Confirmed; all cited lines match master. (One label fix: the third `maybe_reread_subscription` caller is worker.c:740, stream handling.) ### 6. ~~CREATE/ALTER SUBSCRIPTION ... SERVER takes no lock on the foreign server, so a concurrent DROP SERVER leaves a dangling subserver OID~~ — REFUTED on a live cluster **Original claim:** `CreateSubscription`/`AlterSubscription` look up the server via `GetForeignServerByName()` (a syscache read) and record the dependency without locking the server, so a concurrent `DROP SERVER` — whose `findDependentObjects` MVCC scan can't see the uncommitted pg_depend row — could succeed and leave `pg_subscription.subserver` dangling (worker restart loop, DROP SUBSCRIPTION failure, pg_dump NULL-deref crash). **Why it's wrong:** `recordDependencyOn` → `recordMultipleDependencies` calls `dependencyLockAndCheckObject()` (src/backend/catalog/pg_depend.c:121) **before** inserting the pg_depend row. That function takes an `AccessShareLock` on the referenced server via `LockDatabaseObject()` and then rechecks the object still exists, with the express purpose (per its header comment) to "make sure that we don't record a bogus reference permanently in the catalogs." The lock is held to end of transaction and conflicts with the `AccessExclusiveLock` that `DROP SERVER` acquires. This is a generic backstop in the dependency layer that both the finder and its adversarial verifier overlooked — they reasoned only about MVCC row visibility. **Live test (PostgreSQL 20devel from this tree):** - With `CREATE SUBSCRIPTION s6 SERVER test_server ... (connect=false)` held in an open transaction, a concurrent `DROP SERVER test_server` **blocked** and hit `lock_timeout`: `ERROR: canceling statement due to lock timeout / CONTEXT: waiting for AccessExclusiveLock on object 16427 of class 1417`. After commit, `s6.subserver` still resolved to the live server — no dangling reference. - `pg_locks` during the open transaction shows the CREATE holding `AccessShareLock` on `pg_foreign_server` objid 16427; the `ALTER SUBSCRIPTION ... SERVER` path holds the identical lock. - Once the subscription is committed, `DROP SERVER` (and `DROP SERVER CASCADE`) fail cleanly with `cannot drop server test_server because subscription s6 depends on it`; `pg_dump` of the database succeeds. - The pre-dependency window (server lookup → `recordDependencyOn`) is also safe: if a `DROP SERVER` commits there, `dependencyLockAndCheckObject`'s existence recheck aborts the CREATE with an error rather than persisting a dangling OID. No supported sequence produces a dangling `subserver`, so the downstream symptoms (worker restart loop, DROP failure, pg_dump crash) are unreachable. Finding withdrawn. ### 7. ALTER SUBSCRIPTION commands that connect to the publisher skip the foreign-server USAGE privilege check **Where:** `src/backend/commands/subscriptioncmds.c:1571` **Symptom:** A subscription owner whose USAGE on the foreign server was revoked can still make the server connect to the remote host with the user mapping's credentials via ALTER SUBSCRIPTION (REFRESH PUBLICATION/SEQUENCES, SET/ADD/DROP PUBLICATION with refresh, SET (failover/two_phase), retain_dead_tuples checks) — including dropping remote tablesync slots and altering remote slots — while the apply worker, CREATE, DROP, and OWNER TO all enforce USAGE. REVOKE appears ineffective to the DBA. **Mechanism:** `AlterSubscription` calls `GetSubscription(subid, false, orig_conninfo_needed, false)` — `conninfo_aclcheck=false` — so the conninfo is built via `ForeignServerConnectionString` without `object_aclcheck`. That conninfo feeds foreground `walrcv_connect` in `AlterSubscription_refresh` (line 1060, incl. `ReplicationSlotDropAtPubNode`), `AlterSubscription_refresh_seq` (1312), and the failover/two_phase/check_pub_rdt block (2223). The justifying comment says the ACL check "will be done by the subscription worker", but these connections happen in the ALTER command itself. CREATE (779), ALTER..SERVER (1929), OWNER TO (2710), `construct_subserver_conninfo` (2281), and worker.c (5077/5826) all enforce USAGE — the ALTER paths are the inconsistent gap. **Verifier notes:** Confirmed. Not an escalation (the owner uses their own mapping on their own subscription); the impact is that REVOKE USAGE fails to stop owner-initiated publisher connections. --- ## Confirmed — minor ### 8. \dew / \dew+ do not display the FDW connection function **Where:** `src/bin/psql/describe.c:6188` `listForeignDataWrappers()` shows Handler and Validator but never fdwconnection, in either mode; `git grep fdwconnection src/bin/psql` is empty. A DBA cannot see whether an FDW supports `CREATE SUBSCRIPTION ... SERVER` without querying the catalog directly. The commit updated \dRs+ (Server column) but missed \dew; sibling omissions from the same commit (pg_dump b71bf3b, tab completion 5fa7837, catalogs.sgml 90630ec "missed in commit 8185bb5347") were all treated as bugs and fixed. ### 9. DROP SUBSCRIPTION of a server-based subscription fails outright when the conninfo cannot be constructed and any relation is not READY, even with slot_name = NONE **Where:** `src/backend/commands/subscriptioncmds.c:2522` With non-READY pg_subscription_rel rows, DropSubscription calls `construct_subserver_conninfo()`, which traps only ACL failures into `*err`; `ForeignServerConnectionString()` failures (e.g. dropped user mapping) still ereport — a bare `user mapping not found` with no drop context and no HINT, aborting before both tolerant paths (clean return for `wrconn == NULL && !slotname`, and the hinted `ReportSlotConnectionError`). Conninfo-based subscriptions in the same state drop cleanly. Follow-up 702e9df fixed only the `rstates == NIL` case and its commit message acknowledges this residual case. The verifier notes this is a knowingly accepted limitation per the code comment — but it remains user-visible, hint-less, and untested (the regression suite covers only the slot-set variant, subscription.out:218). ### 10. DROP SERVER ... CASCADE cannot drop a dependent subscription, contradicting drop_server.sgml, with no hint and no doc of the restriction **Where:** `doc/src/sgml/ref/drop_server.sgml:65` / `src/backend/catalog/dependency.c:907` `findDependentObjects` unconditionally errors for any dependent in a shared catalog before the deptype/CASCADE handling is consulted, so `DROP SERVER s CASCADE` fails with `cannot drop server s because subscription sub depends on it` — while drop_server.sgml's CASCADE text promises automatic dropping. Blocking the drop is intentional (doDeletion cannot drop subscriptions or orphan the remote slot), but the restriction is documented nowhere, the error has no hint, and no test covers it. DROP FDW CASCADE and DROP OWNED hit it transitively. ### 11. create_foreign_data_wrapper.sgml omits the connection function's required text return type **Where:** `doc/src/sgml/ref/create_foreign_data_wrapper.sgml:113` The CONNECTION entry documents the three argument types only; `lookup_fdw_connection_func()` (foreigncmds.c:542–549) rejects any return type other than text. An author following the docs (`RETURNS varchar`) gets `function my_conn must return type text` on valid-per-docs input. Adjacent HANDLER/VALIDATOR entries do state their return-type contracts; nothing in the docs states the result is used as a libpq conninfo. ### 12. ALTER SUBSCRIPTION forms that never use a connection fail on server-based subscriptions when the conninfo cannot be constructed **Where:** `src/backend/commands/subscriptioncmds.c:1557` Purely local operations — `SKIP (lsn ...)`, `SET (disable_on_error/binary/origin/synchronous_commit ...)`, SET/ADD/DROP PUBLICATION `WITH (refresh = false)` — raise `user mapping not found` on a server-based subscription (even disabled), because `orig_conninfo_needed` stays true for every kind except the four e5c4058 exemptions (DISABLE, SERVER, CONNECTION, lone SET (slot_name=NONE)). The same commands never validate conninfo for CONNECTION-based subscriptions. Verifier note: a blanket exemption is impossible for OPTIONS/PUBLICATION kinds (refresh=true, failover, two_phase, retain_dead_tuples genuinely need conninfo) — the gap is the lack of per-option conditioning. Repro requires the mapping to exist at CREATE and be dropped after, with no PUBLIC fallback. ### 13. CREATE SUBSCRIPTION ... SERVER checks user-mapping existence before FDW subscription capability **Where:** `src/backend/commands/subscriptioncmds.c:784` (comment at 783) Pointing CREATE SUBSCRIPTION at a server of an FDW without a connection function (e.g. file_fdw) first reports `user mapping not found`; after the user creates a useless mapping, the second attempt reveals the real, unfixable `foreign data wrapper "file_fdw" does not support subscription connections`. The capability check lives inside `ForeignServerConnectionString` (foreign.c:209–214), unreachable until a mapping exists, though it needs no mapping and could run first. Same ordering in ALTER ... SERVER (1940/1942). --- ## Unverified (found, ranked below the 13-verifier cap; not adversarially checked) ### 14. User-mapping passwords logged verbatim at DEBUG1 on every worker start (minor/medium) `SetupApplyOrSyncWorker()` (worker.c:5988–5990) logs `connecting to publisher using connection string "%s"` with `MySubscription->conninfo`. The elog pre-dates the commit, but 8185bb5 newly routes pg_user_mapping secrets (password) into that string via `postgres_fdw_connection`. Previously the logged string only duplicated pg_subscription.subconninfo, and user-mapping options were never emitted anywhere; there is no redaction (cf. `libpqrcv_get_conninfo`'s password stripping). Anyone with log access sees the password. ### 15. findDependentObjects rejects shared-catalog dependents before lock-and-recheck: DROP SERVER spuriously fails while a concurrent DROP SUBSCRIPTION is committing (minor/medium) The dependency.c:907 shared-catalog error runs before `AcquireDeletionLock()`/`systable_recheck_tuple()`, so DROP SERVER errors instead of waiting out a concurrent DROP SUBSCRIPTION that has already deleted its pg_depend row but is still dropping the remote slot before commit. Any other dependent class would block and then succeed. ### 16. fdwhandler.sgml never mentions connection functions (minor/medium) The FDW-author chapter still says an author implements "a handler function, and optionally a validator function"; the new third SQL-registered support function — its contract (user oid, server oid, internal → text libpq conninfo) — is absent from the chapter. The partial signature in create_foreign_data_wrapper.sgml is the only author-facing documentation. ### 17. alter_subscription.sgml documents no requirements for ALTER SUBSCRIPTION ... SERVER and none of the new OWNER TO failure conditions (minor/medium) ALTER ... SERVER requires the subscription owner (not the invoker) to have USAGE and a user mapping on the new server, and its FDW to have a connection function; OWNER TO requires the same of the new owner. None of these appear on the ALTER SUBSCRIPTION page, while the equivalent CREATE-time requirements are documented. ### 18. Server's application_name option overrides the subscription name on the publisher (minor/low) `construct_connection_params()` serializes a server-level `application_name` into the conninfo; `libpqrcv_connect` passes the subscription name only as `fallback_application_name`, which loses. pg_stat_replication shows the shared FDW name and `synchronous_standby_names` matching by subscription name silently fails. CONNECTION-based subscriptions don't auto-inject this. ### 19. Lost-invalidation window at worker startup can leave a worker on stale server-derived conninfo indefinitely (minor/low) `InitializeLogRepWorker()` resolves conninfo via `GetSubscription` (worker.c:5826) before registering the FOREIGNSERVEROID/USERMAPPINGOID/FOREIGNDATAWRAPPEROID invalidation callbacks (5896–5916); an ALTER SERVER/USER MAPPING committing in that gap is lost, and no lock serializes server/mapping DDL against worker startup (unlike subscription DDL). Sub-millisecond window; low confidence. --- ## Coverage notes - Areas swept that produced no surviving findings: grammar/keyword regressions, node read/write/copy/equal support, extension upgrade-path state divergence (1.2→1.3 vs fresh install), tab completion (fixed post-commit by 5fa7837), catalogs.sgml (fixed post-commit by 90630ec), pg_subscription view masking, connection-string quoting/injection in `appendEscapedValue`, direct SQL calls of the connection function (nominal three-arg signature includes `internal`, blocking SQL calls). - Already-fixed follow-ups were excluded by instruction and by per-file `git log 8185bb5..master` checks in every finder and verifier. - Verification was static analysis + git archaeology only; no repros were executed against a running cluster.
