Editorial review of SET UNLOGGED

Add a succint comment explaining why it's correct to change the
persistence in this way.  Also s/loggedness/persistence/ because native
speakers didn't like the latter term.

Fabrízio and Álvaro

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d37534ed369eab929412eec06a43c06b78b73bfb..56915c4fe389083e217bfe2cc0ad2b311d92c240 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -152,7 +152,7 @@ typedef struct AlteredTableInfo
    bool        new_notnull;    /* T if we added new NOT NULL constraints */
    bool        rewrite;        /* T if a rewrite is forced */
    Oid         newTableSpace;  /* new tablespace; 0 means no change */
-   bool        chgLoggedness;  /* T if SET LOGGED/UNLOGGED is used */
+   bool        chgPersistence; /* T if SET LOGGED/UNLOGGED is used */
    char        newrelpersistence;      /* if above is true */
    /* Objects to rebuild after completing ALTER TYPE operations */
    List       *changedConstraintOids;  /* OIDs of constraints to rebuild */
@@ -388,8 +388,8 @@ static void change_owner_recurse_to_sequences(Oid relationOid,
 static void ATExecClusterOn(Relation rel, const char *indexName,
                LOCKMODE lockmode);
 static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
-static bool ATPrepChangeLoggedness(Relation rel, bool toLogged);
-static void ATChangeIndexesLoggedness(Oid relid, char relpersistence);
+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);
@@ -3174,19 +3174,19 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
            break;
        case AT_SetLogged:      /* SET LOGGED */
            ATSimplePermissions(rel, ATT_TABLE);
-           tab->chgLoggedness = ATPrepChangeLoggedness(rel, true);
+           tab->chgPersistence = ATPrepChangePersistence(rel, true);
            tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
            /* force rewrite if necessary */
-           if (tab->chgLoggedness)
+           if (tab->chgPersistence)
                tab->rewrite = true;
            pass = AT_PASS_MISC;
            break;
        case AT_SetUnLogged:    /* SET UNLOGGED */
            ATSimplePermissions(rel, ATT_TABLE);
-           tab->chgLoggedness = ATPrepChangeLoggedness(rel, false);
+           tab->chgPersistence = ATPrepChangePersistence(rel, false);
            tab->newrelpersistence = RELPERSISTENCE_UNLOGGED;
            /* force rewrite if necessary */
-           if (tab->chgLoggedness)
+           if (tab->chgPersistence)
                tab->rewrite = true;
            pass = AT_PASS_MISC;
            break;
@@ -3618,6 +3618,13 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
         * We only need to rewrite the table if at least one column needs to
         * be recomputed, we are adding/removing the OID column, or we are
         * changing its persistence.
+        *
+        * There are two reasons for requiring a rewrite when changing
+        * persistence: on one hand, we need to ensure that the buffers
+        * belonging to each of the two relations are marked with or without
+        * BM_PERMANENT properly.  On the other hand, since rewriting creates
+        * and assigns a new relfilenode, we automatically create or drop an
+        * init fork for the relation as appropriate.
         */
        if (tab->rewrite)
        {
@@ -3668,7 +3675,7 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
             * Select persistence of transient table (same as original unless
             * user requested a change)
             */
-           persistence = tab->chgLoggedness ?
+           persistence = tab->chgPersistence ?
                tab->newrelpersistence : OldHeap->rd_rel->relpersistence;
 
            heap_close(OldHeap, NoLock);
@@ -3705,8 +3712,8 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
             * because the rewrite step might read the indexes, and that would
             * cause buffers for them to have the wrong setting.
             */
-           if (tab->chgLoggedness)
-               ATChangeIndexesLoggedness(tab->relid, tab->newrelpersistence);
+           if (tab->chgPersistence)
+               ATChangeIndexesPersistence(tab->relid, tab->newrelpersistence);
 
            /*
             * Swap the physical files of the old and new heaps, then rebuild
@@ -4119,7 +4126,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
    tab->relkind = rel->rd_rel->relkind;
    tab->oldDesc = CreateTupleDescCopy(RelationGetDescr(rel));
    tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
-   tab->chgLoggedness = false;
+   tab->chgPersistence = false;
 
    *wqueue = lappend(*wqueue, tab);
 
@@ -10678,7 +10685,7 @@ ATExecGenericOptions(Relation rel, List *options)
  * checks are skipped), otherwise true.
  */
 static bool
-ATPrepChangeLoggedness(Relation rel, bool toLogged)
+ATPrepChangePersistence(Relation rel, bool toLogged)
 {
    Relation    pg_constraint;
    HeapTuple   tuple;
@@ -10722,7 +10729,7 @@ ATPrepChangeLoggedness(Relation rel, bool toLogged)
 
    /*
     * Scan conrelid if changing to permanent, else confrelid.  This also
-    * determines whether an useful index exists.
+    * determines whether a useful index exists.
     */
    ScanKeyInit(&skey[0],
                toLogged ? Anum_pg_constraint_conrelid :
@@ -10792,7 +10799,7 @@ ATPrepChangeLoggedness(Relation rel, bool toLogged)
  * given persistence.
  */
 static void
-ATChangeIndexesLoggedness(Oid relid, char relpersistence)
+ATChangeIndexesPersistence(Oid relid, char relpersistence)
 {
    Relation    rel;
    Relation    pg_class;