From 6caedf3c86deb1c5bf9b3dc2c333a2ae6b83a3fc Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 14 Nov 2018 11:11:58 -0500 Subject: [PATCH 1/2] Reduce unnecessary list construction in RelationBuildPartitionDesc. The 'partoids' list which was constructed by the previous version of this code was necessarily identical to 'inhoids'. There's no point to duplicating the list, so avoid that. Instead, construct the array representation directly from the original 'inhoids' list. Also, use an array rather than a list for 'boundspecs'. We know exactly how many items we need to store, so there's really no reason to use a list. Using an array instead reduces the number of memory allocations we perform. --- src/backend/utils/cache/partcache.c | 61 ++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 35 deletions(-) diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c index 5757301d05..25c8b69f3f 100644 --- a/src/backend/utils/cache/partcache.c +++ b/src/backend/utils/cache/partcache.c @@ -260,10 +260,9 @@ RelationBuildPartitionKey(Relation relation) void RelationBuildPartitionDesc(Relation rel) { - List *inhoids, - *partoids; + List *inhoids; Oid *oids = NULL; - List *boundspecs = NIL; + PartitionBoundSpec **boundspecs = NULL; ListCell *cell; int i, nparts; @@ -286,17 +285,23 @@ RelationBuildPartitionDesc(Relation rel) /* Get partition oids from pg_inherits */ inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock); + nparts = list_length(inhoids); - /* Collect bound spec nodes in a list */ + if (nparts > 0) + { + oids = palloc(nparts * sizeof(Oid)); + boundspecs = palloc(nparts * sizeof(PartitionBoundSpec *)); + } + + /* Collect bound spec nodes for each partition */ i = 0; - partoids = NIL; foreach(cell, inhoids) { Oid inhrelid = lfirst_oid(cell); HeapTuple tuple; Datum datum; bool isnull; - Node *boundspec; + PartitionBoundSpec *boundspec; tuple = SearchSysCache1(RELOID, inhrelid); if (!HeapTupleIsValid(tuple)) @@ -307,14 +312,16 @@ RelationBuildPartitionDesc(Relation rel) &isnull); if (isnull) elog(ERROR, "null relpartbound for relation %u", inhrelid); - boundspec = (Node *) stringToNode(TextDatumGetCString(datum)); + boundspec = stringToNode(TextDatumGetCString(datum)); + if (!IsA(boundspec, PartitionBoundSpec)) + elog(ERROR, "invalid relpartbound for relation %u", inhrelid); /* * Sanity check: If the PartitionBoundSpec says this is the default * partition, its OID should correspond to whatever's stored in * pg_partitioned_table.partdefid; if not, the catalog is corrupt. */ - if (castNode(PartitionBoundSpec, boundspec)->is_default) + if (boundspec->is_default) { Oid partdefid; @@ -324,20 +331,14 @@ RelationBuildPartitionDesc(Relation rel) inhrelid, partdefid); } - boundspecs = lappend(boundspecs, boundspec); - partoids = lappend_oid(partoids, inhrelid); + oids[i] = inhrelid; + boundspecs[i] = boundspec; + ++i; ReleaseSysCache(tuple); } - nparts = list_length(partoids); - if (nparts > 0) { - oids = (Oid *) palloc(nparts * sizeof(Oid)); - i = 0; - foreach(cell, partoids) - oids[i++] = lfirst_oid(cell); - /* Convert from node to the internal representation */ if (key->strategy == PARTITION_STRATEGY_HASH) { @@ -345,11 +346,9 @@ RelationBuildPartitionDesc(Relation rel) hbounds = (PartitionHashBound **) palloc(nparts * sizeof(PartitionHashBound *)); - i = 0; - foreach(cell, boundspecs) + for (i = 0; i < nparts; ++i) { - PartitionBoundSpec *spec = castNode(PartitionBoundSpec, - lfirst(cell)); + PartitionBoundSpec *spec = boundspecs[i]; if (spec->strategy != PARTITION_STRATEGY_HASH) elog(ERROR, "invalid strategy in partition bound spec"); @@ -360,7 +359,6 @@ RelationBuildPartitionDesc(Relation rel) hbounds[i]->modulus = spec->modulus; hbounds[i]->remainder = spec->remainder; hbounds[i]->index = i; - i++; } /* Sort all the bounds in ascending order */ @@ -374,12 +372,10 @@ RelationBuildPartitionDesc(Relation rel) /* * Create a unified list of non-null values across all partitions. */ - i = 0; null_index = -1; - foreach(cell, boundspecs) + for (i = 0; i < nparts; ++i) { - PartitionBoundSpec *spec = castNode(PartitionBoundSpec, - lfirst(cell)); + PartitionBoundSpec *spec = boundspecs[i]; ListCell *c; if (spec->strategy != PARTITION_STRATEGY_LIST) @@ -393,7 +389,6 @@ RelationBuildPartitionDesc(Relation rel) if (spec->is_default) { default_index = i; - i++; continue; } @@ -425,8 +420,6 @@ RelationBuildPartitionDesc(Relation rel) non_null_values = lappend(non_null_values, list_value); } - - i++; } ndatums = list_length(non_null_values); @@ -465,11 +458,10 @@ RelationBuildPartitionDesc(Relation rel) * Create a unified list of range bounds across all the * partitions. */ - i = ndatums = 0; - foreach(cell, boundspecs) + ndatums = 0; + for (i = 0; i < nparts; ++i) { - PartitionBoundSpec *spec = castNode(PartitionBoundSpec, - lfirst(cell)); + PartitionBoundSpec *spec = boundspecs[i]; PartitionRangeBound *lower, *upper; @@ -483,7 +475,7 @@ RelationBuildPartitionDesc(Relation rel) */ if (spec->is_default) { - default_index = i++; + default_index = i; continue; } @@ -493,7 +485,6 @@ RelationBuildPartitionDesc(Relation rel) false); all_bounds[ndatums++] = lower; all_bounds[ndatums++] = upper; - i++; } Assert(ndatums == nparts * 2 || -- 2.14.3 (Apple Git-98)