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

Commit5f11c6e

Browse files
committed
Use the right memory context for partkey's FmgrInfo
We were using CurrentMemoryContext to put the partsupfunc fmgr_infointo, which isn't right, because we want the PartitionKey as a whole tobe in the isolated Relation->rd_partkeycxt context. This can cause acrash with user-defined support functions in the operator classes usedby partitioning keys. (Maybe this can cause problems with core-suppliedopclasses too, not sure.)This is demonstrably broken in Postgres 10, too, but the initialproposed fix runs afoul of a problem discussed back when8a0596c("Get rid of copy_partition_key") reorganized that code: namely that itis possible to jump out of RelationBuildPartitionKey because of someerror and leave a dangling memory context child of CacheMemoryContext.Also, while reviewing this I noticed that the removed-in-pg11copy_partition_key was doing something wrong, unfixed in pg10, namelydoing memcpy() on the FmgrInfo, which is bogus (should be doingfmgr_info_copy). Therefore, in branch pg10, the sane fix seems to be tobackpatch both the aforementioned8a0596c and its followupbe23432 ("Protect against hypothetical memory leaks inRelationGetPartitionKey"), so do that, then apply the fmgr_info memcxtbugfix on top.Add a test case exercising btree-based custom operator classes, whichcauses a crash prior to this fix. This is not a security problem,because in order to create an operator class you need superuserprivileges anyway.Authors: Álvaro Herrera and Amit LangoteReported and diagnosed by: Amit LangoteDiscussion:https://postgr.es/m/3041e853-b1dd-a0c6-ff21-7cc5633bffd0@lab.ntt.co.jp
1 parent08e6cda commit5f11c6e

File tree

4 files changed

+60
-80
lines changed

4 files changed

+60
-80
lines changed

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

Lines changed: 30 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,6 @@ static Relation AllocateRelationDesc(Form_pg_class relp);
266266
staticvoidRelationParseRelOptions(Relationrelation,HeapTupletuple);
267267
staticvoidRelationBuildTupleDesc(Relationrelation);
268268
staticvoidRelationBuildPartitionKey(Relationrelation);
269-
staticPartitionKeycopy_partition_key(PartitionKeyfromkey);
270269
staticRelationRelationBuildDesc(OidtargetRelId,boolinsertIt);
271270
staticvoidRelationInitPhysicalAddr(Relationrelation);
272271
staticvoidload_critical_index(Oidindexoid,Oidheapoid);
@@ -811,17 +810,16 @@ RelationBuildRuleLock(Relation relation)
811810
* RelationBuildPartitionKey
812811
*Build and attach to relcache partition key data of relation
813812
*
814-
* Partitioning key data is stored in CacheMemoryContext to ensure it survives
815-
* as long as the relcache. To avoid leaking memory in that context in case
816-
* of an error partway through this function, we build the structure in the
817-
* working context (which must be short-lived) and copy the completed
818-
* structure into the cache memory.
819-
*
820-
* Also, since the structure being created here is sufficiently complex, we
821-
* make a private child context of CacheMemoryContext for each relation that
822-
* has associated partition key information. That means no complicated logic
823-
* to free individual elements whenever the relcache entry is flushed - just
824-
* delete the context.
813+
* Partitioning key data is a complex structure; to avoid complicated logic to
814+
* free individual elements whenever the relcache entry is flushed, we give it
815+
* its own memory context, child of CacheMemoryContext, which can easily be
816+
* deleted on its own. To avoid leaking memory in that context in case of an
817+
* error partway through this function, the context is initially created as a
818+
* child of CurTransactionContext and only re-parented to CacheMemoryContext
819+
* at the end, when no further errors are possible. Also, we don't make this
820+
* context the current context except in very brief code sections, out of fear
821+
* that some of our callees allocate memory on their own which would be leaked
822+
* permanently.
825823
*/
826824
staticvoid
827825
RelationBuildPartitionKey(Relationrelation)
@@ -849,7 +847,12 @@ RelationBuildPartitionKey(Relation relation)
849847
if (!HeapTupleIsValid(tuple))
850848
return;
851849

852-
key= (PartitionKey)palloc0(sizeof(PartitionKeyData));
850+
partkeycxt=AllocSetContextCreate(CurTransactionContext,
851+
RelationGetRelationName(relation),
852+
ALLOCSET_SMALL_SIZES);
853+
854+
key= (PartitionKey)MemoryContextAllocZero(partkeycxt,
855+
sizeof(PartitionKeyData));
853856

854857
/* Fixed-length attributes */
855858
form= (Form_pg_partitioned_table)GETSTRUCT(tuple);
@@ -896,13 +899,15 @@ RelationBuildPartitionKey(Relation relation)
896899
* expressions should be in canonical form already (ie, no need for
897900
* OR-merging or constant elimination).
898901
*/
899-
expr=eval_const_expressions(NULL, (Node*)expr);
902+
expr=eval_const_expressions(NULL,expr);
903+
fix_opfuncids(expr);
900904

901-
/* May as well fix opfuncids too */
902-
fix_opfuncids((Node*)expr);
903-
key->partexprs= (List*)expr;
905+
oldcxt=MemoryContextSwitchTo(partkeycxt);
906+
key->partexprs= (List*)copyObject(expr);
907+
MemoryContextSwitchTo(oldcxt);
904908
}
905909

910+
oldcxt=MemoryContextSwitchTo(partkeycxt);
906911
key->partattrs= (AttrNumber*)palloc0(key->partnatts*sizeof(AttrNumber));
907912
key->partopfamily= (Oid*)palloc0(key->partnatts*sizeof(Oid));
908913
key->partopcintype= (Oid*)palloc0(key->partnatts*sizeof(Oid));
@@ -917,6 +922,7 @@ RelationBuildPartitionKey(Relation relation)
917922
key->parttypbyval= (bool*)palloc0(key->partnatts*sizeof(bool));
918923
key->parttypalign= (char*)palloc0(key->partnatts*sizeof(char));
919924
key->parttypcoll= (Oid*)palloc0(key->partnatts*sizeof(Oid));
925+
MemoryContextSwitchTo(oldcxt);
920926

921927
/* Copy partattrs and fill other per-attribute info */
922928
memcpy(key->partattrs,attrs,key->partnatts*sizeof(int16));
@@ -951,7 +957,7 @@ RelationBuildPartitionKey(Relation relation)
951957
BTORDER_PROC,opclassform->opcintype,opclassform->opcintype,
952958
opclassform->opcfamily);
953959

954-
fmgr_info(funcid,&key->partsupfunc[i]);
960+
fmgr_info_cxt(funcid,&key->partsupfunc[i],partkeycxt);
955961

956962
/* Collation */
957963
key->partcollation[i]=collation->values[i];
@@ -984,68 +990,13 @@ RelationBuildPartitionKey(Relation relation)
984990

985991
ReleaseSysCache(tuple);
986992

987-
/* Success --- now copy to the cache memory */
988-
partkeycxt=AllocSetContextCreate(CacheMemoryContext,
989-
RelationGetRelationName(relation),
990-
ALLOCSET_SMALL_SIZES);
993+
/*
994+
* Success --- reparent our context and make the relcache point to the
995+
* newly constructed key
996+
*/
997+
MemoryContextSetParent(partkeycxt,CacheMemoryContext);
991998
relation->rd_partkeycxt=partkeycxt;
992-
oldcxt=MemoryContextSwitchTo(relation->rd_partkeycxt);
993-
relation->rd_partkey=copy_partition_key(key);
994-
MemoryContextSwitchTo(oldcxt);
995-
}
996-
997-
/*
998-
* copy_partition_key
999-
*
1000-
* The copy is allocated in the current memory context.
1001-
*/
1002-
staticPartitionKey
1003-
copy_partition_key(PartitionKeyfromkey)
1004-
{
1005-
PartitionKeynewkey;
1006-
intn;
1007-
1008-
newkey= (PartitionKey)palloc(sizeof(PartitionKeyData));
1009-
1010-
newkey->strategy=fromkey->strategy;
1011-
newkey->partnatts=n=fromkey->partnatts;
1012-
1013-
newkey->partattrs= (AttrNumber*)palloc(n*sizeof(AttrNumber));
1014-
memcpy(newkey->partattrs,fromkey->partattrs,n*sizeof(AttrNumber));
1015-
1016-
newkey->partexprs=copyObject(fromkey->partexprs);
1017-
1018-
newkey->partopfamily= (Oid*)palloc(n*sizeof(Oid));
1019-
memcpy(newkey->partopfamily,fromkey->partopfamily,n*sizeof(Oid));
1020-
1021-
newkey->partopcintype= (Oid*)palloc(n*sizeof(Oid));
1022-
memcpy(newkey->partopcintype,fromkey->partopcintype,n*sizeof(Oid));
1023-
1024-
newkey->partsupfunc= (FmgrInfo*)palloc(n*sizeof(FmgrInfo));
1025-
memcpy(newkey->partsupfunc,fromkey->partsupfunc,n*sizeof(FmgrInfo));
1026-
1027-
newkey->partcollation= (Oid*)palloc(n*sizeof(Oid));
1028-
memcpy(newkey->partcollation,fromkey->partcollation,n*sizeof(Oid));
1029-
1030-
newkey->parttypid= (Oid*)palloc(n*sizeof(Oid));
1031-
memcpy(newkey->parttypid,fromkey->parttypid,n*sizeof(Oid));
1032-
1033-
newkey->parttypmod= (int32*)palloc(n*sizeof(int32));
1034-
memcpy(newkey->parttypmod,fromkey->parttypmod,n*sizeof(int32));
1035-
1036-
newkey->parttyplen= (int16*)palloc(n*sizeof(int16));
1037-
memcpy(newkey->parttyplen,fromkey->parttyplen,n*sizeof(int16));
1038-
1039-
newkey->parttypbyval= (bool*)palloc(n*sizeof(bool));
1040-
memcpy(newkey->parttypbyval,fromkey->parttypbyval,n*sizeof(bool));
1041-
1042-
newkey->parttypalign= (char*)palloc(n*sizeof(bool));
1043-
memcpy(newkey->parttypalign,fromkey->parttypalign,n*sizeof(char));
1044-
1045-
newkey->parttypcoll= (Oid*)palloc(n*sizeof(Oid));
1046-
memcpy(newkey->parttypcoll,fromkey->parttypcoll,n*sizeof(Oid));
1047-
1048-
returnnewkey;
999+
relation->rd_partkey=key;
10491000
}
10501001

10511002
/*

‎src/include/access/hash.h

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

291291
/*
292292
*When a new operator class is declared, we require that the user supply
293-
*us with an amprocprocudure for hashing a key of the new type.
293+
*us with an amprocprocedure for hashing a key of the new type.
294294
*Since we only have one such proc in amproc, it's number 1.
295295
*/
296296
#defineHASHPROC1

‎src/test/regress/expected/create_table.out

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,8 +764,22 @@ Partition of: range_parted4 FOR VALUES FROM (6, 8, MINVALUE) TO (9, MAXVALUE, MA
764764
Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (c IS NOT NULL) AND ((abs(a) > 6) OR ((abs(a) = 6) AND (abs(b) >= 8))) AND (abs(a) <= 9))
765765

766766
DROP TABLE range_parted4;
767+
-- user-defined operator class in partition key
768+
CREATE FUNCTION my_int4_sort(int4,int4) RETURNS int LANGUAGE sql
769+
AS $$ SELECT case WHEN $1 = $2 THEN 0 WHEN $1 > $2 THEN 1 ELSE -1 END; $$;
770+
CREATE OPERATOR CLASS test_int4_ops FOR TYPE int4 USING btree AS
771+
OPERATOR 1 < (int4,int4), OPERATOR 2 <= (int4,int4),
772+
OPERATOR 3 = (int4,int4), OPERATOR 4 >= (int4,int4),
773+
OPERATOR 5 > (int4,int4), FUNCTION 1 my_int4_sort(int4,int4);
774+
CREATE TABLE partkey_t (a int4) PARTITION BY RANGE (a test_int4_ops);
775+
CREATE TABLE partkey_t_1 PARTITION OF partkey_t FOR VALUES FROM (0) TO (1000);
776+
INSERT INTO partkey_t VALUES (100);
777+
INSERT INTO partkey_t VALUES (200);
767778
-- cleanup
768779
DROP TABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
780+
DROP TABLE partkey_t;
781+
DROP OPERATOR CLASS test_int4_ops USING btree;
782+
DROP FUNCTION my_int4_sort(int4,int4);
769783
-- comments on partitioned tables columns
770784
CREATE TABLE parted_col_comment (a int, b text) PARTITION BY LIST (a);
771785
COMMENT ON TABLE parted_col_comment IS 'Am partitioned table';

‎src/test/regress/sql/create_table.sql

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,8 +637,23 @@ CREATE TABLE range_parted4_3 PARTITION OF range_parted4 FOR VALUES FROM (6, 8, M
637637
\d+ range_parted4_3
638638
DROPTABLE range_parted4;
639639

640+
-- user-defined operator class in partition key
641+
CREATEFUNCTIONmy_int4_sort(int4,int4) RETURNSint LANGUAGE sql
642+
AS $$SELECT case WHEN $1= $2 THEN0 WHEN $1> $2 THEN1 ELSE-1 END; $$;
643+
CREATEOPERATOR CLASStest_int4_ops FOR TYPE int4 USING btreeAS
644+
OPERATOR1< (int4,int4), OPERATOR2<= (int4,int4),
645+
OPERATOR3= (int4,int4), OPERATOR4>= (int4,int4),
646+
OPERATOR5> (int4,int4), FUNCTION1 my_int4_sort(int4,int4);
647+
CREATETABLEpartkey_t (a int4) PARTITION BY RANGE (a test_int4_ops);
648+
CREATETABLEpartkey_t_1 PARTITION OF partkey_t FORVALUESFROM (0) TO (1000);
649+
INSERT INTO partkey_tVALUES (100);
650+
INSERT INTO partkey_tVALUES (200);
651+
640652
-- cleanup
641653
DROPTABLE parted, list_parted, range_parted, list_parted2, range_parted2, range_parted3;
654+
DROPTABLE partkey_t;
655+
DROPOPERATOR CLASS test_int4_ops USING btree;
656+
DROPFUNCTION my_int4_sort(int4,int4);
642657

643658
-- comments on partitioned tables columns
644659
CREATETABLEparted_col_comment (aint, btext) PARTITION BY LIST (a);

0 commit comments

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