diff options
| -rw-r--r-- | src/backend/utils/cache/partcache.c | 82 | ||||
| -rw-r--r-- | src/backend/utils/cache/relcache.c | 20 | ||||
| -rw-r--r-- | src/include/utils/rel.h | 8 |
3 files changed, 80 insertions, 30 deletions
diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c index 5757301d054..cd614eefac9 100644 --- a/src/backend/utils/cache/partcache.c +++ b/src/backend/utils/cache/partcache.c @@ -47,11 +47,11 @@ static int32 qsort_partition_rbound_cmp(const void *a, const void *b, /* * 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 @@ -150,6 +150,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)); @@ -157,8 +158,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)); @@ -241,6 +240,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 @@ -252,10 +255,13 @@ RelationBuildPartitionKey(Relation relation) /* * 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) @@ -565,11 +571,24 @@ 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, "partition descriptor", - ALLOCSET_DEFAULT_SIZES); - MemoryContextCopyAndSetIdentifier(rel->rd_pdcxt, RelationGetRelationName(rel)); + ALLOCSET_SMALL_SIZES); + MemoryContextCopyAndSetIdentifier(rel->rd_pdcxt, + RelationGetRelationName(rel)); oldcxt = MemoryContextSwitchTo(rel->rd_pdcxt); @@ -839,11 +858,9 @@ get_partition_qual_relid(Oid relid) * Generate partition predicate from rel's partition bound expression. The * function returns a NIL list if there is no predicate. * - * 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) @@ -860,8 +877,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 */ @@ -907,14 +924,37 @@ 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, + "partition constraint", + ALLOCSET_SMALL_SIZES); + MemoryContextCopyAndSetIdentifier(rel->rd_partcheckcxt, + RelationGetRelationName(rel)); + 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 743d5ea61a4..a9983793762 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1191,11 +1191,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 @@ -2285,8 +2289,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); @@ -5617,18 +5621,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 84469f57151..a989a12c120 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -95,9 +95,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 */ @@ -188,6 +188,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; |