From 1097d60c6b94b07f5c1d28a70b545b27c373a18c Mon Sep 17 00:00:00 2001 From: Amit Khandekar Date: Fri, 28 Sep 2018 15:02:57 +0530 Subject: [PATCH 2/2] Slotify partition tuple conversion. Partition tuples are needlessly formed and deformed during tuple conversion (do_convert_tuple), when the same operation can be done using tuple slots. This is because the input slot might already have a deformed tuple. So convert directly using input and output tuple slots. Have a new function ConvertTupleSlot() that accepts a map of AttrNumber[], rather than TupleConversionMap. ConvertPartitionTupleSlot() is exclusively used for conversion of partition-to-parent and vice versa, so get rid of this. do_convert_tuple() is used for tuples belonging to non-partition tables as well, so leave such calls untouched. May be we can "slotify" these tuple conversions in later commits. Changed the PartitionDispatch->tupmap type from TupleConversionMap to AttrNumber[] because we now don't require the input/output descriptors since they are available in the input/output slots. Author: Amit Khandekar. Reviewer: Amit Langote. --- src/backend/access/common/tupconvert.c | 113 ++++++++++++++++++++++++++++----- src/backend/commands/copy.c | 16 +++-- src/backend/executor/execMain.c | 87 ++++++++++--------------- src/backend/executor/execPartition.c | 58 ++--------------- src/backend/executor/nodeModifyTable.c | 9 +-- src/include/access/tupconvert.h | 8 +++ src/include/executor/execPartition.h | 5 -- 7 files changed, 161 insertions(+), 135 deletions(-) diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c index 3bc67b8..2100d40 100644 --- a/src/backend/access/common/tupconvert.c +++ b/src/backend/access/common/tupconvert.c @@ -22,6 +22,7 @@ #include "access/htup_details.h" #include "access/tupconvert.h" +#include "executor/tuptable.h" #include "utils/builtins.h" @@ -214,6 +215,45 @@ convert_tuples_by_name(TupleDesc indesc, TupleConversionMap *map; AttrNumber *attrMap; int n = outdesc->natts; + + /* Verify compatibility and prepare attribute-number map */ + attrMap = inheritence_tupconv_map(indesc, outdesc, msg); + + if (attrMap == NULL) + { + /* Runtime conversion is not needed */ + return NULL; + } + + /* Prepare the map structure */ + map = (TupleConversionMap *) palloc(sizeof(TupleConversionMap)); + map->indesc = indesc; + map->outdesc = outdesc; + map->attrMap = attrMap; + /* preallocate workspace for Datum arrays */ + map->outvalues = (Datum *) palloc(n * sizeof(Datum)); + map->outisnull = (bool *) palloc(n * sizeof(bool)); + n = indesc->natts + 1; /* +1 for NULL */ + map->invalues = (Datum *) palloc(n * sizeof(Datum)); + map->inisnull = (bool *) palloc(n * sizeof(bool)); + map->invalues[0] = (Datum) 0; /* set up the NULL entry */ + map->inisnull[0] = true; + + return map; +} + +/* + * Returns a bare attribute map for tuple conversion. Returns NULL if + * conversion not required. This is a convenience routine for + * convert_tuples_by_name() and other functions. + */ +AttrNumber * +inheritence_tupconv_map(TupleDesc indesc, + TupleDesc outdesc, + const char *msg) +{ + AttrNumber *attrMap; + int n = outdesc->natts; int i; bool same; @@ -265,22 +305,8 @@ convert_tuples_by_name(TupleDesc indesc, pfree(attrMap); return NULL; } - - /* Prepare the map structure */ - map = (TupleConversionMap *) palloc(sizeof(TupleConversionMap)); - map->indesc = indesc; - map->outdesc = outdesc; - map->attrMap = attrMap; - /* preallocate workspace for Datum arrays */ - map->outvalues = (Datum *) palloc(n * sizeof(Datum)); - map->outisnull = (bool *) palloc(n * sizeof(bool)); - n = indesc->natts + 1; /* +1 for NULL */ - map->invalues = (Datum *) palloc(n * sizeof(Datum)); - map->inisnull = (bool *) palloc(n * sizeof(bool)); - map->invalues[0] = (Datum) 0; /* set up the NULL entry */ - map->inisnull[0] = true; - - return map; + else + return attrMap; } /* @@ -406,6 +432,61 @@ do_convert_tuple(HeapTuple tuple, TupleConversionMap *map) } /* + * Perform conversion of a tuple slot according to the map. + */ +TupleTableSlot * +ConvertTupleSlot(AttrNumber *attrMap, + TupleTableSlot *in_slot, TupleTableSlot *out_slot) +{ + Datum *invalues; + bool *inisnull; + Datum *outvalues; + bool *outisnull; + int outnatts; + int i; + + /* Sanity checks */ + Assert(in_slot->tts_tupleDescriptor != NULL && + out_slot->tts_tupleDescriptor != NULL); + Assert(in_slot->tts_values != NULL && out_slot->tts_values != NULL); + + outnatts = out_slot->tts_tupleDescriptor->natts; + + /* Extract all the values of the in slot. */ + slot_getallattrs(in_slot); + + /* Before doing the mapping, clear any old contents from the out slot */ + ExecClearTuple(out_slot); + + invalues = in_slot->tts_values; + inisnull = in_slot->tts_isnull; + outvalues = out_slot->tts_values; + outisnull = out_slot->tts_isnull; + + /* Transpose into proper fields of the out slot. */ + for (i = 0; i < outnatts; i++) + { + int j = attrMap[i] - 1; + + /* attrMap[i] == 0 means it's a NULL datum. */ + if (j == -1) + { + outvalues[i] = (Datum) 0; + outisnull[i] = true; + } + else + { + outvalues[i] = invalues[j]; + outisnull[i] = inisnull[j]; + } + } + + ExecStoreVirtualTuple(out_slot); + + return out_slot; +} + +/* * Free a TupleConversionMap structure. */ void diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 7fe4be5..2b370c3 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -31,6 +31,7 @@ #include "commands/trigger.h" #include "executor/execPartition.h" #include "executor/executor.h" +#include "executor/tuptable.h" #include "foreign/fdwapi.h" #include "libpq/libpq.h" #include "libpq/pqformat.h" @@ -2862,19 +2863,26 @@ CopyFrom(CopyState cstate) /* * We might need to convert from the parent rowtype to the - * partition rowtype. Don't free the already stored tuple as it - * may still be required for a multi-insert batch. + * partition rowtype. */ map = proute->parent_child_tupconv_maps[leaf_part_index]; if (map != NULL) { TupleTableSlot *new_slot; + MemoryContext oldcontext; 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); + slot = ConvertTupleSlot(map->attrMap, slot, new_slot); + + /* + * Get the tuple in the per-tuple context, so that it will be + * freed after each batch insert. + */ + oldcontext = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate)); + tuple = ExecCopySlotTuple(slot); + MemoryContextSwitchTo(oldcontext); } tuple->t_tableOid = RelationGetRelid(resultRelInfo->ri_RelationDesc); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 862615e..bc7cb91 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1944,25 +1944,20 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, */ if (resultRelInfo->ri_PartitionRoot) { - HeapTuple tuple = ExecFetchSlotTuple(slot); TupleDesc old_tupdesc = RelationGetDescr(rel); - TupleConversionMap *map; + AttrNumber *map; rel = resultRelInfo->ri_PartitionRoot; tupdesc = RelationGetDescr(rel); /* a reverse map */ - map = convert_tuples_by_name(old_tupdesc, tupdesc, - gettext_noop("could not convert row type")); + map = inheritence_tupconv_map(old_tupdesc, tupdesc, + gettext_noop("could not convert row type")); + /* + * Partition-specific slot's tupdesc can't be changed, so allocate a + * new one. + */ 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); - ExecStoreHeapTuple(tuple, slot, false); - } + slot = ConvertTupleSlot(map, slot, MakeTupleTableSlot(tupdesc)); } insertedCols = GetInsertedColumns(resultRelInfo, estate); @@ -2028,24 +2023,20 @@ ExecConstraints(ResultRelInfo *resultRelInfo, */ if (resultRelInfo->ri_PartitionRoot) { - HeapTuple tuple = ExecFetchSlotTuple(slot); - TupleConversionMap *map; + AttrNumber *map; rel = resultRelInfo->ri_PartitionRoot; tupdesc = RelationGetDescr(rel); /* a reverse map */ - map = convert_tuples_by_name(orig_tupdesc, tupdesc, - gettext_noop("could not convert row type")); + map = inheritence_tupconv_map(orig_tupdesc, tupdesc, + gettext_noop("could not convert row type")); + /* + * Partition-specific slot's tupdesc can't be changed, so + * allocate a new one. + */ 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); - ExecStoreHeapTuple(tuple, slot, false); - } + slot = ConvertTupleSlot(map, slot, + MakeTupleTableSlot(tupdesc)); } insertedCols = GetInsertedColumns(resultRelInfo, estate); @@ -2079,25 +2070,21 @@ ExecConstraints(ResultRelInfo *resultRelInfo, /* See the comment above. */ if (resultRelInfo->ri_PartitionRoot) { - HeapTuple tuple = ExecFetchSlotTuple(slot); TupleDesc old_tupdesc = RelationGetDescr(rel); - TupleConversionMap *map; + AttrNumber *map; rel = resultRelInfo->ri_PartitionRoot; tupdesc = RelationGetDescr(rel); /* a reverse map */ - map = convert_tuples_by_name(old_tupdesc, tupdesc, - gettext_noop("could not convert row type")); + map = inheritence_tupconv_map(old_tupdesc, tupdesc, + gettext_noop("could not convert row type")); + /* + * Partition-specific slot's tupdesc can't be changed, so + * allocate a new one. + */ 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); - ExecStoreHeapTuple(tuple, slot, false); - } + slot = ConvertTupleSlot(map, slot, + MakeTupleTableSlot(tupdesc)); } insertedCols = GetInsertedColumns(resultRelInfo, estate); @@ -2189,25 +2176,21 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, /* See the comment in ExecConstraints(). */ if (resultRelInfo->ri_PartitionRoot) { - HeapTuple tuple = ExecFetchSlotTuple(slot); TupleDesc old_tupdesc = RelationGetDescr(rel); - TupleConversionMap *map; + AttrNumber *map; rel = resultRelInfo->ri_PartitionRoot; tupdesc = RelationGetDescr(rel); /* a reverse map */ - map = convert_tuples_by_name(old_tupdesc, tupdesc, - gettext_noop("could not convert row type")); + map = inheritence_tupconv_map(old_tupdesc, tupdesc, + gettext_noop("could not convert row type")); + /* + * Partition-specific slot's tupdesc can't be changed, + * so allocate a new one. + */ 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); - ExecStoreHeapTuple(tuple, slot, false); - } + slot = ConvertTupleSlot(map, slot, + MakeTupleTableSlot(tupdesc)); } insertedCols = GetInsertedColumns(resultRelInfo, estate); diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index e695470..a90fdcf 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -57,7 +57,7 @@ typedef struct PartitionDispatchData List *keystate; /* list of ExprState */ PartitionDesc partdesc; TupleTableSlot *tupslot; - TupleConversionMap *tupmap; + AttrNumber *tupmap; int *indexes; } PartitionDispatchData; @@ -220,7 +220,6 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, TupleTableSlot *ecxt_scantuple_old = ecxt->ecxt_scantuple; TupleTableSlot *myslot = NULL; MemoryContext oldcxt; - HeapTuple tuple; /* use per-tuple context here to avoid leaking memory */ oldcxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate)); @@ -233,11 +232,10 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, ExecPartitionCheck(resultRelInfo, slot, estate, true); /* start with the root partitioned table */ - tuple = ExecFetchSlotTuple(slot); dispatch = pd[0]; while (true) { - TupleConversionMap *map = dispatch->tupmap; + AttrNumber *map = dispatch->tupmap; int cur_index = -1; rel = dispatch->reldesc; @@ -248,11 +246,7 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, */ myslot = dispatch->tupslot; if (myslot != NULL && map != NULL) - { - tuple = do_convert_tuple(tuple, map); - ExecStoreHeapTuple(tuple, myslot, true); - slot = myslot; - } + slot = ConvertTupleSlot(map, slot, myslot); /* * Extract partition key from tuple. Expression evaluation machinery @@ -297,16 +291,6 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, { /* move down one level */ dispatch = pd[-dispatch->indexes[cur_index]]; - - /* - * Release the dedicated slot, if it was used. Create a copy of - * the tuple first, for the next iteration. - */ - if (slot == myslot) - { - tuple = ExecCopySlotTuple(myslot); - ExecClearTuple(myslot); - } } } @@ -837,36 +821,6 @@ TupConvMapForLeaf(PartitionTupleRouting *proute, } /* - * ConvertPartitionTupleSlot -- convenience function for tuple conversion. - * The tuple, if converted, is stored in new_slot, and *p_my_slot is - * updated to point to it. new_slot typically should be one of the - * dedicated partition tuple slots. If map is NULL, *p_my_slot is not changed. - * - * Returns the converted tuple, unless map is NULL, in which case original - * tuple is returned unmodified. - */ -HeapTuple -ConvertPartitionTupleSlot(TupleConversionMap *map, - HeapTuple tuple, - TupleTableSlot *new_slot, - TupleTableSlot **p_my_slot, - bool shouldFree) -{ - Assert(map != NULL && new_slot != NULL); - - tuple = do_convert_tuple(tuple, map); - - /* - * Change the partition tuple slot descriptor, as per converted tuple. - */ - *p_my_slot = new_slot; - Assert(new_slot != NULL); - ExecStoreHeapTuple(tuple, new_slot, shouldFree); - - return tuple; -} - -/* * ExecCleanupTupleRouting -- Clean up objects allocated for partition tuple * routing. * @@ -1021,9 +975,9 @@ get_partition_dispatch_recurse(Relation rel, Relation parent, * for tuple routing. */ pd->tupslot = MakeSingleTupleTableSlot(tupdesc); - pd->tupmap = convert_tuples_by_name(RelationGetDescr(parent), - tupdesc, - gettext_noop("could not convert row type")); + pd->tupmap = inheritence_tupconv_map(RelationGetDescr(parent), + tupdesc, + gettext_noop("could not convert row type")); } else { diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 9f60be6..d1ab9b4 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1162,11 +1162,8 @@ lreplace:; Assert(map_index >= 0 && map_index < mtstate->mt_nplans); tupconv_map = tupconv_map_for_subplan(mtstate, map_index); if (tupconv_map != NULL) - tuple = ConvertPartitionTupleSlot(tupconv_map, - tuple, - proute->root_tuple_slot, - &slot, - true); + slot = ConvertTupleSlot(tupconv_map->attrMap, + slot, proute->root_tuple_slot); /* * Prepare for tuple routing, making it look like we're inserting @@ -1800,7 +1797,7 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate, 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); + slot = ConvertTupleSlot(map->attrMap, slot, new_slot); } /* Initialize information needed to handle ON CONFLICT DO UPDATE. */ diff --git a/src/include/access/tupconvert.h b/src/include/access/tupconvert.h index 66c0ed0..77568eb 100644 --- a/src/include/access/tupconvert.h +++ b/src/include/access/tupconvert.h @@ -16,6 +16,7 @@ #include "access/htup.h" #include "access/tupdesc.h" +#include "executor/tuptable.h" typedef struct TupleConversionMap @@ -38,12 +39,19 @@ extern TupleConversionMap *convert_tuples_by_name(TupleDesc indesc, TupleDesc outdesc, const char *msg); +extern AttrNumber *inheritence_tupconv_map(TupleDesc indesc, + TupleDesc outdesc, + const char *msg); + extern AttrNumber *convert_tuples_by_name_map(TupleDesc indesc, TupleDesc outdesc, const char *msg); extern HeapTuple do_convert_tuple(HeapTuple tuple, TupleConversionMap *map); +extern TupleTableSlot *ConvertTupleSlot(AttrNumber *attrMap, + TupleTableSlot *in_slot, TupleTableSlot *out_slot); + extern void free_conversion_map(TupleConversionMap *map); #endif /* TUPCONVERT_H */ diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h index 235e60f..8b4a9ca 100644 --- a/src/include/executor/execPartition.h +++ b/src/include/executor/execPartition.h @@ -191,11 +191,6 @@ extern void ExecInitRoutingInfo(ModifyTableState *mtstate, extern void ExecSetupChildParentMapForLeaf(PartitionTupleRouting *proute); extern TupleConversionMap *TupConvMapForLeaf(PartitionTupleRouting *proute, ResultRelInfo *rootRelInfo, int leaf_index); -extern HeapTuple ConvertPartitionTupleSlot(TupleConversionMap *map, - HeapTuple tuple, - TupleTableSlot *new_slot, - TupleTableSlot **p_my_slot, - bool shouldFree); extern void ExecCleanupTupleRouting(ModifyTableState *mtstate, PartitionTupleRouting *proute); extern PartitionPruneState *ExecCreatePartitionPruneState(PlanState *planstate, -- 2.1.4