Commit de570047 authored by Tom Lane's avatar Tom Lane

Fix some oversights in commit 2455ab48.

The idea was to generate all the junk in a destroyable subcontext rather
than leaking it in the caller's context, but partition_bounds_create was
still being called in the caller's context, allowing plenty of scope for
leakage.  Also, get_rel_relkind() was still being called in the rel's
rd_pdcxt, creating a risk of session-lifespan memory wastage.

Simplify the logic a bit while at it.  Also, reduce rd_pdcxt to
ALLOCSET_SMALL_SIZES, since it seems likely to not usually be big.

Probably something like this needs to be back-patched into v11,
but for now let's get some buildfarm testing on this.

Discussion: https://postgr.es/m/15943.1552601288@sss.pgh.pa.us
parent 61dc4078
...@@ -67,8 +67,8 @@ RelationBuildPartitionDesc(Relation rel) ...@@ -67,8 +67,8 @@ RelationBuildPartitionDesc(Relation rel)
nparts; nparts;
PartitionKey key = RelationGetPartitionKey(rel); PartitionKey key = RelationGetPartitionKey(rel);
MemoryContext oldcxt; MemoryContext oldcxt;
MemoryContext rbcontext;
int *mapping; int *mapping;
MemoryContext rbcontext = NULL;
/* /*
* While building the partition descriptor, we create various temporary * While building the partition descriptor, we create various temporary
...@@ -187,56 +187,50 @@ RelationBuildPartitionDesc(Relation rel) ...@@ -187,56 +187,50 @@ RelationBuildPartitionDesc(Relation rel)
/* Now build the actual relcache partition descriptor */ /* Now build the actual relcache partition descriptor */
rel->rd_pdcxt = AllocSetContextCreate(CacheMemoryContext, rel->rd_pdcxt = AllocSetContextCreate(CacheMemoryContext,
"partition descriptor", "partition descriptor",
ALLOCSET_DEFAULT_SIZES); ALLOCSET_SMALL_SIZES);
MemoryContextCopyAndSetIdentifier(rel->rd_pdcxt, MemoryContextCopyAndSetIdentifier(rel->rd_pdcxt,
RelationGetRelationName(rel)); RelationGetRelationName(rel));
MemoryContextSwitchTo(rel->rd_pdcxt); partdesc = (PartitionDescData *)
partdesc = (PartitionDescData *) palloc0(sizeof(PartitionDescData)); MemoryContextAllocZero(rel->rd_pdcxt, sizeof(PartitionDescData));
partdesc->nparts = nparts; partdesc->nparts = nparts;
/* oids and boundinfo are allocated below. */ /* If there are no partitions, the rest of the partdesc can stay zero */
if (nparts > 0)
MemoryContextSwitchTo(oldcxt);
if (nparts == 0)
{ {
/* We can exit early in this case. */ /* Create PartitionBoundInfo, using the temporary context. */
rel->rd_partdesc = partdesc; boundinfo = partition_bounds_create(boundspecs, nparts, key, &mapping);
/* Blow away the temporary context. */ /* Now copy all info into relcache's partdesc. */
MemoryContextDelete(rbcontext); MemoryContextSwitchTo(rel->rd_pdcxt);
return; partdesc->boundinfo = partition_bounds_copy(boundinfo, key);
} partdesc->oids = (Oid *) palloc(nparts * sizeof(Oid));
partdesc->is_leaf = (bool *) palloc(nparts * sizeof(bool));
/* First create PartitionBoundInfo */ /*
boundinfo = partition_bounds_create(boundspecs, nparts, key, &mapping); * Assign OIDs from the original array into mapped indexes of the
* result array. The order of OIDs in the former is defined by the
/* Now copy boundinfo and oids into partdesc. */ * catalog scan that retrieved them, whereas that in the latter is
oldcxt = MemoryContextSwitchTo(rel->rd_pdcxt); * defined by canonicalized representation of the partition bounds.
partdesc->boundinfo = partition_bounds_copy(boundinfo, key); *
partdesc->oids = (Oid *) palloc(partdesc->nparts * sizeof(Oid)); * Also record leaf-ness of each partition. For this we use
partdesc->is_leaf = (bool *) palloc(partdesc->nparts * sizeof(bool)); * get_rel_relkind() which may leak memory, so be sure to run it in
* the temporary context.
/* */
* Now assign OIDs from the original array into mapped indexes of the MemoryContextSwitchTo(rbcontext);
* result array. The order of OIDs in the former is defined by the for (i = 0; i < nparts; i++)
* catalog scan that retrieved them, whereas that in the latter is defined {
* by canonicalized representation of the partition bounds. int index = mapping[i];
*/
for (i = 0; i < partdesc->nparts; i++)
{
int index = mapping[i];
partdesc->oids[index] = oids[i]; partdesc->oids[index] = oids[i];
/* Record if the partition is a leaf partition */ partdesc->is_leaf[index] =
partdesc->is_leaf[index] = (get_rel_relkind(oids[i]) != RELKIND_PARTITIONED_TABLE);
(get_rel_relkind(oids[i]) != RELKIND_PARTITIONED_TABLE); }
} }
MemoryContextSwitchTo(oldcxt);
rel->rd_partdesc = partdesc; rel->rd_partdesc = partdesc;
/* Blow away the temporary context. */ /* Return to caller's context, and blow away the temporary context. */
MemoryContextSwitchTo(oldcxt);
MemoryContextDelete(rbcontext); MemoryContextDelete(rbcontext);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment