Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitbe23432

Browse files
committed
Protect against hypothetical memory leaks in RelationGetPartitionKey
Also, fix a comment that commit8a0596c made obsolete.Reported-by: Robert HaasDiscussion:http://postgr.es/m/CA+TgmoYbpuUUUp2GhYNwWm0qkah39spiU7uOiNXLz20ASfKYoA@mail.gmail.com
1 parentb726eaa commitbe23432

File tree

2 files changed

+30
-25
lines changed

2 files changed

+30
-25
lines changed

‎src/backend/utils/cache/relcache.c

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -807,17 +807,16 @@ RelationBuildRuleLock(Relation relation)
807807
* RelationBuildPartitionKey
808808
*Build and attach to relcache partition key data of relation
809809
*
810-
* Partitioning key data is stored in CacheMemoryContext to ensure it survives
811-
* as long as the relcache. To avoid leaking memory in that context in case
812-
* of an error partway through this function, we build the structure in the
813-
* working context (which must be short-lived) and copy the completed
814-
* structure into the cache memory.
815-
*
816-
* Also, since the structure being created here is sufficiently complex, we
817-
* make a private child context of CacheMemoryContext for each relation that
818-
* has associated partition key information. That means no complicated logic
819-
* to free individual elements whenever the relcache entry is flushed - just
820-
* delete the context.
810+
* Partitioning key data is a complex structure; to avoid complicated logic to
811+
* free individual elements whenever the relcache entry is flushed, we give it
812+
* its own memory context, child of CacheMemoryContext, which can easily be
813+
* deleted on its own. To avoid leaking memory in that context in case of an
814+
* error partway through this function, the context is initially created as a
815+
* child of CurTransactionContext and only re-parented to CacheMemoryContext
816+
* at the end, when no further errors are possible. Also, we don't make this
817+
* context the current context except in very brief code sections, out of fear
818+
* that some of our callees allocate memory on their own which would be leaked
819+
* permanently.
821820
*/
822821
staticvoid
823822
RelationBuildPartitionKey(Relationrelation)
@@ -850,9 +849,9 @@ RelationBuildPartitionKey(Relation relation)
850849
RelationGetRelationName(relation),
851850
MEMCONTEXT_COPY_NAME,
852851
ALLOCSET_SMALL_SIZES);
853-
oldcxt=MemoryContextSwitchTo(partkeycxt);
854852

855-
key= (PartitionKey)palloc0(sizeof(PartitionKeyData));
853+
key= (PartitionKey)MemoryContextAllocZero(partkeycxt,
854+
sizeof(PartitionKeyData));
856855

857856
/* Fixed-length attributes */
858857
form= (Form_pg_partitioned_table)GETSTRUCT(tuple);
@@ -894,17 +893,20 @@ RelationBuildPartitionKey(Relation relation)
894893
/*
895894
* Run the expressions through const-simplification since the planner
896895
* will be comparing them to similarly-processed qual clause operands,
897-
* and may fail to detect valid matches without this step. We don't
898-
* need to bother with canonicalize_qual() though, because partition
899-
* expressions are not full-fledged qualification clauses.
896+
* and may fail to detect valid matches without this step; fix
897+
* opfuncids while at it. We don't need to bother with
898+
* canonicalize_qual() though, because partition expressions are not
899+
* full-fledged qualification clauses.
900900
*/
901-
expr=eval_const_expressions(NULL, (Node*)expr);
901+
expr=eval_const_expressions(NULL,expr);
902+
fix_opfuncids(expr);
902903

903-
/* May as well fix opfuncids too */
904-
fix_opfuncids((Node*)expr);
905-
key->partexprs= (List*)expr;
904+
oldcxt=MemoryContextSwitchTo(partkeycxt);
905+
key->partexprs= (List*)copyObject(expr);
906+
MemoryContextSwitchTo(oldcxt);
906907
}
907908

909+
oldcxt=MemoryContextSwitchTo(partkeycxt);
908910
key->partattrs= (AttrNumber*)palloc0(key->partnatts*sizeof(AttrNumber));
909911
key->partopfamily= (Oid*)palloc0(key->partnatts*sizeof(Oid));
910912
key->partopcintype= (Oid*)palloc0(key->partnatts*sizeof(Oid));
@@ -919,8 +921,9 @@ RelationBuildPartitionKey(Relation relation)
919921
key->parttypbyval= (bool*)palloc0(key->partnatts*sizeof(bool));
920922
key->parttypalign= (char*)palloc0(key->partnatts*sizeof(char));
921923
key->parttypcoll= (Oid*)palloc0(key->partnatts*sizeof(Oid));
924+
MemoryContextSwitchTo(oldcxt);
922925

923-
/*For the hash partitioning, an extended hashfunctionwill be used. */
926+
/*determine supportfunctionnumber to search for */
924927
procnum= (key->strategy==PARTITION_STRATEGY_HASH) ?
925928
HASHEXTENDED_PROC :BTORDER_PROC;
926929

@@ -952,7 +955,7 @@ RelationBuildPartitionKey(Relation relation)
952955
if (!OidIsValid(funcid))
953956
ereport(ERROR,
954957
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
955-
errmsg("operator class \"%s\" of access method %s is missing support function %d fordatatype\"%s\"",
958+
errmsg("operator class \"%s\" of access method %s is missing support function %d for type%s",
956959
NameStr(opclassform->opcname),
957960
(key->strategy==PARTITION_STRATEGY_HASH) ?
958961
"hash" :"btree",
@@ -989,11 +992,13 @@ RelationBuildPartitionKey(Relation relation)
989992

990993
ReleaseSysCache(tuple);
991994

992-
/* Success --- make the relcache point to the newly constructed key */
995+
/*
996+
* Success --- reparent our context and make the relcache point to the
997+
* newly constructed key
998+
*/
993999
MemoryContextSetParent(partkeycxt,CacheMemoryContext);
9941000
relation->rd_partkeycxt=partkeycxt;
9951001
relation->rd_partkey=key;
996-
MemoryContextSwitchTo(oldcxt);
9971002
}
9981003

9991004
/*

‎src/include/access/hash.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ typedef HashMetaPageData *HashMetaPage;
338338

339339
/*
340340
* When a new operator class is declared, we require that the user supply
341-
* us with an amprocprocudure for hashing a key of the new type, returning
341+
* us with an amprocprocedure for hashing a key of the new type, returning
342342
* a 32-bit hash value. We call this the "standard" hash procedure. We
343343
* also allow an optional "extended" hash procedure which accepts a salt and
344344
* returns a 64-bit hash value. This is highly recommended but, for reasons

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp