From da4eab64bacae0f783b69f4ec89ee0e11d1d1006 Mon Sep 17 00:00:00 2001 From: Amit Khandekar Date: Fri, 28 Sep 2018 15:01:32 +0530 Subject: [PATCH 1/2] 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. Author: Amit Langote. Reviewer: Amit Khandekar. --- 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 d06662b..7fe4be5 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 5443cbf..862615e 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1955,8 +1955,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); ExecStoreHeapTuple(tuple, slot, false); } } @@ -2034,8 +2038,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); ExecStoreHeapTuple(tuple, slot, false); } } @@ -2082,8 +2090,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); ExecStoreHeapTuple(tuple, slot, false); } } @@ -2188,8 +2200,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); ExecStoreHeapTuple(tuple, slot, false); } } diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index ec7a526..e695470 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -144,17 +144,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) { @@ -739,6 +731,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. */ @@ -831,8 +852,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); @@ -841,7 +861,6 @@ ConvertPartitionTupleSlot(TupleConversionMap *map, */ *p_my_slot = new_slot; Assert(new_slot != NULL); - ExecSetSlotDescriptor(new_slot, map->outdesc); ExecStoreHeapTuple(tuple, new_slot, shouldFree); return tuple; @@ -915,8 +934,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 bf0d5e8..9f60be6 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 89ce538..235e60f 100644 --- a/src/include/executor/execPartition.h +++ b/src/include/executor/execPartition.h @@ -58,10 +58,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 @@ -80,7 +83,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.1.4