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 2068e03c571dbe1b8c912d0df2e795aca116bbec..5ff61edb989cc7d359355229562a3d6837cbafca 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;
@@ -272,6 +278,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. */
@@ -714,6 +723,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 4292e7fb8f46453d441f0c7bf2de5ec2067f2c9e..630d2498fa0782fadf4ef6c73ae7a2d0a8ca3181 100644 (file)
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -2868,6 +2868,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;
 
@@ -2987,6 +2990,7 @@ AlterSubscriptionOwner_oid(Oid subid, Oid newOwnerId)
 {
        HeapTuple       tup;
        Relation        rel;
+       Form_pg_subscription form;
 
        rel = table_open(SubscriptionRelationId, RowExclusiveLock);
 
@@ -2997,7 +3001,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);