Fix REASSIGN OWNED for subscriptions in other databases.

Subscription objects are conceptually database-local objects, but
pg_subscription is a shared catalog so that the launcher process can
scan it.

Check readers of pg_subscription to ensure that, unless it's the
launcher process, it filters by MyDatabaseId. Most readers were
already doing so, but this commit fixes REASSIGN OWNED and adds guards
to catch other problems in the future. Also, clarify documentation.

Author: Dilip Kumar <dilipbalaut@gmail.com>
Reported-by: Noah Misch <noah@leadboat.com>
Reviewed-by: shveta malik <shveta.malik@gmail.com>
Discussion: https://postgr.es/m/20260710192533.4f.noahmisch@microsoft.com
Backpatch-through: 19

diff --git a/contrib/postgres_fdw/t/010_subscription.pl b/contrib/postgres_fdw/t/010_subscription.pl
index 163c788d209665998d4a060cb2548c6126645eeb..c34b3d15b8dfc2a911c520dc27f80cbb7db7dd39 100644 (file)
--- a/contrib/postgres_fdw/t/010_subscription.pl
+++ b/contrib/postgres_fdw/t/010_subscription.pl
@@ -99,4 +99,49 @@ $result =
 is($result, qq(1073),
        'check subscription after ALTER SUBSCRIPTION ... SERVER');
 
+# Check that REASSIGN OWNED processes a subscription only when run in the
+# database to which the subscription belongs.
+$node_subscriber->safe_psql(
+       'postgres', q{
+       CREATE ROLE regress_sub_old;
+       CREATE ROLE regress_sub_new;
+       GRANT pg_create_subscription TO regress_sub_old;
+       CREATE DATABASE regress_sub_db;
+});
+
+$node_subscriber->safe_psql(
+       'regress_sub_db', qq{
+       CREATE EXTENSION postgres_fdw;
+       CREATE SERVER regress_sub_server FOREIGN DATA WRAPPER postgres_fdw
+               OPTIONS (host '$publisher_host', port '$publisher_port', dbname 'postgres');
+       CREATE USER MAPPING FOR regress_sub_old SERVER regress_sub_server
+               OPTIONS (user 'unused', password 'secret');
+       CREATE USER MAPPING FOR regress_sub_new SERVER regress_sub_server
+               OPTIONS (user 'unused', password 'secret');
+       GRANT USAGE ON FOREIGN SERVER regress_sub_server
+               TO regress_sub_old, regress_sub_new;
+       GRANT CREATE ON DATABASE regress_sub_db TO regress_sub_old;
+       SET SESSION AUTHORIZATION regress_sub_old;
+       CREATE SUBSCRIPTION regress_sub SERVER regress_sub_server
+               PUBLICATION tap_pub WITH (connect = false, slot_name = NONE);
+});
+
+$node_subscriber->safe_psql('postgres',
+       'REASSIGN OWNED BY regress_sub_old TO regress_sub_new');
+$result = $node_subscriber->safe_psql(
+       'postgres',
+       q{SELECT subowner::regrole FROM pg_subscription
+         WHERE subname = 'regress_sub'});
+is($result, 'regress_sub_old',
+       'REASSIGN OWNED in another database skips subscription');
+
+$node_subscriber->safe_psql('regress_sub_db',
+       'REASSIGN OWNED BY regress_sub_old TO regress_sub_new');
+$result = $node_subscriber->safe_psql(
+       'regress_sub_db',
+       q{SELECT subowner::regrole FROM pg_subscription
+         WHERE subname = 'regress_sub'});
+is($result, 'regress_sub_new',
+       'REASSIGN OWNED in subscription database changes owner');
+
 done_testing();

diff --git a/doc/src/sgml/ref/reassign_owned.sgml b/doc/src/sgml/ref/reassign_owned.sgml
index ab692bd06908b34334a75790739b667e0dc35c98..5fca2329de24e80ea56cb0d2042f8c0a0be14bd6 100644 (file)
--- a/doc/src/sgml/ref/reassign_owned.sgml
+++ b/doc/src/sgml/ref/reassign_owned.sgml
@@ -48,7 +48,10 @@ REASSIGN OWNED BY { <replaceable class="parameter">old_role</replaceable> | CURR
       The name of a role. The ownership of all the objects within the
       current database, and of all shared objects (databases, tablespaces),
       owned by this role will be reassigned to
-      <replaceable class="parameter">new_role</replaceable>.
+      <replaceable class="parameter">new_role</replaceable>. Subscriptions,
+      although stored in a shared catalog, belong to a specific database
+      and are reassigned only when <command>REASSIGN OWNED</command> is executed
+      in that database.
      </para>
     </listitem>
    </varlistentry>

diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index d68dd2610121ea09ef44cd7dba82fb163dcb34b3..1663794caec2103d5a49ff939d505f2736886271 100644 (file)
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -530,7 +530,9 @@ ALTER TABLE bobs_table OWNER TO alice;
    that contains objects owned by the role.  (Note that the first
    such <command>REASSIGN OWNED</command> will change the ownership of any
    shared-across-databases objects, that is databases or tablespaces, that
-   are owned by the role-to-be-dropped.)
+   are owned by the role-to-be-dropped. Subscriptions, while stored in a
+   shared catalog, belong to a specific database and are reassigned only when
+   <command>REASSIGN OWNED</command> is executed in that database.)
   </para>
 
   <para>

diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index b5cb301db88a0593ee3e706fbab9cf8c602fb2cb..d1a110f1ff30555db22c992e76092162c5998c02 100644 (file)
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -114,6 +114,12 @@ GetSubscription(Oid subid, bool missing_ok, bool conninfo_needed,
 
        subform = (Form_pg_subscription) GETSTRUCT(tup);
 
+       /*
+        * It's only safe to access subscriptions from another database from the
+        * launcher process, which does not call GetSubscription().
+        */
+       Assert(subform->subdbid == MyDatabaseId);
+
        sub = palloc0_object(Subscription);
        sub->cxt = cxt;
        sub->oid = subid;
@@ -265,6 +271,9 @@ DisableSubscription(Oid subid)
        if (!HeapTupleIsValid(tup))
                elog(ERROR, "cache lookup failed for subscription %u", subid);
 
+       /* Must only modify subscriptions belonging to the current database. */
+       Assert(((Form_pg_subscription) GETSTRUCT(tup))->subdbid == MyDatabaseId);
+
        LockSharedObject(SubscriptionRelationId, subid, 0, AccessShareLock);
 
        /* Form a new tuple. */
@@ -707,6 +716,9 @@ UpdateDeadTupleRetentionStatus(Oid subid, bool active)
        if (!HeapTupleIsValid(tup))
                elog(ERROR, "cache lookup failed for subscription %u", subid);
 
+       /* Must only modify subscriptions belonging to the current database. */
+       Assert(((Form_pg_subscription) GETSTRUCT(tup))->subdbid == MyDatabaseId);
+
        LockSharedObject(SubscriptionRelationId, subid, 0, AccessShareLock);
 
        /* Form a new tuple. */

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index ee06a726f420d3f14d1df853e14fece6b0022f18..1284d4107900297c962a90bfb8b88af656e9c5cb 100644 (file)
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -2665,6 +2665,9 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
 
        form = (Form_pg_subscription) GETSTRUCT(tup);
 
+       /* Must only alter subscriptions belonging to the current database. */
+       Assert(form->subdbid == MyDatabaseId);
+
        if (form->subowner == newOwnerId)
                return;
 
@@ -2779,6 +2782,7 @@ AlterSubscriptionOwner_oid(Oid subid, Oid newOwnerId)
 {
        HeapTuple       tup;
        Relation        rel;
+       Form_pg_subscription form;
 
        rel = table_open(SubscriptionRelationId, RowExclusiveLock);
 
@@ -2789,7 +2793,15 @@ AlterSubscriptionOwner_oid(Oid subid, Oid newOwnerId)
                                (errcode(ERRCODE_UNDEFINED_OBJECT),
                                 errmsg("subscription with OID %u does not exist", subid)));
 
-       AlterSubscriptionOwner_internal(rel, tup, newOwnerId);
+       form = (Form_pg_subscription) GETSTRUCT(tup);
+
+       /*
+        * Don't process subscriptions belonging to other databases. While
+        * pg_subscription is a shared catalog, subscriptions refer to db-local
+        * objects which exist only in the database identified by subdbid.
+        */
+       if (form->subdbid == MyDatabaseId)
+               AlterSubscriptionOwner_internal(rel, tup, newOwnerId);
 
        heap_freetuple(tup);