Refactor some code for ALTER TABLE SET LOGGED/UNLOGGED in tablecmds.c

Both sub-commands use the same routine to switch the relpersistence of a
relation, duplicated the same checks, and used a style inconsistent with
access methods and tablespaces.

SET LOGEED/UNLOGGED is refactored to avoid any duplication, setting the
reason why a relation rewrite happens within ATPrepChangePersistence().
This shaves some code.

Discussion: https://postgr.es/m/[email protected]

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index dac39df83aca3a10cf5f8821591901d177682177..94fc3a082b24dd5e3bda2035781f06b050723dca 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -590,7 +590,8 @@ static ObjectAddress ATExecClusterOn(Relation rel, const char *indexName,
 static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
 static void ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname);
 static void ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethodId);
-static bool ATPrepChangePersistence(Relation rel, bool toLogged);
+static void ATPrepChangePersistence(AlteredTableInfo *tab, Relation rel,
+                                   bool toLogged);
 static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                                const char *tablespacename, LOCKMODE lockmode);
 static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode);
@@ -4953,33 +4954,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
            pass = AT_PASS_MISC;
            break;
        case AT_SetLogged:      /* SET LOGGED */
-           ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_SEQUENCE);
-           if (tab->chgPersistence)
-               ereport(ERROR,
-                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                        errmsg("cannot change persistence setting twice")));
-           tab->chgPersistence = ATPrepChangePersistence(rel, true);
-           /* force rewrite if necessary; see comment in ATRewriteTables */
-           if (tab->chgPersistence)
-           {
-               tab->rewrite |= AT_REWRITE_ALTER_PERSISTENCE;
-               tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
-           }
-           pass = AT_PASS_MISC;
-           break;
        case AT_SetUnLogged:    /* SET UNLOGGED */
            ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_SEQUENCE);
            if (tab->chgPersistence)
                ereport(ERROR,
                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                         errmsg("cannot change persistence setting twice")));
-           tab->chgPersistence = ATPrepChangePersistence(rel, false);
-           /* force rewrite if necessary; see comment in ATRewriteTables */
-           if (tab->chgPersistence)
-           {
-               tab->rewrite |= AT_REWRITE_ALTER_PERSISTENCE;
-               tab->newrelpersistence = RELPERSISTENCE_UNLOGGED;
-           }
+           ATPrepChangePersistence(tab, rel, cmd->subtype == AT_SetLogged);
            pass = AT_PASS_MISC;
            break;
        case AT_DropOids:       /* SET WITHOUT OIDS */
@@ -16894,12 +16875,9 @@ ATExecSetCompression(Relation rel,
  * This verifies that we're not trying to change a temp table.  Also,
  * existing foreign key constraints are checked to avoid ending up with
  * permanent tables referencing unlogged tables.
- *
- * Return value is false if the operation is a no-op (in which case the
- * checks are skipped), otherwise true.
  */
-static bool
-ATPrepChangePersistence(Relation rel, bool toLogged)
+static void
+ATPrepChangePersistence(AlteredTableInfo *tab, Relation rel, bool toLogged)
 {
    Relation    pg_constraint;
    HeapTuple   tuple;
@@ -16923,12 +16901,12 @@ ATPrepChangePersistence(Relation rel, bool toLogged)
        case RELPERSISTENCE_PERMANENT:
            if (toLogged)
                /* nothing to do */
-               return false;
+               return;
            break;
        case RELPERSISTENCE_UNLOGGED:
            if (!toLogged)
                /* nothing to do */
-               return false;
+               return;
            break;
    }
 
@@ -17011,7 +16989,13 @@ ATPrepChangePersistence(Relation rel, bool toLogged)
 
    table_close(pg_constraint, AccessShareLock);
 
-   return true;
+   /* force rewrite if necessary; see comment in ATRewriteTables */
+   tab->rewrite |= AT_REWRITE_ALTER_PERSISTENCE;
+   if (toLogged)
+       tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
+   else
+       tab->newrelpersistence = RELPERSISTENCE_UNLOGGED;
+   tab->chgPersistence = true;
 }
 
 /*