From d31e2a495b6f2127afc31b4da2e5f4e89aa2cdfe Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Sat, 12 Feb 2011 08:27:55 -0500
Subject: [PATCH] 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
---
doc/src/sgml/ref/alter_table.sgml | 16 +++---
src/backend/catalog/index.c | 5 ++
src/backend/commands/tablecmds.c | 90 +++++++++++++++++++++++--------
3 files changed, 83 insertions(+), 28 deletions(-)
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 9f02674f44f..7e6e72f008e 100644
--- 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 9e6012125fc..452ced6644e 100644
--- 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 e4f352c6c7c..1db42d044ac 100644
--- 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)
--
GitLab