aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorNeil Conway <neilc@samurai.com>2005-02-09 23:17:26 +0000
committerNeil Conway <neilc@samurai.com>2005-02-09 23:17:26 +0000
commit3df9abd1a5e90b4d32137065373cbb82e64e865c (patch)
treeb90c598af7e2f1f55ae7b69c6f7780a80aa63a3d /src
parentd32b3aec52ed2284b7547927a161de0ca2c2ea72 (diff)
downloadpostgresql-3df9abd1a5e90b4d32137065373cbb82e64e865c.tar.gz
postgresql-3df9abd1a5e90b4d32137065373cbb82e64e865c.zip
ALTER TABLE ADD COLUMN exhibits a significant memory leak when adding a
column with a default expression. In that situation, we need to rewrite the heap relation. To evaluate the new default expression, we use ExecEvalExpr(); however, this can allocate memory in the current memory context, and ATRewriteTable() does not switch out of the active portal's heap memory context. The end result is a rather large memory leak (on the order of gigabytes for a reasonably sized table). This patch changes ATRewriteTable() to switch to the per-tuple memory context before beginning the per-tuple loop. It also removes an explicit heap_freetuple() in the loop, since that is no longer needed. In an unrelated change, I noticed the code was scanning through the attributes of the new tuple descriptor for each tuple of the old table. I changed this to use precomputation, which should slightly speed up the loop. Thanks to steve@deefs.net for reporting the leak.
Diffstat (limited to 'src')
-rw-r--r--src/backend/commands/tablecmds.c48
1 files changed, 32 insertions, 16 deletions
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4f96902beab..fddfd13c716 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.145 2005/01/27 23:23:55 neilc Exp $
+ * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.146 2005/02/09 23:17:26 neilc Exp $
*
*-------------------------------------------------------------------------
*/
@@ -2460,6 +2460,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
TupleTableSlot *newslot;
HeapScanDesc scan;
HeapTuple tuple;
+ MemoryContext oldCxt;
+ List *dropped_attrs = NIL;
+ ListCell *lc;
econtext = GetPerTupleExprContext(estate);
@@ -2481,28 +2484,39 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
memset(nulls, 'n', i * sizeof(char));
/*
+ * Any attributes that are dropped according to the new tuple
+ * descriptor can be set to NULL. We precompute the list of
+ * dropped attributes to avoid needing to do so in the
+ * per-tuple loop.
+ */
+ for (i = 0; i < newTupDesc->natts; i++)
+ {
+ if (newTupDesc->attrs[i]->attisdropped)
+ dropped_attrs = lappend_int(dropped_attrs, i);
+ }
+
+ /*
* Scan through the rows, generating a new row if needed and then
* checking all the constraints.
*/
scan = heap_beginscan(oldrel, SnapshotNow, 0, NULL);
+ /*
+ * Switch to per-tuple memory context and reset it for each
+ * tuple produced, so we don't leak memory.
+ */
+ oldCxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
+
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
if (newrel)
{
- /*
- * Extract data from old tuple. We can force to null any
- * columns that are deleted according to the new tuple.
- */
- int natts = newTupDesc->natts;
-
+ /* Extract data from old tuple */
heap_deformtuple(tuple, oldTupDesc, values, nulls);
- for (i = 0; i < natts; i++)
- {
- if (newTupDesc->attrs[i]->attisdropped)
- nulls[i] = 'n';
- }
+ /* Set dropped attributes to null in new tuple */
+ foreach (lc, dropped_attrs)
+ nulls[lfirst_int(lc)] = 'n';
/*
* Process supplied expressions to replace selected
@@ -2526,6 +2540,11 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
nulls[ex->attnum - 1] = ' ';
}
+ /*
+ * Form the new tuple. Note that we don't explicitly
+ * pfree it, since the per-tuple memory context will
+ * be reset shortly.
+ */
tuple = heap_formtuple(newTupDesc, values, nulls);
}
@@ -2572,17 +2591,14 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
/* Write the tuple out to the new relation */
if (newrel)
- {
simple_heap_insert(newrel, tuple);
- heap_freetuple(tuple);
- }
-
ResetExprContext(econtext);
CHECK_FOR_INTERRUPTS();
}
+ MemoryContextSwitchTo(oldCxt);
heap_endscan(scan);
}