Ensure we have a snapshot when updating various system catalogs.
authorNathan Bossart <nathan@postgresql.org>
Fri, 30 May 2025 20:17:28 +0000 (15:17 -0500)
committerNathan Bossart <nathan@postgresql.org>
Fri, 30 May 2025 20:17:28 +0000 (15:17 -0500)
A few places that access system catalogs don't set up an active
snapshot before potentially accessing their TOAST tables.  To fix,
push an active snapshot just before each section of code that might
require accessing one of these TOAST tables, and pop it shortly
afterwards.  While at it, this commit adds some rather strict
assertions in an attempt to prevent such issues in the future.

Commit 16bf24e0e4 recently removed pg_replication_origin's TOAST
table in order to fix the same problem for that catalog.  On the
back-branches, those bugs are left in place.  We cannot easily
remove a catalog's TOAST table on released major versions, and only
replication origins with extremely long names are affected.  Given
the low severity of the issue, fixing older versions doesn't seem
worth the trouble of significantly modifying the patch.

Also, on v13 and v14, the aforementioned strict assertions have
been omitted because commit 2776922201, which added
HaveRegisteredOrActiveSnapshot(), was not back-patched.  While we
could probably back-patch it now, I've opted against it because it
seems unlikely that new TOAST snapshot issues will be introduced in
the oldest supported versions.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/18127-fe54b6a667f29658%40postgresql.org
Discussion: https://postgr.es/m/18309-c0bf914950c46692%40postgresql.org
Discussion: https://postgr.es/m/ZvMSUPOqUU-VNADN%40nathan
Backpatch-through: 13

src/backend/access/heap/heapam.c
src/backend/commands/indexcmds.c
src/backend/commands/tablecmds.c
src/backend/postmaster/autovacuum.c
src/backend/replication/logical/worker.c

index 9ec8cda1c6801bba3b549aa7e72da41bf822e4f2..2be7f817c78ad624014897f17331a48f0ab5f6b4 100644 (file)
@@ -213,6 +213,27 @@ static const int MultiXactStatusLock[MaxMultiXactStatus + 1] =
 #define TUPLOCK_from_mxstatus(status) \
            (MultiXactStatusLock[(status)])
 
+/*
+ * Check that we have a valid snapshot if we might need TOAST access.
+ */
+static inline void
+AssertHasSnapshotForToast(Relation rel)
+{
+#ifdef USE_ASSERT_CHECKING
+
+   /* bootstrap mode in particular breaks this rule */
+   if (!IsNormalProcessingMode())
+       return;
+
+   /* if the relation doesn't have a TOAST table, we are good */
+   if (!OidIsValid(rel->rd_rel->reltoastrelid))
+       return;
+
+   Assert(HaveRegisteredOrActiveSnapshot());
+
+#endif                         /* USE_ASSERT_CHECKING */
+}
+
 /* ----------------------------------------------------------------
  *                      heap support routines
  * ----------------------------------------------------------------
@@ -2066,6 +2087,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
    Assert(HeapTupleHeaderGetNatts(tup->t_data) <=
           RelationGetNumberOfAttributes(relation));
 
+   AssertHasSnapshotForToast(relation);
+
    /*
     * Fill in tuple header fields and toast the tuple if necessary.
     *
@@ -2343,6 +2366,8 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
    /* currently not needed (thus unsupported) for heap_multi_insert() */
    Assert(!(options & HEAP_INSERT_NO_LOGICAL));
 
+   AssertHasSnapshotForToast(relation);
+
    needwal = RelationNeedsWAL(relation);
    saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
                                                   HEAP_DEFAULT_FILLFACTOR);
@@ -2765,6 +2790,8 @@ heap_delete(Relation relation, ItemPointer tid,
 
    Assert(ItemPointerIsValid(tid));
 
+   AssertHasSnapshotForToast(relation);
+
    /*
     * Forbid this during a parallel operation, lest it allocate a combo CID.
     * Other workers might need that combo CID for visibility checks, and we
@@ -3260,6 +3287,8 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
    Assert(HeapTupleHeaderGetNatts(newtup->t_data) <=
           RelationGetNumberOfAttributes(relation));
 
+   AssertHasSnapshotForToast(relation);
+
    /*
     * Forbid this during a parallel operation, lest it allocate a combo CID.
     * Other workers might need that combo CID for visibility checks, and we
index d962fe392cd27a7503276da6410f34bf56aec0a8..c3ec2076a52ef7fc1b7f4ddc3ea482d236e4a5e4 100644 (file)
@@ -4226,7 +4226,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
                                     false);
 
        /*
-        * Updating pg_index might involve TOAST table access, so ensure we
+        * Swapping the indexes might involve TOAST table access, so ensure we
         * have a valid snapshot.
         */
        PushActiveSnapshot(GetTransactionSnapshot());
index 54ad38247aa32e34507f13ab817cb9da694580d7..acf11e83c04e3476e34c82106c668ac71ad7a83d 100644 (file)
@@ -20964,9 +20964,17 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
        tab->rel = rel;
    }
 
+   /*
+    * Detaching the partition might involve TOAST table access, so ensure we
+    * have a valid snapshot.
+    */
+   PushActiveSnapshot(GetTransactionSnapshot());
+
    /* Do the final part of detaching */
    DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid);
 
+   PopActiveSnapshot();
+
    ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel));
 
    /* keep our lock until commit */
index 981be42e3afc89c0e8c31cdf17c96cc07c31703f..451fb90a610a7a5cb7bbf482986d4458e18337d0 100644 (file)
@@ -2234,6 +2234,12 @@ do_autovacuum(void)
                        get_namespace_name(classForm->relnamespace),
                        NameStr(classForm->relname))));
 
+       /*
+        * Deletion might involve TOAST table access, so ensure we have a
+        * valid snapshot.
+        */
+       PushActiveSnapshot(GetTransactionSnapshot());
+
        object.classId = RelationRelationId;
        object.objectId = relid;
        object.objectSubId = 0;
@@ -2246,6 +2252,7 @@ do_autovacuum(void)
         * To commit the deletion, end current transaction and start a new
         * one.  Note this also releases the locks we took.
         */
+       PopActiveSnapshot();
        CommitTransactionCommand();
        StartTransactionCommand();
 
index 4151a4b2a96ba67803895b61aa344e8ab7254d78..a23262957acb56913996c076aeb46bd958cda0f2 100644 (file)
@@ -4626,8 +4626,16 @@ run_apply_worker()
        walrcv_startstreaming(LogRepWorkerWalRcvConn, &options);
 
        StartTransactionCommand();
+
+       /*
+        * Updating pg_subscription might involve TOAST table access, so
+        * ensure we have a valid snapshot.
+        */
+       PushActiveSnapshot(GetTransactionSnapshot());
+
        UpdateTwoPhaseState(MySubscription->oid, LOGICALREP_TWOPHASE_STATE_ENABLED);
        MySubscription->twophasestate = LOGICALREP_TWOPHASE_STATE_ENABLED;
+       PopActiveSnapshot();
        CommitTransactionCommand();
    }
    else
@@ -4843,7 +4851,15 @@ DisableSubscriptionAndExit(void)
 
    /* Disable the subscription */
    StartTransactionCommand();
+
+   /*
+    * Updating pg_subscription might involve TOAST table access, so ensure we
+    * have a valid snapshot.
+    */
+   PushActiveSnapshot(GetTransactionSnapshot());
+
    DisableSubscription(MySubscription->oid);
+   PopActiveSnapshot();
    CommitTransactionCommand();
 
    /* Ensure we remove no-longer-useful entry for worker's start time */
@@ -4947,6 +4963,12 @@ clear_subscription_skip_lsn(XLogRecPtr finish_lsn)
        started_tx = true;
    }
 
+   /*
+    * Updating pg_subscription might involve TOAST table access, so ensure we
+    * have a valid snapshot.
+    */
+   PushActiveSnapshot(GetTransactionSnapshot());
+
    /*
     * Protect subskiplsn of pg_subscription from being concurrently updated
     * while clearing it.
@@ -5005,6 +5027,8 @@ clear_subscription_skip_lsn(XLogRecPtr finish_lsn)
    heap_freetuple(tup);
    table_close(rel, NoLock);
 
+   PopActiveSnapshot();
+
    if (started_tx)
        CommitTransactionCommand();
 }