commit 6789358c1422cd5ba7bc6e3d81f59655af3d6f3a Author: Amit Khandekar Date: Tue Aug 14 11:59:07 2018 +0530 Prevent SetSlotDescriptor() calls during partition tuple conversion. Author: Amit Langote. diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 9bc67ce..276a262 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,16 @@ 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 c583e02..cd43af1 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1936,12 +1936,11 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, /* a reverse map */ map = convert_tuples_by_name(old_tupdesc, tupdesc, gettext_noop("could not convert row type")); + /* Input slot might be of a partition, which has a fixed tupdesc. */ + slot = MakeTupleTableSlot(tupdesc); if (map != NULL) - { tuple = do_convert_tuple(tuple, map); - ExecSetSlotDescriptor(slot, tupdesc); - ExecStoreTuple(tuple, slot, InvalidBuffer, false); - } + ExecStoreTuple(tuple, slot, InvalidBuffer, false); } insertedCols = GetInsertedColumns(resultRelInfo, estate); @@ -2015,12 +2014,14 @@ ExecConstraints(ResultRelInfo *resultRelInfo, /* a reverse map */ map = convert_tuples_by_name(orig_tupdesc, tupdesc, gettext_noop("could not convert row type")); + /* + * Input slot might be partition-specific, which has a + * fixed tupdesc. + */ + slot = MakeTupleTableSlot(tupdesc); if (map != NULL) - { tuple = do_convert_tuple(tuple, map); - ExecSetSlotDescriptor(slot, tupdesc); - ExecStoreTuple(tuple, slot, InvalidBuffer, false); - } + ExecStoreTuple(tuple, slot, InvalidBuffer, false); } insertedCols = GetInsertedColumns(resultRelInfo, estate); @@ -2063,12 +2064,14 @@ ExecConstraints(ResultRelInfo *resultRelInfo, /* a reverse map */ map = convert_tuples_by_name(old_tupdesc, tupdesc, gettext_noop("could not convert row type")); + /* + * Input slot might be partition-specific, which has a + * fixed tupdesc. + */ + slot = MakeTupleTableSlot(tupdesc); if (map != NULL) - { tuple = do_convert_tuple(tuple, map); - ExecSetSlotDescriptor(slot, tupdesc); - ExecStoreTuple(tuple, slot, InvalidBuffer, false); - } + ExecStoreTuple(tuple, slot, InvalidBuffer, false); } insertedCols = GetInsertedColumns(resultRelInfo, estate); @@ -2169,12 +2172,14 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, /* a reverse map */ map = convert_tuples_by_name(old_tupdesc, tupdesc, gettext_noop("could not convert row type")); + /* + * Input slot might be partition-specific, which has a + * fixed tupdesc. + */ + slot = MakeTupleTableSlot(tupdesc); if (map != NULL) - { tuple = do_convert_tuple(tuple, map); - ExecSetSlotDescriptor(slot, tupdesc); - ExecStoreTuple(tuple, slot, InvalidBuffer, false); - } + ExecStoreTuple(tuple, slot, InvalidBuffer, false); } insertedCols = GetInsertedColumns(resultRelInfo, estate); diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index d13be41..03779db 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,33 @@ ExecInitRoutingInfo(ModifyTableState *mtstate, gettext_noop("could not convert row type")); /* + * Initialize an empty slot that will be used to manipulate tuples of any + * this partition's rowtype. + */ + if (proute->parent_child_tupconv_maps[partidx] != NULL) + { + Relation partrel = partRelInfo->ri_RelationDesc; + /* + * Initialized the array 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 *)); + + /* + * Now initialize the slot itself and add it to the list of allocated + * slots. + */ + proute->partition_tuple_slots[partidx] = + MakeTupleTableSlot(RelationGetDescr(partrel)); + proute->partition_tuple_slots_alloced = + lappend(proute->partition_tuple_slots_alloced, + proute->partition_tuple_slots[partidx]); + } + + /* * If the partition is a foreign table, let the FDW init itself for * routing tuples to the partition. */ @@ -801,8 +820,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 +829,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; @@ -829,6 +846,7 @@ ExecCleanupTupleRouting(ModifyTableState *mtstate, { int i; int subplan_index = 0; + ListCell *lc; /* * Remember, proute->partition_dispatch_info[0] corresponds to the root @@ -885,8 +903,12 @@ 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); + foreach(lc, proute->partition_tuple_slots_alloced) + { + TupleTableSlot *slot = lfirst(lc); + + ExecDropSingleTupleTableSlot(slot); + } } /* diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index d8d89c7..9b4cd36 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1161,12 +1161,14 @@ 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 * into the root. @@ -1703,6 +1705,7 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate, int partidx; ResultRelInfo *partrel; HeapTuple tuple; + TupleConversionMap *map; /* * Determine the target partition. If ExecFindPartition does not find a @@ -1790,11 +1793,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 f6cd842..72623d4 100644 --- a/src/include/executor/execPartition.h +++ b/src/include/executor/execPartition.h @@ -86,10 +86,15 @@ 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 + * partition_tuple_slots TupleTableSlots to be used to manipulate any * given leaf partition's rowtype after that * partition is chosen for insertion by - * tuple-routing. + * tuple-routing; contains valid entry for each + * leaf partition whose attribute numbers are + * found to differ from the root parent and NULL + * otherwise + * partition_tuple_slots_alloced List of allocated slots that are stored in + * partition_tuple_slots * 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 +113,8 @@ 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; + List *partition_tuple_slots_alloced; TupleTableSlot *root_tuple_slot; } PartitionTupleRouting;