Deduplicate choice of horizon for a relation procarray.c.

5a1e1d83022 was a minimal bug fix for dc7420c2c92. To avoid future bugs of
that kind, deduplicate the choice of a relation's horizon into a new helper,
GlobalVisHorizonKindForRel().

As the code in question was only introduced in dc7420c2c92 it seems worth
backpatching this change as well, otherwise 14 will look different from all
other branches.

A different approach to this was suggested by Matthias van de Meent.

Author: Andres Freund
Discussion: https://postgr.es/m/20210621122919.2qhu3pfugxxp3cji@alap3.anarazel.de
Backpatch: 14, like 5a1e1d83022

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 793df973b4248cbb5a818602cd6793a57d0e6fde..aa3e1419a9e4a35bd50463efec6ffdfa4e300060 100644 (file)
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -246,6 +246,17 @@ typedef struct ComputeXidHorizonsResult
 
 } ComputeXidHorizonsResult;
 
+/*
+ * Return value for GlobalVisHorizonKindForRel().
+ */
+typedef enum GlobalVisHorizonKind
+{
+       VISHORIZON_SHARED,
+       VISHORIZON_CATALOG,
+       VISHORIZON_DATA,
+       VISHORIZON_TEMP
+} GlobalVisHorizonKind;
+
 
 static ProcArrayStruct *procArray;
 
@@ -1952,6 +1963,33 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
        GlobalVisUpdateApply(h);
 }
 
+/*
+ * Determine what kind of visibility horizon needs to be used for a
+ * relation. If rel is NULL, the most conservative horizon is used.
+ */
+static inline GlobalVisHorizonKind
+GlobalVisHorizonKindForRel(Relation rel)
+{
+       /*
+        * Other relkkinds currently don't contain xids, nor always the necessary
+        * logical decoding markers.
+        */
+       Assert(!rel ||
+                  rel->rd_rel->relkind == RELKIND_RELATION ||
+                  rel->rd_rel->relkind == RELKIND_MATVIEW ||
+                  rel->rd_rel->relkind == RELKIND_TOASTVALUE);
+
+       if (rel == NULL || rel->rd_rel->relisshared || RecoveryInProgress())
+               return VISHORIZON_SHARED;
+       else if (IsCatalogRelation(rel) ||
+                        RelationIsAccessibleInLogicalDecoding(rel))
+               return VISHORIZON_CATALOG;
+       else if (!RELATION_IS_LOCAL(rel))
+               return VISHORIZON_DATA;
+       else
+               return VISHORIZON_TEMP;
+}
+
 /*
  * Return the oldest XID for which deleted tuples must be preserved in the
  * passed table.
@@ -1970,16 +2008,19 @@ GetOldestNonRemovableTransactionId(Relation rel)
 
        ComputeXidHorizons(&horizons);
 
-       /* select horizon appropriate for relation */
-       if (rel == NULL || rel->rd_rel->relisshared || RecoveryInProgress())
-               return horizons.shared_oldest_nonremovable;
-       else if (IsCatalogRelation(rel) ||
-                        RelationIsAccessibleInLogicalDecoding(rel))
-               return horizons.catalog_oldest_nonremovable;
-       else if (RELATION_IS_LOCAL(rel))
-               return horizons.temp_oldest_nonremovable;
-       else
-               return horizons.data_oldest_nonremovable;
+       switch (GlobalVisHorizonKindForRel(rel))
+       {
+               case VISHORIZON_SHARED:
+                       return horizons.shared_oldest_nonremovable;
+               case VISHORIZON_CATALOG:
+                       return horizons.catalog_oldest_nonremovable;
+               case VISHORIZON_DATA:
+                       return horizons.data_oldest_nonremovable;
+               case VISHORIZON_TEMP:
+                       return horizons.temp_oldest_nonremovable;
+       }
+
+       return InvalidTransactionId;
 }
 
 /*
@@ -3986,38 +4027,27 @@ DisplayXidCache(void)
 GlobalVisState *
 GlobalVisTestFor(Relation rel)
 {
-       bool            need_shared;
-       bool            need_catalog;
        GlobalVisState *state;
 
        /* XXX: we should assert that a snapshot is pushed or registered */
        Assert(RecentXmin);
 
-       if (!rel)
-               need_shared = need_catalog = true;
-       else
+       switch (GlobalVisHorizonKindForRel(rel))
        {
-               /*
-                * Other kinds currently don't contain xids, nor always the necessary
-                * logical decoding markers.
-                */
-               Assert(rel->rd_rel->relkind == RELKIND_RELATION ||
-                          rel->rd_rel->relkind == RELKIND_MATVIEW ||
-                          rel->rd_rel->relkind == RELKIND_TOASTVALUE);
-
-               need_shared = rel->rd_rel->relisshared || RecoveryInProgress();
-               need_catalog = IsCatalogRelation(rel) || RelationIsAccessibleInLogicalDecoding(rel);
+               case VISHORIZON_SHARED:
+                       state = &GlobalVisSharedRels;
+                       break;
+               case VISHORIZON_CATALOG:
+                       state = &GlobalVisCatalogRels;
+                       break;
+               case VISHORIZON_DATA:
+                       state = &GlobalVisDataRels;
+                       break;
+               case VISHORIZON_TEMP:
+                       state = &GlobalVisTempRels;
+                       break;
        }
 
-       if (need_shared)
-               state = &GlobalVisSharedRels;
-       else if (need_catalog)
-               state = &GlobalVisCatalogRels;
-       else if (RELATION_IS_LOCAL(rel))
-               state = &GlobalVisTempRels;
-       else
-               state = &GlobalVisDataRels;
-
        Assert(FullTransactionIdIsValid(state->definitely_needed) &&
                   FullTransactionIdIsValid(state->maybe_needed));