Avoid repeating loads of frozen ID values.

Repeating loads of inplace-updated fields tends to cause bugs like the
one from the previous commit.  While there's no bug to fix in these code
sites, adopt the load-once style.  This improves the chance of future
copy/paste finding the safe style.

Discussion: https://postgr.es/m/20240423003956.e7.nmisch@google.com

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 32e7d3c146436b36f1b5d4a736a529ac57143c7a..4be0dee4de0b544ed6f85ac6c135dd719d06013e 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -5907,7 +5907,6 @@ heap_abort_speculative(Relation relation, ItemPointer tid)
        Page            page;
        BlockNumber block;
        Buffer          buffer;
-       TransactionId prune_xid;
 
        Assert(ItemPointerIsValid(tid));
 
@@ -5960,11 +5959,16 @@ heap_abort_speculative(Relation relation, ItemPointer tid)
         * TransactionXmin, so there's no race here).
         */
        Assert(TransactionIdIsValid(TransactionXmin));
-       if (TransactionIdPrecedes(TransactionXmin, relation->rd_rel->relfrozenxid))
-               prune_xid = relation->rd_rel->relfrozenxid;
-       else
-               prune_xid = TransactionXmin;
-       PageSetPrunable(page, prune_xid);
+       {
+               TransactionId relfrozenxid = relation->rd_rel->relfrozenxid;
+               TransactionId prune_xid;
+
+               if (TransactionIdPrecedes(TransactionXmin, relfrozenxid))
+                       prune_xid = relfrozenxid;
+               else
+                       prune_xid = TransactionXmin;
+               PageSetPrunable(page, prune_xid);
+       }
 
        /* store transaction information of xact deleting the tuple */
        tp.t_data->t_infomask &= ~(HEAP_XMAX_BITS | HEAP_MOVED);

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index c04886c4090ababde2f19584c2c79e8a8aafd4db..78f96789b0e84dc0a246c9e0334455e0201c997b 100644 (file)
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -919,18 +919,24 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
         * FreezeXid will become the table's new relfrozenxid, and that mustn't go
         * backwards, so take the max.
         */
-       if (TransactionIdIsValid(OldHeap->rd_rel->relfrozenxid) &&
-               TransactionIdPrecedes(cutoffs.FreezeLimit,
-                                                         OldHeap->rd_rel->relfrozenxid))
-               cutoffs.FreezeLimit = OldHeap->rd_rel->relfrozenxid;
+       {
+               TransactionId relfrozenxid = OldHeap->rd_rel->relfrozenxid;
+
+               if (TransactionIdIsValid(relfrozenxid) &&
+                       TransactionIdPrecedes(cutoffs.FreezeLimit, relfrozenxid))
+                       cutoffs.FreezeLimit = relfrozenxid;
+       }
 
        /*
         * MultiXactCutoff, similarly, shouldn't go backwards either.
         */
-       if (MultiXactIdIsValid(OldHeap->rd_rel->relminmxid) &&
-               MultiXactIdPrecedes(cutoffs.MultiXactCutoff,
-                                                       OldHeap->rd_rel->relminmxid))
-               cutoffs.MultiXactCutoff = OldHeap->rd_rel->relminmxid;
+       {
+               MultiXactId relminmxid = OldHeap->rd_rel->relminmxid;
+
+               if (MultiXactIdIsValid(relminmxid) &&
+                       MultiXactIdPrecedes(cutoffs.MultiXactCutoff, relminmxid))
+                       cutoffs.MultiXactCutoff = relminmxid;
+       }
 
        /*
         * Decide whether to use an indexscan or seqscan-and-optional-sort to scan

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a63a71c984de017d7f0b5bbb2022f92701fc4a59..521ee74586a6e4e39dc1779c9d27df7cfdd4f6b4 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1200,7 +1200,7 @@ vacuum_get_cutoffs(Relation rel, const VacuumParams *params,
        aggressiveXIDCutoff = nextXID - freeze_table_age;
        if (!TransactionIdIsNormal(aggressiveXIDCutoff))
                aggressiveXIDCutoff = FirstNormalTransactionId;
-       if (TransactionIdPrecedesOrEquals(rel->rd_rel->relfrozenxid,
+       if (TransactionIdPrecedesOrEquals(cutoffs->relfrozenxid,
                                                                          aggressiveXIDCutoff))
                return true;
 
@@ -1221,7 +1221,7 @@ vacuum_get_cutoffs(Relation rel, const VacuumParams *params,
        aggressiveMXIDCutoff = nextMXID - multixact_freeze_table_age;
        if (aggressiveMXIDCutoff < FirstMultiXactId)
                aggressiveMXIDCutoff = FirstMultiXactId;
-       if (MultiXactIdPrecedesOrEquals(rel->rd_rel->relminmxid,
+       if (MultiXactIdPrecedesOrEquals(cutoffs->relminmxid,
                                                                        aggressiveMXIDCutoff))
                return true;
 

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index c367ede6f88530eb725fcb18b42f566b87443afa..9a925a10cdc7df3b77949b6197e83c0c46de28b8 100644 (file)
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2938,6 +2938,7 @@ relation_needs_vacanalyze(Oid relid,
        int                     freeze_max_age;
        int                     multixact_freeze_max_age;
        TransactionId xidForceLimit;
+       TransactionId relfrozenxid;
        MultiXactId multiForceLimit;
 
        Assert(classForm != NULL);
@@ -2989,16 +2990,18 @@ relation_needs_vacanalyze(Oid relid,
        xidForceLimit = recentXid - freeze_max_age;
        if (xidForceLimit < FirstNormalTransactionId)
                xidForceLimit -= FirstNormalTransactionId;
-       force_vacuum = (TransactionIdIsNormal(classForm->relfrozenxid) &&
-                                       TransactionIdPrecedes(classForm->relfrozenxid,
-                                                                                 xidForceLimit));
+       relfrozenxid = classForm->relfrozenxid;
+       force_vacuum = (TransactionIdIsNormal(relfrozenxid) &&
+                                       TransactionIdPrecedes(relfrozenxid, xidForceLimit));
        if (!force_vacuum)
        {
+               MultiXactId relminmxid = classForm->relminmxid;
+
                multiForceLimit = recentMulti - multixact_freeze_max_age;
                if (multiForceLimit < FirstMultiXactId)
                        multiForceLimit -= FirstMultiXactId;
-               force_vacuum = MultiXactIdIsValid(classForm->relminmxid) &&
-                       MultiXactIdPrecedes(classForm->relminmxid, multiForceLimit);
+               force_vacuum = MultiXactIdIsValid(relminmxid) &&
+                       MultiXactIdPrecedes(relminmxid, multiForceLimit);
        }
        *wraparound = force_vacuum;