diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 8219d05d830c99c424448d36ce4afe7bc24a22f7..67cb34028339e434e672d7033d7a936c8d26f85e 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -142,10 +142,13 @@ static int partition_bound_bsearch(PartitionKey key,
/*
* RelationBuildPartitionDesc
- * Form rel's partition descriptor
+ * Form rel's partition descriptor, and store in relcache entry
*
- * Not flushed from the cache by RelationClearRelation() unless changed because
- * of addition or removal of partition.
+ * Note: the descriptor won't be flushed from the cache by
+ * RelationClearRelation() unless it's changed because of
+ * addition or removal of a partition. Hence, code holding a lock
+ * that's sufficient to prevent that can assume that rd_partdesc
+ * won't change underneath it.
*/
void
RelationBuildPartitionDesc(Relation rel)
@@ -422,10 +425,22 @@ RelationBuildPartitionDesc(Relation rel)
(int) key->strategy);
}
- /* Now build the actual relcache partition descriptor */
+ /* Assert we aren't about to leak any old data structure */
+ Assert(rel->rd_pdcxt == NULL);
+ Assert(rel->rd_partdesc == NULL);
+
+ /*
+ * Now build the actual relcache partition descriptor. Note that the
+ * order of operations here is fairly critical. If we fail partway
+ * through this code, we won't have leaked memory because the rd_pdcxt is
+ * attached to the relcache entry immediately, so it'll be freed whenever
+ * the entry is rebuilt or destroyed. However, we don't assign to
+ * rd_partdesc until the cached data structure is fully complete and
+ * valid, so that no other code might try to use it.
+ */
rel->rd_pdcxt = AllocSetContextCreate(CacheMemoryContext,
RelationGetRelationName(rel),
- ALLOCSET_DEFAULT_SIZES);
+ ALLOCSET_SMALL_SIZES);
oldcxt = MemoryContextSwitchTo(rel->rd_pdcxt);
result = (PartitionDescData *) palloc0(sizeof(PartitionDescData));
@@ -1816,11 +1831,9 @@ get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
*
* Generate partition predicate from rel's partition bound expression
*
- * Result expression tree is stored CacheMemoryContext to ensure it survives
- * as long as the relcache entry. But we should be running in a less long-lived
- * working context. To avoid leaking cache memory if this routine fails partway
- * through, we build in working memory and then copy the completed structure
- * into cache memory.
+ * We cache a copy of the result in the relcache entry, after constructing
+ * it using the caller's context. This approach avoids leaking any data
+ * into long-lived cache contexts, especially if we fail partway through.
*/
static List *
generate_partition_qual(Relation rel)
@@ -1838,8 +1851,8 @@ generate_partition_qual(Relation rel)
/* Guard against stack overflow due to overly deep partition tree */
check_stack_depth();
- /* Quick copy */
- if (rel->rd_partcheck != NIL)
+ /* If we already cached the result, just return a copy */
+ if (rel->rd_partcheckvalid)
return copyObject(rel->rd_partcheck);
/* Grab at least an AccessShareLock on the parent table */
@@ -1882,14 +1895,35 @@ generate_partition_qual(Relation rel)
if (found_whole_row)
elog(ERROR, "unexpected whole-row reference found in partition key");
- /* Save a copy in the relcache */
- oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
- rel->rd_partcheck = copyObject(result);
- MemoryContextSwitchTo(oldcxt);
+ /* Assert that we're not leaking any old data during assignments below */
+ Assert(rel->rd_partcheckcxt == NULL);
+ Assert(rel->rd_partcheck == NIL);
+
+ /*
+ * Save a copy in the relcache. The order of these operations is fairly
+ * critical to avoid memory leaks and ensure that we don't leave a corrupt
+ * relcache entry if we fail partway through copyObject.
+ *
+ * If, as is definitely possible, the partcheck list is NIL, then we do
+ * not need to make a context to hold it.
+ */
+ if (result != NIL)
+ {
+ rel->rd_partcheckcxt = AllocSetContextCreate(CacheMemoryContext,
+ RelationGetRelationName(rel),
+ ALLOCSET_SMALL_SIZES);
+ oldcxt = MemoryContextSwitchTo(rel->rd_partcheckcxt);
+ rel->rd_partcheck = copyObject(result);
+ MemoryContextSwitchTo(oldcxt);
+ }
+ else
+ rel->rd_partcheck = NIL;
+ rel->rd_partcheckvalid = true;
/* Keep the parent locked until commit */
relation_close(parent, NoLock);
+ /* Return the working copy to the caller */
return result;
}
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index e2be8ea28c85ea4df666f0faf82818fbee99fd4a..5b9005e48b1537afaaf2ec289a83fcada6f4dc9b 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -808,11 +808,11 @@ RelationBuildRuleLock(Relation relation)
/*
* RelationBuildPartitionKey
- * Build and attach to relcache partition key data of relation
+ * Build partition key data of relation, and attach to relcache
*
* Partitioning key data is a complex structure; to avoid complicated logic to
* free individual elements whenever the relcache entry is flushed, we give it
- * its own memory context, child of CacheMemoryContext, which can easily be
+ * its own memory context, a child of CacheMemoryContext, which can easily be
* deleted on its own. To avoid leaking memory in that context in case of an
* error partway through this function, the context is initially created as a
* child of CurTransactionContext and only re-parented to CacheMemoryContext
@@ -907,6 +907,7 @@ RelationBuildPartitionKey(Relation relation)
MemoryContextSwitchTo(oldcxt);
}
+ /* Allocate assorted arrays in the partkeycxt, which we'll fill below */
oldcxt = MemoryContextSwitchTo(partkeycxt);
key->partattrs = (AttrNumber *) palloc0(key->partnatts * sizeof(AttrNumber));
key->partopfamily = (Oid *) palloc0(key->partnatts * sizeof(Oid));
@@ -914,8 +915,6 @@ RelationBuildPartitionKey(Relation relation)
key->partsupfunc = (FmgrInfo *) palloc0(key->partnatts * sizeof(FmgrInfo));
key->partcollation = (Oid *) palloc0(key->partnatts * sizeof(Oid));
-
- /* Gather type and collation info as well */
key->parttypid = (Oid *) palloc0(key->partnatts * sizeof(Oid));
key->parttypmod = (int32 *) palloc0(key->partnatts * sizeof(int32));
key->parttyplen = (int16 *) palloc0(key->partnatts * sizeof(int16));
@@ -990,6 +989,10 @@ RelationBuildPartitionKey(Relation relation)
ReleaseSysCache(tuple);
+ /* Assert that we're not leaking any old data during assignments below */
+ Assert(relation->rd_partkeycxt == NULL);
+ Assert(relation->rd_partkey == NULL);
+
/*
* Success --- reparent our context and make the relcache point to the
* newly constructed key
@@ -1314,11 +1317,15 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
}
else
{
- relation->rd_partkeycxt = NULL;
relation->rd_partkey = NULL;
+ relation->rd_partkeycxt = NULL;
relation->rd_partdesc = NULL;
relation->rd_pdcxt = NULL;
}
+ /* ... but partcheck is not loaded till asked for */
+ relation->rd_partcheck = NIL;
+ relation->rd_partcheckvalid = false;
+ relation->rd_partcheckcxt = NULL;
/*
* if it's an index, initialize index-related information
@@ -2403,8 +2410,8 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc)
MemoryContextDelete(relation->rd_partkeycxt);
if (relation->rd_pdcxt)
MemoryContextDelete(relation->rd_pdcxt);
- if (relation->rd_partcheck)
- pfree(relation->rd_partcheck);
+ if (relation->rd_partcheckcxt)
+ MemoryContextDelete(relation->rd_partcheckcxt);
if (relation->rd_fdwroutine)
pfree(relation->rd_fdwroutine);
pfree(relation);
@@ -5663,18 +5670,20 @@ load_relcache_init_file(bool shared)
* format is complex and subject to change). They must be rebuilt if
* needed by RelationCacheInitializePhase3. This is not expected to
* be a big performance hit since few system catalogs have such. Ditto
- * for RLS policy data, index expressions, predicates, exclusion info,
- * and FDW info.
+ * for RLS policy data, partition info, index expressions, predicates,
+ * exclusion info, and FDW info.
*/
rel->rd_rules = NULL;
rel->rd_rulescxt = NULL;
rel->trigdesc = NULL;
rel->rd_rsdesc = NULL;
- rel->rd_partkeycxt = NULL;
rel->rd_partkey = NULL;
- rel->rd_pdcxt = NULL;
+ rel->rd_partkeycxt = NULL;
rel->rd_partdesc = NULL;
+ rel->rd_pdcxt = NULL;
rel->rd_partcheck = NIL;
+ rel->rd_partcheckvalid = false;
+ rel->rd_partcheckcxt = NULL;
rel->rd_indexprs = NIL;
rel->rd_indpred = NIL;
rel->rd_exclops = NULL;
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 4bc61e53806b50c9f7d5f0efef7ef95daf3bbdbc..58877e7f1cf0f6d830a523497c99eae6769a91cf 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -125,9 +125,9 @@ typedef struct RelationData
List *rd_fkeylist; /* list of ForeignKeyCacheInfo (see below) */
bool rd_fkeyvalid; /* true if list has been computed */
- MemoryContext rd_partkeycxt; /* private memory cxt for the below */
+ MemoryContext rd_partkeycxt; /* private context for rd_partkey, if any */
struct PartitionKeyData *rd_partkey; /* partition key, or NULL */
- MemoryContext rd_pdcxt; /* private context for partdesc */
+ MemoryContext rd_pdcxt; /* private context for rd_partdesc, if any */
struct PartitionDescData *rd_partdesc; /* partitions, or NULL */
List *rd_partcheck; /* partition CHECK quals */
@@ -216,6 +216,10 @@ typedef struct RelationData
/* use "struct" here to avoid needing to include pgstat.h: */
struct PgStat_TableStatus *pgstat_info; /* statistics collection area */
+
+ /* placed here to avoid ABI break before v12: */
+ bool rd_partcheckvalid; /* true if list has been computed */
+ MemoryContext rd_partcheckcxt; /* private cxt for rd_partcheck, if any */
} RelationData;