Get rid of SET LOGGED indexes persistence kludge

This removes ATChangeIndexesPersistence() introduced by f41872d0c1239d36
which was too ugly to live for long.  Instead, the correct persistence
marking is passed all the way down to reindex_index, so that the
transient relation built to contain the index relfilenode can
get marked correctly right from the start.

Author: Fabrízio de Royes Mello
Review and editorialization by Michael Paquier
                                     and Álvaro Herrera

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 912038a712efe10c04579cb450b3e1586f20c8dc..b57aa95fafd49b6983af6e4424bf38083a54eabd 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1980,7 +1980,7 @@ index_build(Relation heapRelation,
         * created it, or truncated twice in a subsequent transaction, the
         * relfilenode won't change, and nothing needs to be done here.
         */
-       if (heapRelation->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED &&
+       if (indexRelation->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED &&
                !smgrexists(indexRelation->rd_smgr, INIT_FORKNUM))
        {
                RegProcedure ambuildempty = indexRelation->rd_am->ambuildempty;
@@ -3130,7 +3130,7 @@ IndexGetRelation(Oid indexId, bool missing_ok)
  * reindex_index - This routine is used to recreate a single index
  */
 void
-reindex_index(Oid indexId, bool skip_constraint_checks)
+reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
 {
        Relation        iRel,
                                heapRelation;
@@ -3191,6 +3191,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks)
                        indexInfo->ii_ExclusionStrats = NULL;
                }
 
+               /* Set the relpersistence of the new index */
+               iRel->rd_rel->relpersistence = persistence;
+
                /* We'll build a new physical relation for the index */
                RelationSetNewRelfilenode(iRel, InvalidTransactionId,
                                                                  InvalidMultiXactId);
@@ -3310,6 +3313,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks)
  * performance, other callers should include the flag only after transforming
  * the data in a manner that risks a change in constraint validity.
  *
+ * REINDEX_REL_FORCE_INDEXES_UNLOGGED: if true, set the persistence of the
+ * rebuilt indexes to unlogged.
+ *
+ * REINDEX_REL_FORCE_INDEXES_LOGGED: if true, set the persistence of the
+ * rebuilt indexes to permanent.
+ *
  * Returns true if any indexes were rebuilt (including toast table's index
  * when relevant).  Note that a CommandCounterIncrement will occur after each
  * index rebuild.
@@ -3371,6 +3380,7 @@ reindex_relation(Oid relid, int flags)
        {
                List       *doneIndexes;
                ListCell   *indexId;
+               char            persistence;
 
                if (flags & REINDEX_REL_SUPPRESS_INDEX_USE)
                {
@@ -3384,6 +3394,17 @@ reindex_relation(Oid relid, int flags)
                        CommandCounterIncrement();
                }
 
+               /*
+                * Compute persistence of indexes: same as that of owning rel, unless
+                * caller specified otherwise.
+                */
+               if (flags & REINDEX_REL_FORCE_INDEXES_UNLOGGED)
+                       persistence = RELPERSISTENCE_UNLOGGED;
+               else if (flags & REINDEX_REL_FORCE_INDEXES_PERMANENT)
+                       persistence = RELPERSISTENCE_PERMANENT;
+               else
+                       persistence = rel->rd_rel->relpersistence;
+
                /* Reindex all the indexes. */
                doneIndexes = NIL;
                foreach(indexId, indexIds)
@@ -3393,7 +3414,8 @@ reindex_relation(Oid relid, int flags)
                        if (is_pg_class)
                                RelationSetIndexList(rel, doneIndexes, InvalidOid);
 
-                       reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS));
+                       reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
+                                                 persistence);
 
                        CommandCounterIncrement();
 

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 6a578ec58f59f805b9122a04ea224a9c959199de..bc5f33fb78574c1073eb14babc9baf39fa57b79a 100644 (file)
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -589,7 +589,8 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
         */
        finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog,
                                         swap_toast_by_content, false, true,
-                                        frozenXid, cutoffMulti);
+                                        frozenXid, cutoffMulti,
+                                        OldHeap->rd_rel->relpersistence);
 }
 
 
@@ -1475,7 +1476,8 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
                                 bool check_constraints,
                                 bool is_internal,
                                 TransactionId frozenXid,
-                                MultiXactId cutoffMulti)
+                                MultiXactId cutoffMulti,
+                                char newrelpersistence)
 {
        ObjectAddress object;
        Oid                     mapped_tables[4];
@@ -1519,6 +1521,16 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
        reindex_flags = REINDEX_REL_SUPPRESS_INDEX_USE;
        if (check_constraints)
                reindex_flags |= REINDEX_REL_CHECK_CONSTRAINTS;
+
+       /*
+        * Ensure that the indexes have the same persistence as the parent
+        * relation.
+        */
+       if (newrelpersistence == RELPERSISTENCE_UNLOGGED)
+               reindex_flags |= REINDEX_REL_FORCE_INDEXES_UNLOGGED;
+       else if (newrelpersistence == RELPERSISTENCE_PERMANENT)
+               reindex_flags |= REINDEX_REL_FORCE_INDEXES_PERMANENT;
+
        reindex_relation(OIDOldHeap, reindex_flags);
 
        /*

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 02055950b58b523f14c87652ad0a7d5518000f38..12b4ac7b3ce74d1647eb7b9ea76d4bf2df7e4a42 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1689,7 +1689,7 @@ ReindexIndex(RangeVar *indexRelation)
                                                                          RangeVarCallbackForReindexIndex,
                                                                          (void *) &heapOid);
 
-       reindex_index(indOid, false);
+       reindex_index(indOid, false, indexRelation->relpersistence);
 
        return indOid;
 }

diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 523ba35ba245c550c5fb12fcddf40d225dec7eb9..b19da4ac63cf109a8ac9d2f90d878d36b8a20de9 100644 (file)
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -67,7 +67,7 @@ static void mv_GenerateOper(StringInfo buf, Oid opoid);
 
 static void refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
                                                 int save_sec_context);
-static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap);
+static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence);
 
 static void OpenMatViewIncrementalMaintenance(void);
 static void CloseMatViewIncrementalMaintenance(void);
@@ -303,7 +303,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
                Assert(matview_maintenance_depth == old_depth);
        }
        else
-               refresh_by_heap_swap(matviewOid, OIDNewHeap);
+               refresh_by_heap_swap(matviewOid, OIDNewHeap, relpersistence);
 
        /* Roll back any GUC changes */
        AtEOXact_GUC(false, save_nestlevel);
@@ -759,10 +759,10 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
  * swapping is handled by the called function, so it is not needed here.
  */
 static void
-refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap)
+refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence)
 {
        finish_heap_swap(matviewOid, OIDNewHeap, false, false, true, true,
-                                        RecentXmin, ReadNextMultiXactId());
+                                        RecentXmin, ReadNextMultiXactId(), relpersistence);
 }
 
 

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 714a9f1ee787e9b1d69be32a7e1f4b78eaa2ee1b..093224f4e644875ebed72b51f12b247031575df9 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -393,7 +393,6 @@ static void ATExecClusterOn(Relation rel, const char *indexName,
                                LOCKMODE lockmode);
 static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
 static bool ATPrepChangePersistence(Relation rel, bool toLogged);
-static void ATChangeIndexesPersistence(Oid relid, char relpersistence);
 static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                                        char *tablespacename, LOCKMODE lockmode);
 static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode);
@@ -3734,16 +3733,6 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
                         */
                        ATRewriteTable(tab, OIDNewHeap, lockmode);
 
-                       /*
-                        * Change the persistence marking of indexes, if necessary.  This
-                        * is so that the new copies are built with the right persistence
-                        * in the reindex step below.  Note we cannot do this earlier,
-                        * because the rewrite step might read the indexes, and that would
-                        * cause buffers for them to have the wrong setting.
-                        */
-                       if (tab->chgPersistence)
-                               ATChangeIndexesPersistence(tab->relid, tab->newrelpersistence);
-
                        /*
                         * Swap the physical files of the old and new heaps, then rebuild
                         * indexes and discard the old heap.  We can use RecentXmin for
@@ -3756,7 +3745,8 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
                                                         false, false, true,
                                                         !OidIsValid(tab->newTableSpace),
                                                         RecentXmin,
-                                                        ReadNextMultiXactId());
+                                                        ReadNextMultiXactId(),
+                                                        persistence);
                }
                else
                {
@@ -10879,48 +10869,6 @@ ATPrepChangePersistence(Relation rel, bool toLogged)
        return true;
 }
 
-/*
- * Update the pg_class entry of each index for the given relation to the
- * given persistence.
- */
-static void
-ATChangeIndexesPersistence(Oid relid, char relpersistence)
-{
-       Relation        rel;
-       Relation        pg_class;
-       List       *indexes;
-       ListCell   *cell;
-
-       pg_class = heap_open(RelationRelationId, RowExclusiveLock);
-
-       /* We already have a lock on the table */
-       rel = relation_open(relid, NoLock);
-       indexes = RelationGetIndexList(rel);
-       foreach(cell, indexes)
-       {
-               Oid                     indexid = lfirst_oid(cell);
-               HeapTuple       tuple;
-               Form_pg_class pg_class_form;
-
-               tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexid));
-               if (!HeapTupleIsValid(tuple))
-                       elog(ERROR, "cache lookup failed for relation %u",
-                                indexid);
-
-               pg_class_form = (Form_pg_class) GETSTRUCT(tuple);
-               pg_class_form->relpersistence = relpersistence;
-               simple_heap_update(pg_class, &tuple->t_self, tuple);
-
-               /* keep catalog indexes current */
-               CatalogUpdateIndexes(pg_class, tuple);
-
-               heap_freetuple(tuple);
-       }
-
-       heap_close(pg_class, RowExclusiveLock);
-       heap_close(rel, NoLock);
-}
-
 /*
  * Execute ALTER TABLE SET SCHEMA
  */

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 4b4528f79175945c3762a9bc4cd76bbc84f67d87..2250c56d28b219c3acfa5f1c9aa12b9e1b1db23d 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -3078,6 +3078,7 @@ RelationSetNewRelfilenode(Relation relation, TransactionId freezeXid,
        }
        classform->relfrozenxid = freezeXid;
        classform->relminmxid = minmulti;
+       classform->relpersistence = relation->rd_rel->relpersistence;
 
        simple_heap_update(pg_class, &tuple->t_self, tuple);
        CatalogUpdateIndexes(pg_class, tuple);

diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index c36a729c91f90b036535420533311f3a6966a2e1..199f2d7834df5621f8eeaa6b3e731ae6d2e3feae 100644 (file)
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -111,12 +111,15 @@ extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot);
 
 extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
 
-extern void reindex_index(Oid indexId, bool skip_constraint_checks);
+extern void reindex_index(Oid indexId, bool skip_constraint_checks,
+                         char relpersistence);
 
 /* Flag bits for reindex_relation(): */
-#define REINDEX_REL_PROCESS_TOAST              0x01
-#define REINDEX_REL_SUPPRESS_INDEX_USE 0x02
-#define REINDEX_REL_CHECK_CONSTRAINTS  0x04
+#define REINDEX_REL_PROCESS_TOAST                      0x01
+#define REINDEX_REL_SUPPRESS_INDEX_USE         0x02
+#define REINDEX_REL_CHECK_CONSTRAINTS          0x04
+#define REINDEX_REL_FORCE_INDEXES_UNLOGGED     0x08
+#define REINDEX_REL_FORCE_INDEXES_PERMANENT    0x10
 
 extern bool reindex_relation(Oid relid, int flags);
 

diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
index f7730a9c03520e1f97678b513b9c5c5ceef7df3f..0b7e87798e290ed28688488ea0d867aa176c7b85 100644 (file)
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/cluster.h
@@ -33,6 +33,7 @@ extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
                                 bool check_constraints,
                                 bool is_internal,
                                 TransactionId frozenXid,
-                                MultiXactId minMulti);
+                                MultiXactId minMulti,
+                                char newrelpersistence);
 
 #endif   /* CLUSTER_H */