diff options
author | Noah Misch <noah@leadboat.com> | 2024-04-29 10:25:33 -0700 |
---|---|---|
committer | Noah Misch <noah@leadboat.com> | 2024-04-29 10:25:33 -0700 |
commit | dd0183469bb779247c96e86c2272dca7ff4ec9e7 (patch) | |
tree | 1e2998dec368e7476dde36894e2b7f4356308f12 /src/backend/commands | |
parent | f65ab862e3b8d96c6886641155d9447bc73b5126 (diff) | |
download | postgresql-dd0183469bb779247c96e86c2272dca7ff4ec9e7.tar.gz postgresql-dd0183469bb779247c96e86c2272dca7ff4ec9e7.zip |
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
Diffstat (limited to 'src/backend/commands')
-rw-r--r-- | src/backend/commands/cluster.c | 22 | ||||
-rw-r--r-- | src/backend/commands/vacuum.c | 4 |
2 files changed, 16 insertions, 10 deletions
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index c04886c4090..78f96789b0e 100644 --- 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 a63a71c984d..521ee74586a 100644 --- 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; |