mirror/postgresql
2
0
Fork 0
mirror of https://git.postgresql.org/git/postgresql.git synced 2025-01-30 19:00:29 +08:00
Code Issues Packages Projects Releases Wiki Activity

ALTER TABLE ADD COLUMN exhibits a significant memory leak when adding a

Browse Source 
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.
This commit is contained in:
Neil Conway 2005-02-09 23:17:26 +00:00
parent d32b3aec52
commit 3df9abd1a5
1 changed files with 32 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
src/backend/commands/tablecmds.c
View File

@ -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);
@ -2480,29 +2483,40 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
memset(values, 0, i * sizeof(Datum));
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);
}