Teach ALTER TABLE .. SET DATA TYPE to avoid some table rewrites.

When the old type is binary coercible to the new type and the using
clause does not change the column contents, we can avoid a full table
rewrite, though any indexes on the affected columns will still need
to be rebuilt.  This applies, for example, when changing a varchar
column to be of type text.

The prior coding assumed that the set of operations that force a
rewrite is identical to the set of operations that must be propagated
to tables making use of the affected table's rowtype.  This is
no longer true: even though the tuples in those tables wouldn't
need to be modified, the data type change invalidate indexes built
using those composite type columns.  Indexes on the table we're
actually modifying can be invalidated too, of course, but the
existing machinery is sufficient to handle that case.

Along the way, add some debugging messages that make it possible
to understand what operations ALTER TABLE is actually performing
in these cases.

Noah Misch and Robert Haas

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 9f02674f44f4a6231acadf36e7ad81383a27d4f2..7e6e72f008e0d58b3bf54230daae07c6029060ad 100644 (file)
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -766,9 +766,13 @@ ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
    <para>
     Adding a column with a non-null default or changing the type of an
     existing column will require the entire table and indexes to be rewritten.
-    This might take a significant amount of time for a large table; and it will
-    temporarily require double the disk space.  Adding or removing a system
-    <literal>oid</> column likewise requires rewriting the entire table.
+    As an exception, if the old type type is binary coercible to the new
+    type and the <literal>USING</> clause does not change the column contents,
+    a table rewrite is not needed, but any indexes on the affected columns 
+    must still be rebuilt.  Adding or removing a system <literal>oid</> column
+    also requires rewriting the entire table.  Table and/or index rebuilds may
+    take a significant amount of time for a large table; and will temporarily
+    require as much as double the disk space.
    </para>
 
    <para>
@@ -797,9 +801,9 @@ ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
    <para>
     To force an immediate rewrite of the table, you can use
     <link linkend="SQL-VACUUM">VACUUM FULL</>, <xref linkend="SQL-CLUSTER">
-    or one of the forms of ALTER TABLE that forces a rewrite, such as
-    SET DATA TYPE.  This results in no semantically-visible change in the
-    table, but gets rid of no-longer-useful data.
+    or one of the forms of ALTER TABLE that forces a rewrite.  This results in
+    no semantically-visible change in the table, but gets rid of
+    no-longer-useful data.
    </para>
 
    <para>

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 9e6012125fc46feb1099fbb2417fe36fe7152acb..452ced6644eac17f71f3b8af1cb436b0b4a22773 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1685,6 +1685,11 @@ index_build(Relation heapRelation,
    procedure = indexRelation->rd_am->ambuild;
    Assert(RegProcedureIsValid(procedure));
 
+   ereport(DEBUG1,
+           (errmsg("building index \"%s\" on table \"%s\"",
+                   RelationGetRelationName(indexRelation),
+                   RelationGetRelationName(heapRelation))));
+
    /*
     * Switch to the table owner's userid, so that any index functions are run
     * as that user.  Also lock down security-restricted operations and

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e4f352c6c7c6fb9927a43e01955cfd1fe5371bc7..1db42d044ac1f3134d95f788c111d572dea28c8d 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -141,7 +141,7 @@ typedef struct AlteredTableInfo
    List       *constraints;    /* List of NewConstraint */
    List       *newvals;        /* List of NewColumnValue */
    bool        new_notnull;    /* T if we added new NOT NULL constraints */
-   bool        new_changeoids; /* T if we added/dropped the OID column */
+   bool        rewrite;        /* T if we a rewrite is forced */
    Oid         newTableSpace;  /* new tablespace; 0 means no change */
    /* Objects to rebuild after completing ALTER TYPE operations */
    List       *changedConstraintOids;  /* OIDs of constraints to rebuild */
@@ -336,6 +336,7 @@ static void ATPrepAlterColumnType(List **wqueue,
                      AlteredTableInfo *tab, Relation rel,
                      bool recurse, bool recursing,
                      AlterTableCmd *cmd, LOCKMODE lockmode);
+static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno);
 static void ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
                      const char *colName, TypeName *typeName, LOCKMODE lockmode);
 static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode);
@@ -3218,11 +3219,34 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
        if (tab->relkind == RELKIND_FOREIGN_TABLE)
            continue;
 
+       /*
+        * If we change column data types or add/remove OIDs, the operation
+        * has to be propagated to tables that use this table's rowtype as a
+        * column type.  tab->newvals will also be non-NULL in the case where
+        * we're adding a column with a default.  We choose to forbid that
+        * case as well, since composite types might eventually support
+        * defaults.
+        *
+        * (Eventually we'll probably need to check for composite type
+        * dependencies even when we're just scanning the table without a
+        * rewrite, but at the moment a composite type does not enforce any
+        * constraints, so it's not necessary/appropriate to enforce them
+        * just during ALTER.)
+        */
+       if (tab->newvals != NIL || tab->rewrite)
+       {
+           Relation    rel;
+
+           rel = heap_open(tab->relid, NoLock);
+           find_composite_type_dependencies(rel->rd_rel->reltype, rel, NULL);
+           heap_close(rel, NoLock);
+       }
+
        /*
         * We only need to rewrite the table if at least one column needs to
         * be recomputed, or we are adding/removing the OID column.
         */
-       if (tab->newvals != NIL || tab->new_changeoids)
+       if (tab->rewrite)
        {
            /* Build a temporary relation and copy data */
            Relation    OldHeap;
@@ -3408,22 +3432,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
        hi_options = 0;
    }
 
-   /*
-    * If we change column data types or add/remove OIDs, the operation has to
-    * be propagated to tables that use this table's rowtype as a column type.
-    * newrel will also be non-NULL in the case where we're adding a column
-    * with a default.  We choose to forbid that case as well, since composite
-    * types might eventually support defaults.
-    *
-    * (Eventually we'll probably need to check for composite type
-    * dependencies even when we're just scanning the table without a rewrite,
-    * but at the moment a composite type does not enforce any constraints,
-    * so it's not necessary/appropriate to enforce them just during ALTER.)
-    */
-   if (newrel)
-       find_composite_type_dependencies(oldrel->rd_rel->reltype,
-                                        oldrel, NULL);
-
    /*
     * Generate the constraint and default execution states
     */
@@ -3490,6 +3498,15 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
        List       *dropped_attrs = NIL;
        ListCell   *lc;
 
+       if (newrel)
+           ereport(DEBUG1,
+                   (errmsg("rewriting table \"%s\"",
+                           RelationGetRelationName(oldrel))));
+       else
+           ereport(DEBUG1,
+                   (errmsg("verifying table \"%s\"",
+                           RelationGetRelationName(oldrel))));
+
        econtext = GetPerTupleExprContext(estate);
 
        /*
@@ -3532,7 +3549,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
 
        while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
        {
-           if (newrel)
+           if (tab->rewrite)
            {
                Oid         tupOid = InvalidOid;
 
@@ -4330,6 +4347,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
            newval->expr = defval;
 
            tab->newvals = lappend(tab->newvals, newval);
+           tab->rewrite = true;
        }
 
        /*
@@ -4346,7 +4364,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
     * table to fix that.
     */
    if (isOid)
-       tab->new_changeoids = true;
+       tab->rewrite = true;
 
    /*
     * Add needed dependency entries for the new column.
@@ -5019,7 +5037,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
        tab = ATGetQueueEntry(wqueue, rel);
 
        /* Tell Phase 3 to physically remove the OID column */
-       tab->new_changeoids = true;
+       tab->rewrite = true;
    }
 }
 
@@ -5043,7 +5061,7 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
    /* suppress schema rights check when rebuilding existing index */
    check_rights = !is_rebuild;
    /* skip index build if phase 3 will have to rewrite table anyway */
-   skip_build = (tab->newvals != NIL);
+   skip_build = tab->rewrite;
    /* suppress notices when rebuilding existing index */
    quiet = is_rebuild;
 
@@ -6002,6 +6020,9 @@ validateForeignKeyConstraint(char *conname,
    HeapTuple   tuple;
    Trigger     trig;
 
+   ereport(DEBUG1,
+           (errmsg("validating foreign key constraint \"%s\"", conname)));
+
    /*
     * Build a trigger call structure; we'll need it either way.
     */
@@ -6560,6 +6581,8 @@ ATPrepAlterColumnType(List **wqueue,
        newval->expr = (Expr *) transform;
 
        tab->newvals = lappend(tab->newvals, newval);
+       if (ATColumnChangeRequiresRewrite(transform, attnum))
+           tab->rewrite = true;
    }
    else if (tab->relkind == RELKIND_FOREIGN_TABLE)
    {
@@ -6599,6 +6622,29 @@ ATPrepAlterColumnType(List **wqueue,
        ATTypedTableRecursion(wqueue, rel, cmd, lockmode);
 }
 
+/*
+ * When the data type of a column is changed, a rewrite might not be require
+ * if the data type is being changed to its current type, or more interestingly
+ * to a type to which it is binary coercible.  But we must check carefully that
+ * the USING clause isn't trying to insert some other value.
+ */
+static bool
+ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno)
+{
+   Assert(expr != NULL);
+
+   for (;;)
+   {
+       /* only one varno, so no need to check that */
+       if (IsA(expr, Var) && ((Var *) expr)->varattno == varattno)
+           return false;
+       else if (IsA(expr, RelabelType))
+           expr = (Node *) ((RelabelType *) expr)->arg;
+       else
+           return true;
+   }
+}
+
 static void
 ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
                      const char *colName, TypeName *typeName, LOCKMODE lockmode)