From a543b15f83f5e1adaef2a46de59022b0b21711e4 Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 28 Jun 2018 15:47:47 +0900 Subject: [PATCH v2] Allocate dedicated slots of partition tuple conversion Currently there is just one slot called partition_tuple_slot and we do ExecSetSlotDescriptor every time a tuple is inserted into a partition that requires tuple conversion. That's excessively inefficient during bulk inserts. Fix that by allocating a dedicated slot for each partitions that requires it. --- src/backend/commands/copy.c | 17 +++++++++---- src/backend/executor/execMain.c | 24 +++++++++++++++--- src/backend/executor/execPartition.c | 45 +++++++++++++++++++++++----------- src/backend/executor/nodeModifyTable.c | 27 ++++++++++++-------- src/include/executor/execPartition.h | 13 ++++++---- 5 files changed, 88 insertions(+), 38 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 9bc67ce60f..f61db3e173 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2690,6 +2690,7 @@ CopyFrom(CopyState cstate) if (proute) { int leaf_part_index; + TupleConversionMap *map; /* * Away we go ... If we end up not finding a partition after all, @@ -2864,11 +2865,17 @@ CopyFrom(CopyState cstate) * partition rowtype. Don't free the already stored tuple as it * may still be required for a multi-insert batch. */ - tuple = ConvertPartitionTupleSlot(proute->parent_child_tupconv_maps[leaf_part_index], - tuple, - proute->partition_tuple_slot, - &slot, - false); + map = proute->parent_child_tupconv_maps[leaf_part_index]; + if (map != NULL) + { + TupleTableSlot *new_slot; + + Assert(proute->partition_tuple_slots != NULL && + proute->partition_tuple_slots[leaf_part_index] != NULL); + new_slot = proute->partition_tuple_slots[leaf_part_index]; + tuple = ConvertPartitionTupleSlot(map, tuple, new_slot, &slot, + false); + } tuple->t_tableOid = RelationGetRelid(resultRelInfo->ri_RelationDesc); } diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index c583e020a0..415670ed7e 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1938,8 +1938,12 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, gettext_noop("could not convert row type")); if (map != NULL) { + /* + * Partition-specific slot's tupdesc can't be changed, so allocate + * a new one. + */ + slot = MakeTupleTableSlot(tupdesc); tuple = do_convert_tuple(tuple, map); - ExecSetSlotDescriptor(slot, tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } } @@ -2017,8 +2021,12 @@ ExecConstraints(ResultRelInfo *resultRelInfo, gettext_noop("could not convert row type")); if (map != NULL) { + /* + * Partition-specific slot's tupdesc can't be changed, + * so allocate a new one. + */ + slot = MakeTupleTableSlot(tupdesc); tuple = do_convert_tuple(tuple, map); - ExecSetSlotDescriptor(slot, tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } } @@ -2065,8 +2073,12 @@ ExecConstraints(ResultRelInfo *resultRelInfo, gettext_noop("could not convert row type")); if (map != NULL) { + /* + * Partition-specific slot's tupdesc can't be changed, + * so allocate a new one. + */ + slot = MakeTupleTableSlot(tupdesc); tuple = do_convert_tuple(tuple, map); - ExecSetSlotDescriptor(slot, tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } } @@ -2171,8 +2183,12 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, gettext_noop("could not convert row type")); if (map != NULL) { + /* + * Partition-specific slot's tupdesc can't be + * changed, so allocate a new one. + */ + slot = MakeTupleTableSlot(tupdesc); tuple = do_convert_tuple(tuple, map); - ExecSetSlotDescriptor(slot, tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } } diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 1a9943c3aa..2af7599152 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -114,17 +114,9 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel) * We need an additional tuple slot for storing transient tuples that * are converted to the root table descriptor. */ - proute->root_tuple_slot = MakeTupleTableSlot(NULL); + proute->root_tuple_slot = MakeTupleTableSlot(RelationGetDescr(rel)); } - /* - * Initialize an empty slot that will be used to manipulate tuples of any - * given partition's rowtype. It is attached to the caller-specified node - * (such as ModifyTableState) and released when the node finishes - * processing. - */ - proute->partition_tuple_slot = MakeTupleTableSlot(NULL); - i = 0; foreach(cell, leaf_parts) { @@ -709,6 +701,35 @@ ExecInitRoutingInfo(ModifyTableState *mtstate, gettext_noop("could not convert row type")); /* + * If partition has different rowtype than root parent, initialize a slot + * dedicated to storing this partition's tuples. The slot is used for + * various operations that are applied to tuple after routing, such as + * checking constraints. + */ + if (proute->parent_child_tupconv_maps[partidx] != NULL) + { + Relation partrel = partRelInfo->ri_RelationDesc; + + /* + * Initialize the array in proute where these slots are stored, if not + * already done. + */ + if (proute->partition_tuple_slots == NULL) + proute->partition_tuple_slots = (TupleTableSlot **) + palloc0(proute->num_partitions * + sizeof(TupleTableSlot *)); + + /* + * Initialize the slot itself setting its descriptor to this + * partition's TupleDesc; TupleDesc reference will be released + * at the end of the command. + */ + proute->partition_tuple_slots[partidx] = + ExecInitExtraTupleSlot(estate, + RelationGetDescr(partrel)); + } + + /* * If the partition is a foreign table, let the FDW init itself for * routing tuples to the partition. */ @@ -801,8 +822,7 @@ ConvertPartitionTupleSlot(TupleConversionMap *map, TupleTableSlot **p_my_slot, bool shouldFree) { - if (!map) - return tuple; + Assert(map != NULL && new_slot != NULL); tuple = do_convert_tuple(tuple, map); @@ -811,7 +831,6 @@ ConvertPartitionTupleSlot(TupleConversionMap *map, */ *p_my_slot = new_slot; Assert(new_slot != NULL); - ExecSetSlotDescriptor(new_slot, map->outdesc); ExecStoreTuple(tuple, new_slot, InvalidBuffer, shouldFree); return tuple; @@ -885,8 +904,6 @@ ExecCleanupTupleRouting(ModifyTableState *mtstate, /* Release the standalone partition tuple descriptors, if any */ if (proute->root_tuple_slot) ExecDropSingleTupleTableSlot(proute->root_tuple_slot); - if (proute->partition_tuple_slot) - ExecDropSingleTupleTableSlot(proute->partition_tuple_slot); } /* diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index d8d89c7983..e4b2d374b4 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1161,11 +1161,12 @@ lreplace:; map_index = resultRelInfo - mtstate->resultRelInfo; Assert(map_index >= 0 && map_index < mtstate->mt_nplans); tupconv_map = tupconv_map_for_subplan(mtstate, map_index); - tuple = ConvertPartitionTupleSlot(tupconv_map, - tuple, - proute->root_tuple_slot, - &slot, - true); + if (tupconv_map != NULL) + tuple = ConvertPartitionTupleSlot(tupconv_map, + tuple, + proute->root_tuple_slot, + &slot, + true); /* * Prepare for tuple routing, making it look like we're inserting @@ -1703,6 +1704,7 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate, int partidx; ResultRelInfo *partrel; HeapTuple tuple; + TupleConversionMap *map; /* * Determine the target partition. If ExecFindPartition does not find a @@ -1790,11 +1792,16 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate, /* * Convert the tuple, if necessary. */ - ConvertPartitionTupleSlot(proute->parent_child_tupconv_maps[partidx], - tuple, - proute->partition_tuple_slot, - &slot, - true); + map = proute->parent_child_tupconv_maps[partidx]; + if (map != NULL) + { + TupleTableSlot *new_slot; + + Assert(proute->partition_tuple_slots != NULL && + proute->partition_tuple_slots[partidx] != NULL); + new_slot = proute->partition_tuple_slots[partidx]; + ConvertPartitionTupleSlot(map, tuple, new_slot, &slot, true); + } /* Initialize information needed to handle ON CONFLICT DO UPDATE. */ Assert(mtstate != NULL); diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h index f6cd842cc9..c2a2dc563e 100644 --- a/src/include/executor/execPartition.h +++ b/src/include/executor/execPartition.h @@ -86,10 +86,13 @@ typedef struct PartitionDispatchData *PartitionDispatch; * element of this array has the index into the * corresponding partition in partitions array. * num_subplan_partition_offsets Length of 'subplan_partition_offsets' array - * partition_tuple_slot TupleTableSlot to be used to manipulate any - * given leaf partition's rowtype after that - * partition is chosen for insertion by - * tuple-routing. + * partition_tuple_slots Array of TupleTableSlot objects; if non-NULL, + * contains one entry for every leaf partition, + * of which only those of the leaf partitions + * whose attribute numbers differ from the root + * parent have a non-NULL value. NULL if all of + * the partitions encountered by a given command + * happen to have same rowtype as the root parent * root_tuple_slot TupleTableSlot to be used to transiently hold * copy of a tuple that's being moved across * partitions in the root partitioned table's @@ -108,7 +111,7 @@ typedef struct PartitionTupleRouting bool *child_parent_map_not_required; int *subplan_partition_offsets; int num_subplan_partition_offsets; - TupleTableSlot *partition_tuple_slot; + TupleTableSlot **partition_tuple_slots; TupleTableSlot *root_tuple_slot; } PartitionTupleRouting; -- 2.11.0