On 2018/11/01 10:30, David Rowley wrote:
> It's great to know the patch is now so perfect that we've only the
> macro naming left to debate ;-)

I looked over v12 again and noticed a couple minor issues.

+ *              table then we store the index into parenting
+ *              PartitionTupleRouting 'partition_dispatch_info' array.  An

s/PartitionTupleRouting/PartitionTupleRouting's/g

Also, I got a bit concerned about "parenting".  Does it mean something
like "enclosing", because the PartitionDispatch is a member of
PartitionTupleRouting?  I got concerned because using "parent" like this
may be confusing as this is the partitioning code we're talking about,
where "parent" is generally used to mean "parent" table.

+     * the partitioned table that's the target of the command.  If we must
+     * route tuple via some sub-partitioned table, then the PartitionDispatch
+     * for those is only built the first time it's required.

... via some sub-partitioned table"s"

Or perhaps rewrite a bit as:

If we must route the tuple via some sub-partitioned table, then its
PartitionDispatch is built the first time it's required.


The macro naming discussion got me thinking today about the macro itself.
It encapsulates access to the various PartitionTupleRouting arrays
containing the maps, but maybe we've got the interface of tuple routing a
bit (maybe a lot given this thread!) wrong to begin with.  Instead of
ExecFindPartition returning indexes into various PartitionTupleRouting
arrays and its callers then using those indexes to fetch various objects
from those arrays, why doesn't it return those objects itself?  Although
we made sure that the callers don't need to worry about the meaning of
these indexes changing with this patch, it still seems a bit odd for them
to have to go back to those arrays to get various objects.

How about we change ExecFindPartition's interface so that it returns the
ResultRelInfo, the two maps, and the partition slot?  So, the arrays
simply become a cache for ExecFindPartition et al and are no longer
accessed outside execPartition.c.  Although that makes the interface of
ExecFindPartition longer, I think it reduces overall complexity.

I've implemented that in the attached patch
1-revise-ExecFindPartition-interface.patch.

Also, since all members of PartitionTupleRouting are only accessed within
execPartition.c save root_tuple_slot, we can move it to execPartition.c to
make its internals private, after doing something about root_tuple_slot.
Looking at the code related to root_tuple_slot, it seems the field really
belongs in ModifyTableState, because it got nothing to do with routing.
Attached 2-make-PartitionTupleRouting-private.patch does that.

These patches 1 and 2 apply on top of v12-0001.. patch.

Thanks,
Amit

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 0b0696e61e..b45972682f 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2316,6 +2316,7 @@ CopyFrom(CopyState cstate)
        bool       *nulls;
        ResultRelInfo *resultRelInfo;
        ResultRelInfo *target_resultRelInfo;
+       ResultRelInfo *prev_part_rel = NULL;
        EState     *estate = CreateExecutorState(); /* for ExecConstraints() */
        ModifyTableState *mtstate;
        ExprContext *econtext;
@@ -2331,7 +2332,6 @@ CopyFrom(CopyState cstate)
        CopyInsertMethod insertMethod;
        uint64          processed = 0;
        int                     nBufferedTuples = 0;
-       int                     prev_leaf_part_index = -1;
        bool            has_before_insert_row_trig;
        bool            has_instead_insert_row_trig;
        bool            leafpart_use_multi_insert = false;
@@ -2685,19 +2685,24 @@ CopyFrom(CopyState cstate)
                /* Determine the partition to heap_insert the tuple into */
                if (proute)
                {
-                       int                     leaf_part_index;
-                       TupleConversionMap *map;
+                       TupleTableSlot *partition_slot = NULL;
+                       TupleConversionMap *child_to_parent_map,
+                                                          *parent_to_child_map;
 
                        /*
                         * Attempt to find a partition suitable for this tuple.
                         * ExecFindPartition() will raise an error if none can 
be found.
+                        * This replaces the original target ResultRelInfo with
+                        * partition's.
                         */
-                       leaf_part_index = ExecFindPartition(mtstate, 
target_resultRelInfo,
-                                                                               
                proute, slot, estate);
-                       Assert(leaf_part_index >= 0 &&
-                                  leaf_part_index < proute->num_partitions);
+                       resultRelInfo = ExecFindPartition(mtstate, 
target_resultRelInfo,
+                                                                               
          proute, slot, estate,
+                                                                               
          &parent_to_child_map,
+                                                                               
          &partition_slot,
+                                                                               
          &child_to_parent_map);
+                       Assert(resultRelInfo != NULL);
 
-                       if (prev_leaf_part_index != leaf_part_index)
+                       if (prev_part_rel != resultRelInfo)
                        {
                                /* Check if we can multi-insert into this 
partition */
                                if (insertMethod == CIM_MULTI_CONDITIONAL)
@@ -2710,12 +2715,9 @@ CopyFrom(CopyState cstate)
                                        if (nBufferedTuples > 0)
                                        {
                                                ExprContext *swapcontext;
-                                               ResultRelInfo *presultRelInfo;
-
-                                               presultRelInfo = 
proute->partitions[prev_leaf_part_index];
 
                                                CopyFromInsertBatch(cstate, 
estate, mycid, hi_options,
-                                                                               
        presultRelInfo, myslot, bistate,
+                                                                               
        prev_part_rel, myslot, bistate,
                                                                                
        nBufferedTuples, bufferedTuples,
                                                                                
        firstBufferedLineNo);
                                                nBufferedTuples = 0;
@@ -2772,13 +2774,6 @@ CopyFrom(CopyState cstate)
                                        }
                                }
 
-                               /*
-                                * Overwrite resultRelInfo with the 
corresponding partition's
-                                * one.
-                                */
-                               resultRelInfo = 
proute->partitions[leaf_part_index];
-                               Assert(resultRelInfo != NULL);
-
                                /* Determine which triggers exist on this 
partition */
                                has_before_insert_row_trig = 
(resultRelInfo->ri_TrigDesc &&
                                                                                
          resultRelInfo->ri_TrigDesc->trig_insert_before_row);
@@ -2804,7 +2799,7 @@ CopyFrom(CopyState cstate)
                                 * buffer when the partition being inserted 
into changes.
                                 */
                                ReleaseBulkInsertStatePin(bistate);
-                               prev_leaf_part_index = leaf_part_index;
+                               prev_part_rel = resultRelInfo;
                        }
 
                        /*
@@ -2826,8 +2821,7 @@ CopyFrom(CopyState cstate)
                                         * tuplestore format.
                                         */
                                        
cstate->transition_capture->tcs_original_insert_tuple = NULL;
-                                       cstate->transition_capture->tcs_map =
-                                               
PartitionChildToParentMap(proute, leaf_part_index);
+                                       cstate->transition_capture->tcs_map = 
child_to_parent_map;
                                }
                                else
                                {
@@ -2844,16 +2838,13 @@ CopyFrom(CopyState cstate)
                         * We might need to convert from the parent rowtype to 
the
                         * partition rowtype.
                         */
-                       map = PartitionParentToChildMap(proute, 
leaf_part_index);
-                       if (map != NULL)
+                       if (parent_to_child_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];
-                               slot = execute_attr_map_slot(map->attrMap, 
slot, new_slot);
+                               Assert(partition_slot != NULL);
+                               slot = 
execute_attr_map_slot(parent_to_child_map->attrMap,
+                                                                               
         slot, partition_slot);
 
                                /*
                                 * Get the tuple in the per-tuple context, so 
that it will be
@@ -2997,12 +2988,8 @@ CopyFrom(CopyState cstate)
        {
                if (insertMethod == CIM_MULTI_CONDITIONAL)
                {
-                       ResultRelInfo *presultRelInfo;
-
-                       presultRelInfo = 
proute->partitions[prev_leaf_part_index];
-
                        CopyFromInsertBatch(cstate, estate, mycid, hi_options,
-                                                               presultRelInfo, 
myslot, bistate,
+                                                               prev_part_rel, 
myslot, bistate,
                                                                
nBufferedTuples, bufferedTuples,
                                                                
firstBufferedLineNo);
                }
diff --git a/src/backend/executor/execPartition.c 
b/src/backend/executor/execPartition.c
index 542578102f..4b27874f01 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -70,11 +70,16 @@ typedef struct PartitionDispatchData
 static void ExecHashSubPlanResultRelsByOid(ModifyTableState *mtstate,
                                                           
PartitionTupleRouting *proute);
 static void ExecExpandRoutingArrays(PartitionTupleRouting *proute);
-static int ExecInitPartitionInfo(ModifyTableState *mtstate,
+static ResultRelInfo *ExecInitPartitionInfo(ModifyTableState *mtstate,
                                          ResultRelInfo *rootResultRelInfo,
                                          PartitionTupleRouting *proute,
                                          EState *estate,
                                          PartitionDispatch dispatch, int 
partidx);
+static void ExecInitRoutingInfo(ModifyTableState *mtstate,
+                                       EState *estate,
+                                       PartitionTupleRouting *proute,
+                                       ResultRelInfo *partRelInfo,
+                                       int partidx);
 static PartitionDispatch ExecInitPartitionDispatchInfo(PartitionTupleRouting 
*proute,
                                                          Oid partoid, 
PartitionDispatch parent_pd, int partidx);
 static void FormPartitionKeyDatum(PartitionDispatch pd,
@@ -194,14 +199,25 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, 
Relation rel)
  * partition key(s)
  *
  * If no leaf partition is found, this routine errors out with the appropriate
- * error message, else it returns the index of the leaf partition's
- * ResultRelInfo in the proute->partitions array.
+ * error message, else it returns the leaf partition's ResultRelInfo.
+ *
+ * *parent_to_child_map is set if the parent tuples would need to be converted
+ * before inserting into the chosen partition.  In that case,
+ * *partition_tuple_slot is also set.
+ *
+ * *child_to_parent_map is set if the tuples inserted into the partition after
+ * routing would need to be converted back to parent's rowtype for storing
+ * into the transition tuple store, that is, only if transition capture is
+ * active for the command.
  */
-int
+ResultRelInfo *
 ExecFindPartition(ModifyTableState *mtstate,
                                  ResultRelInfo *resultRelInfo,
                                  PartitionTupleRouting *proute,
-                                 TupleTableSlot *slot, EState *estate)
+                                 TupleTableSlot *slot, EState *estate,
+                                 TupleConversionMap **parent_to_child_map,
+                                 TupleTableSlot **partition_slot,
+                                 TupleConversionMap **child_to_parent_map)
 {
        PartitionDispatch *pd = proute->partition_dispatch_info;
        Datum           values[PARTITION_MAX_KEYS];
@@ -214,6 +230,9 @@ ExecFindPartition(ModifyTableState *mtstate,
        TupleTableSlot *myslot = NULL;
        MemoryContext oldcxt;
 
+       *parent_to_child_map = *child_to_parent_map = NULL;
+       *partition_slot = NULL;
+
        /* use per-tuple context here to avoid leaking memory */
        oldcxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
 
@@ -274,7 +293,8 @@ ExecFindPartition(ModifyTableState *mtstate,
 
                if (partdesc->is_leaf[partidx])
                {
-                       int                     result = -1;
+                       int                     index = -1;
+                       ResultRelInfo *result = NULL;
 
                        /*
                         * Get this leaf partition's index in the
@@ -285,7 +305,8 @@ ExecFindPartition(ModifyTableState *mtstate,
                        {
                                /* ResultRelInfo already built */
                                Assert(dispatch->indexes[partidx] < 
proute->num_partitions);
-                               result = dispatch->indexes[partidx];
+                               index = dispatch->indexes[partidx];
+                               result = proute->partitions[index];
                        }
                        else
                        {
@@ -296,36 +317,58 @@ ExecFindPartition(ModifyTableState *mtstate,
                                 */
                                if (proute->subplan_resultrel_hash)
                                {
-                                       ResultRelInfo *rri;
                                        Oid                     partoid = 
partdesc->oids[partidx];
 
-                                       rri = 
hash_search(proute->subplan_resultrel_hash,
-                                                                         
&partoid, HASH_FIND, NULL);
+                                       result = 
hash_search(proute->subplan_resultrel_hash,
+                                                                               
 &partoid, HASH_FIND, NULL);
 
-                                       if (rri)
+                                       if (result)
                                        {
-                                               result = 
proute->num_partitions++;
-                                               dispatch->indexes[partidx] = 
result;
+                                               index = 
proute->num_partitions++;
+                                               dispatch->indexes[partidx] = 
index;
 
 
                                                /* Allocate more space in the 
arrays, if required */
-                                               if (result >= 
proute->partitions_allocsize)
+                                               if (index >= 
proute->partitions_allocsize)
                                                        
ExecExpandRoutingArrays(proute);
 
                                                /* Save here for later use. */
-                                               proute->partitions[result] = 
rri;
+                                               proute->partitions[index] = 
result;
+
+                                               /*
+                                                * We need to make this result 
rel "routable" if it's
+                                                * the first time is is being 
used for routing.  Also,
+                                                * we would've only checked if 
the relation is a valid
+                                                * target for UPDATE when 
creating this ResultRelInfo
+                                                * and now we're about to 
insert the routed tuple into
+                                                * it, so we need to check if 
it's a valid target for
+                                                * INSERT as well.
+                                                */
+                                               if 
(!result->ri_PartitionReadyForRouting)
+                                               {
+                                                       
CheckValidResultRel(result, CMD_INSERT);
+
+                                                       /*
+                                                        * Also set up 
information needed for routing
+                                                        * tuples to the 
partition.
+                                                        */
+                                                       
ExecInitRoutingInfo(mtstate, estate, proute,
+                                                                               
                result, index);
+                                               }
                                        }
                                }
 
                                /* We need to create a new one. */
-                               if (result < 0)
+                               if (result == NULL)
                                {
                                        MemoryContextSwitchTo(oldcxt);
                                        result = ExecInitPartitionInfo(mtstate, 
resultRelInfo,
                                                                                
                   proute, estate,
                                                                                
                   dispatch, partidx);
                                        
MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
-                                       Assert(result >= 0 && result < 
proute->num_partitions);
+                                       Assert(result != NULL);
+                                       index = dispatch->indexes[partidx];
+                                       Assert(index >= 0 && index < 
proute->num_partitions);
                                }
                        }
 
@@ -335,6 +378,26 @@ ExecFindPartition(ModifyTableState *mtstate,
 
                        MemoryContextSwitchTo(oldcxt);
                        ecxt->ecxt_scantuple = ecxt_scantuple_old;
+
+                       /* Set the values of result maps and the slot if 
needed. */
+                       if (proute->parent_child_tupconv_maps)
+                       {
+                               *parent_to_child_map = 
proute->parent_child_tupconv_maps[index];
+
+                               /*
+                                * If non-NULL, correponding tuple slot must 
have been
+                                * initialized for the partition.
+                                */
+                               if (*parent_to_child_map != NULL)
+                               {
+                                       *partition_slot = 
proute->partition_tuple_slots[index];
+                                       Assert(*partition_slot != NULL);
+                               }
+                       }
+
+                       if (proute->child_parent_tupconv_maps)
+                               *child_to_parent_map = 
proute->child_parent_tupconv_maps[index];
+
                        return result;
                }
                else
@@ -374,6 +437,9 @@ ExecFindPartition(ModifyTableState *mtstate,
                        }
                }
        }
+
+       Assert(false);
+       return NULL;    /* keep compiler quiet */
 }
 
 /*
@@ -475,9 +541,10 @@ ExecExpandRoutingArrays(PartitionTupleRouting *proute)
  * ExecInitPartitionInfo
  *             Initialize ResultRelInfo and other information for a partition
  *             and store it in the next empty slot in proute's partitions 
array.
- *             Return the index of the array element.
+ *
+ * Returns the ResultRelInfo
  */
-static int
+static ResultRelInfo *
 ExecInitPartitionInfo(ModifyTableState *mtstate,
                                          ResultRelInfo *rootResultRelInfo,
                                          PartitionTupleRouting *proute,
@@ -742,9 +809,10 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
                 */
                if (node->onConflictAction == ONCONFLICT_UPDATE)
                {
-                       TupleConversionMap *map;
+                       TupleConversionMap *map = NULL;
 
-                       map = PartitionParentToChildMap(proute, 
part_result_rel_index);
+                       if (proute->parent_child_tupconv_maps)
+                               map = 
proute->parent_child_tupconv_maps[part_result_rel_index];
 
                        Assert(node->onConflictSet != NIL);
                        Assert(rootResultRelInfo->ri_onConflict != NULL);
@@ -849,14 +917,14 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
 
        MemoryContextSwitchTo(oldContext);
 
-       return part_result_rel_index;
+       return leaf_part_rri;
 }
 
 /*
  * ExecInitRoutingInfo
  *             Set up information needed for routing tuples to a leaf partition
  */
-void
+static void
 ExecInitRoutingInfo(ModifyTableState *mtstate,
                                        EState *estate,
                                        PartitionTupleRouting *proute,
diff --git a/src/backend/executor/nodeModifyTable.c 
b/src/backend/executor/nodeModifyTable.c
index 840b98811f..18c55e2b19 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1697,46 +1697,22 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate,
                                                TupleTableSlot *slot)
 {
        ModifyTable *node;
-       int                     partidx;
        ResultRelInfo *partrel;
        HeapTuple       tuple;
-       TupleConversionMap *map;
+       TupleTableSlot *partition_slot = NULL;
+       TupleConversionMap *child_to_parent_map,
+                                          *parent_to_child_map;
 
        /*
         * Determine the target partition.  If ExecFindPartition does not find a
-        * partition after all, it doesn't return here; otherwise, the returned
-        * value is to be used as an index into the arrays for the ResultRelInfo
-        * and TupleConversionMap for the partition.
+        * partition after all, it doesn't return here.
         */
-       partidx = ExecFindPartition(mtstate, targetRelInfo, proute, slot, 
estate);
-       Assert(partidx >= 0 && partidx < proute->num_partitions);
-
-       Assert(proute->partitions[partidx] != NULL);
-       /* Get the ResultRelInfo corresponding to the selected partition. */
-       partrel = proute->partitions[partidx];
+       partrel = ExecFindPartition(mtstate, targetRelInfo, proute, slot, 
estate,
+                                                               
&parent_to_child_map, &partition_slot,
+                                                               
&child_to_parent_map);
        Assert(partrel != NULL);
 
        /*
-        * Check whether the partition is routable if we didn't yet
-        *
-        * Note: an UPDATE of a partition key invokes an INSERT that moves the
-        * tuple to a new partition.  This check would be applied to a subplan
-        * partition of such an UPDATE that is chosen as the partition to route
-        * the tuple to.  The reason we do this check here rather than in
-        * ExecSetupPartitionTupleRouting is to avoid aborting such an UPDATE
-        * unnecessarily due to non-routable subplan partitions that may not be
-        * chosen for update tuple movement after all.
-        */
-       if (!partrel->ri_PartitionReadyForRouting)
-       {
-               /* Verify the partition is a valid target for INSERT. */
-               CheckValidResultRel(partrel, CMD_INSERT);
-
-               /* Set up information needed for routing tuples to the 
partition. */
-               ExecInitRoutingInfo(mtstate, estate, proute, partrel, partidx);
-       }
-
-       /*
         * Make it look like we are inserting into the partition.
         */
        estate->es_result_relation_info = partrel;
@@ -1758,8 +1734,7 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate,
                         * to be ready to convert their result back to 
tuplestore format.
                         */
                        
mtstate->mt_transition_capture->tcs_original_insert_tuple = NULL;
-                       mtstate->mt_transition_capture->tcs_map =
-                                                                       
PartitionChildToParentMap(proute, partidx);
+                       mtstate->mt_transition_capture->tcs_map = 
child_to_parent_map;
                }
                else
                {
@@ -1772,23 +1747,17 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate,
                }
        }
        if (mtstate->mt_oc_transition_capture != NULL)
-       {
-               mtstate->mt_oc_transition_capture->tcs_map =
-                                                               
PartitionChildToParentMap(proute, partidx);
-       }
+               mtstate->mt_oc_transition_capture->tcs_map = 
child_to_parent_map;
 
        /*
         * Convert the tuple, if necessary.
         */
-       map = PartitionParentToChildMap(proute, partidx);
-       if (map != NULL)
+       if (parent_to_child_map != NULL)
        {
-               TupleTableSlot *new_slot;
 
-               Assert(proute->partition_tuple_slots != NULL &&
-                          proute->partition_tuple_slots[partidx] != NULL);
-               new_slot = proute->partition_tuple_slots[partidx];
-               slot = execute_attr_map_slot(map->attrMap, slot, new_slot);
+               Assert(partition_slot != NULL);
+               slot = execute_attr_map_slot(parent_to_child_map->attrMap, slot,
+                                                                        
partition_slot);
        }
 
        /* Initialize information needed to handle ON CONFLICT DO UPDATE. */
diff --git a/src/include/executor/execPartition.h 
b/src/include/executor/execPartition.h
index 45d5f6a8d0..7c8314362c 100644
--- a/src/include/executor/execPartition.h
+++ b/src/include/executor/execPartition.h
@@ -122,20 +122,6 @@ typedef struct PartitionTupleRouting
 } PartitionTupleRouting;
 
 /*
- * Accessor macros for tuple conversion maps contained in
- * PartitionTupleRouting.  Beware of multiple evaluations of p!
- */
-#define PartitionChildToParentMap(p, i) \
-                       ((p)->child_parent_tupconv_maps != NULL ? \
-                               (p)->child_parent_tupconv_maps[(i)] : \
-                                                       NULL)
-
-#define PartitionParentToChildMap(p, i) \
-                       ((p)->parent_child_tupconv_maps != NULL ? \
-                               (p)->parent_child_tupconv_maps[(i)] : \
-                                                       NULL)
-
-/*
  * PartitionedRelPruningData - Per-partitioned-table data for run-time pruning
  * of partitions.  For a multilevel partitioned table, we have one of these
  * for the topmost partition plus one for each non-leaf child partition.
@@ -223,20 +209,14 @@ typedef struct PartitionPruneState
 
 extern PartitionTupleRouting *ExecSetupPartitionTupleRouting(ModifyTableState 
*mtstate,
                                                           Relation rel);
-extern int ExecFindPartition(ModifyTableState *mtstate,
+extern ResultRelInfo *ExecFindPartition(ModifyTableState *mtstate,
                                  ResultRelInfo *resultRelInfo,
                                  PartitionTupleRouting *proute,
                                  TupleTableSlot *slot,
-                                 EState *estate);
-extern ResultRelInfo *ExecGetPartitionInfo(ModifyTableState *mtstate,
-                                        ResultRelInfo *resultRelInfo,
-                                        PartitionTupleRouting *proute,
-                                        EState *estate, int partidx);
-extern void ExecInitRoutingInfo(ModifyTableState *mtstate,
-                                       EState *estate,
-                                       PartitionTupleRouting *proute,
-                                       ResultRelInfo *partRelInfo,
-                                       int partidx);
+                                 EState *estate,
+                                 TupleConversionMap **parent_to_child_map,
+                                 TupleTableSlot **partition_slot,
+                                 TupleConversionMap **child_to_parent_map);
 extern void ExecCleanupTupleRouting(ModifyTableState *mtstate,
                                                PartitionTupleRouting *proute);
 extern PartitionPruneState *ExecCreatePartitionPruneState(PlanState *planstate,
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index b45972682f..2b5e71b843 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2528,7 +2528,7 @@ CopyFrom(CopyState cstate)
         * CopyFrom tuple routing.
         */
        if (cstate->rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-               proute = ExecSetupPartitionTupleRouting(NULL, cstate->rel);
+               proute = ExecSetupPartitionTupleRouting(mtstate, cstate->rel);
 
        /*
         * It's more efficient to prepare a bunch of tuples for insertion, and
diff --git a/src/backend/executor/execPartition.c 
b/src/backend/executor/execPartition.c
index 4b27874f01..0978a55f48 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -66,6 +66,98 @@ typedef struct PartitionDispatchData
        int                     indexes[FLEXIBLE_ARRAY_MEMBER];
 } PartitionDispatchData;
 
+/*-----------------------
+ * PartitionTupleRouting - Encapsulates all information required to
+ * route a tuple inserted into a partitioned table to one of its leaf
+ * partitions
+ *
+ * partition_root                      The partitioned table that's the target 
of the
+ *                                                     command.
+ *
+ * partition_dispatch_info     Array of 'dispatch_allocsize' elements 
containing
+ *                                                     a pointer to a 
PartitionDispatch objects for every
+ *                                                     partitioned table 
touched by tuple routing.  The
+ *                                                     entry for the target 
partitioned table is *always*
+ *                                                     present in the 0th 
element of this array.  See
+ *                                                     comment for 
PartitionDispatchData->indexes for
+ *                                                     details on how this 
array is indexed.
+ *
+ * num_dispatch                                The current number of items 
stored in the
+ *                                                     
'partition_dispatch_info' array.  Also serves as
+ *                                                     the index of the next 
free array element for new
+ *                                                     PartitionDispatch which 
need to be stored.
+ *
+ * dispatch_allocsize          The current allocated size of the
+ *                                                     
'partition_dispatch_info' array.
+ *
+ * partitions                          Array of 'partitions_allocsize' elements
+ *                                                     containing pointers to 
a ResultRelInfos of all
+ *                                                     leaf partitions touched 
by tuple routing.  Some of
+ *                                                     these are pointers to 
ResultRelInfos which are
+ *                                                     borrowed out of 
'subplan_resultrel_hash'.  The
+ *                                                     remainder have been 
built especially for tuple
+ *                                                     routing.  See comment 
for
+ *                                                     
PartitionDispatchData->indexes for details on how
+ *                                                     this array is indexed.
+ *
+ * num_partitions                      The current number of items stored in 
the
+ *                                                     'partitions' array.  
Also serves as the index of
+ *                                                     the next free array 
element for new ResultRelInfos
+ *                                                     which need to be stored.
+ *
+ * partitions_allocsize                The current allocated size of the 
'partitions'
+ *                                                     array.  Also, if 
they're non-NULL, marks the size
+ *                                                     of the 
'parent_child_tupconv_maps',
+ *                                                     
'child_parent_tupconv_maps',
+ *                                                     
'child_parent_map_not_required' and
+ *                                                     'partition_tuple_slots' 
arrays.
+ *
+ * parent_child_tupconv_maps   Array of partitions_allocsize elements
+ *                                                     containing information 
on how to convert tuples of
+ *                                                     partition_root's 
rowtype to the rowtype of the
+ *                                                     corresponding partition 
as stored in 'partitions',
+ *                                                     or NULL if no 
conversion is required.  The entire
+ *                                                     array is only allocated 
when the first conversion
+ *                                                     map needs to stored.  
When not allocated it's set
+ *                                                     to NULL.
+ *
+ * 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
+ *
+ * child_parent_tupconv_maps   As 'parent_child_tupconv_maps' but stores
+ *                                                     conversion maps to 
translate partition tuples into
+ *                                                     partition_root's 
rowtype, needed if transition
+ *                                                     capture is active
+ *
+ * Note: The following fields are used only when UPDATE ends up needing to
+ * do tuple routing.
+ *
+ * subplan_resultrel_hash      Hash table to store subplan ResultRelInfos by 
Oid.
+ *                                                     This is used to cache 
ResultRelInfos from subplans
+ *                                                     of a ModifyTable node.  
Some of these may be
+ *                                                     useful for tuple 
routing to save having to build
+ *                                                     duplicates.
+ *-----------------------
+ */
+typedef struct PartitionTupleRouting
+{
+       Relation        partition_root;
+       PartitionDispatch *partition_dispatch_info;
+       int                     num_dispatch;
+       int                     dispatch_allocsize;
+       ResultRelInfo **partitions;
+       int                     num_partitions;
+       int                     partitions_allocsize;
+       TupleConversionMap **parent_child_tupconv_maps;
+       TupleConversionMap **child_parent_tupconv_maps;
+       TupleTableSlot **partition_tuple_slots;
+       HTAB       *subplan_resultrel_hash;
+} PartitionTupleRouting;
 
 static void ExecHashSubPlanResultRelsByOid(ModifyTableState *mtstate,
                                                           
PartitionTupleRouting *proute);
@@ -179,12 +271,12 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, 
Relation rel)
        if (node && node->operation == CMD_UPDATE)
        {
                ExecHashSubPlanResultRelsByOid(mtstate, proute);
-               proute->root_tuple_slot = 
MakeTupleTableSlot(RelationGetDescr(rel));
+               mtstate->mt_root_tuple_slot = 
MakeTupleTableSlot(RelationGetDescr(rel));
        }
        else
        {
                proute->subplan_resultrel_hash = NULL;
-               proute->root_tuple_slot = NULL;
+               mtstate->mt_root_tuple_slot = NULL;
        }
 
        return proute;
@@ -1175,10 +1267,6 @@ ExecCleanupTupleRouting(ModifyTableState *mtstate,
                ExecCloseIndices(resultRelInfo);
                heap_close(resultRelInfo->ri_RelationDesc, NoLock);
        }
-
-       /* Release the standalone partition tuple descriptors, if any */
-       if (proute->root_tuple_slot)
-               ExecDropSingleTupleTableSlot(proute->root_tuple_slot);
 }
 
 /* ----------------
diff --git a/src/backend/executor/nodeModifyTable.c 
b/src/backend/executor/nodeModifyTable.c
index 18c55e2b19..390191bdc8 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1161,8 +1161,8 @@ lreplace:;
                        Assert(map_index >= 0 && map_index < 
mtstate->mt_nplans);
                        tupconv_map = tupconv_map_for_subplan(mtstate, 
map_index);
                        if (tupconv_map != NULL)
-                               slot = 
execute_attr_map_slot(tupconv_map->attrMap,
-                                                                               
         slot, proute->root_tuple_slot);
+                               slot = 
execute_attr_map_slot(tupconv_map->attrMap, slot,
+                                                                               
         mtstate->mt_root_tuple_slot);
 
                        /*
                         * Prepare for tuple routing, making it look like we're 
inserting
@@ -2616,6 +2616,10 @@ ExecEndModifyTable(ModifyTableState *node)
         */
        for (i = 0; i < node->mt_nplans; i++)
                ExecEndNode(node->mt_plans[i]);
+
+       /* Release the standalone partition tuple descriptors, if any */
+       if (node->mt_root_tuple_slot)
+               ExecDropSingleTupleTableSlot(node->mt_root_tuple_slot);
 }
 
 void
diff --git a/src/include/executor/execPartition.h 
b/src/include/executor/execPartition.h
index 7c8314362c..91886f1b19 100644
--- a/src/include/executor/execPartition.h
+++ b/src/include/executor/execPartition.h
@@ -18,108 +18,9 @@
 #include "nodes/plannodes.h"
 #include "partitioning/partprune.h"
 
-/* See execPartition.c for the definition. */
+/* See execPartition.c for the definitions. */
 typedef struct PartitionDispatchData *PartitionDispatch;
-
-/*-----------------------
- * PartitionTupleRouting - Encapsulates all information required to
- * route a tuple inserted into a partitioned table to one of its leaf
- * partitions
- *
- * partition_root                      The partitioned table that's the target 
of the
- *                                                     command.
- *
- * partition_dispatch_info     Array of 'dispatch_allocsize' elements 
containing
- *                                                     a pointer to a 
PartitionDispatch objects for every
- *                                                     partitioned table 
touched by tuple routing.  The
- *                                                     entry for the target 
partitioned table is *always*
- *                                                     present in the 0th 
element of this array.  See
- *                                                     comment for 
PartitionDispatchData->indexes for
- *                                                     details on how this 
array is indexed.
- *
- * num_dispatch                                The current number of items 
stored in the
- *                                                     
'partition_dispatch_info' array.  Also serves as
- *                                                     the index of the next 
free array element for new
- *                                                     PartitionDispatch which 
need to be stored.
- *
- * dispatch_allocsize          The current allocated size of the
- *                                                     
'partition_dispatch_info' array.
- *
- * partitions                          Array of 'partitions_allocsize' elements
- *                                                     containing pointers to 
a ResultRelInfos of all
- *                                                     leaf partitions touched 
by tuple routing.  Some of
- *                                                     these are pointers to 
ResultRelInfos which are
- *                                                     borrowed out of 
'subplan_resultrel_hash'.  The
- *                                                     remainder have been 
built especially for tuple
- *                                                     routing.  See comment 
for
- *                                                     
PartitionDispatchData->indexes for details on how
- *                                                     this array is indexed.
- *
- * num_partitions                      The current number of items stored in 
the
- *                                                     'partitions' array.  
Also serves as the index of
- *                                                     the next free array 
element for new ResultRelInfos
- *                                                     which need to be stored.
- *
- * partitions_allocsize                The current allocated size of the 
'partitions'
- *                                                     array.  Also, if 
they're non-NULL, marks the size
- *                                                     of the 
'parent_child_tupconv_maps',
- *                                                     
'child_parent_tupconv_maps',
- *                                                     
'child_parent_map_not_required' and
- *                                                     'partition_tuple_slots' 
arrays.
- *
- * parent_child_tupconv_maps   Array of partitions_allocsize elements
- *                                                     containing information 
on how to convert tuples of
- *                                                     partition_root's 
rowtype to the rowtype of the
- *                                                     corresponding partition 
as stored in 'partitions',
- *                                                     or NULL if no 
conversion is required.  The entire
- *                                                     array is only allocated 
when the first conversion
- *                                                     map needs to stored.  
When not allocated it's set
- *                                                     to NULL.
- *
- * 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
- *
- * child_parent_tupconv_maps   As 'parent_child_tupconv_maps' but stores
- *                                                     conversion maps to 
translate partition tuples into
- *                                                     partition_root's 
rowtype, needed if transition
- *                                                     capture is active
- *
- * Note: The following fields are used only when UPDATE ends up needing to
- * do tuple routing.
- *
- * subplan_resultrel_hash      Hash table to store subplan ResultRelInfos by 
Oid.
- *                                                     This is used to cache 
ResultRelInfos from subplans
- *                                                     of a ModifyTable node.  
Some of these may be
- *                                                     useful for tuple 
routing to save having to build
- *                                                     duplicates.
- *
- * root_tuple_slot                     During UPDATE tuple routing, this tuple 
slot is
- *                                                     used to transiently 
store a tuple using the root
- *                                                     table's rowtype after 
converting it from the
- *                                                     tuple's source leaf 
partition's rowtype.  That is,
- *                                                     if leaf partition's 
rowtype is different.
- *-----------------------
- */
-typedef struct PartitionTupleRouting
-{
-       Relation        partition_root;
-       PartitionDispatch *partition_dispatch_info;
-       int                     num_dispatch;
-       int                     dispatch_allocsize;
-       ResultRelInfo **partitions;
-       int                     num_partitions;
-       int                     partitions_allocsize;
-       TupleConversionMap **parent_child_tupconv_maps;
-       TupleConversionMap **child_parent_tupconv_maps;
-       TupleTableSlot **partition_tuple_slots;
-       TupleTableSlot *root_tuple_slot;
-       HTAB       *subplan_resultrel_hash;
-} PartitionTupleRouting;
+typedef struct PartitionTupleRouting PartitionTupleRouting;
 
 /*
  * PartitionedRelPruningData - Per-partitioned-table data for run-time pruning
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 880a03e4e4..73ecc20074 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1084,6 +1084,14 @@ typedef struct ModifyTableState
 
        /* Per plan map for tuple conversion from child to root */
        TupleConversionMap **mt_per_subplan_tupconv_maps;
+
+       /*
+        * During UPDATE tuple routing, this tuple slot is used to transiently
+        * store a tuple using the root table's rowtype after converting it from
+        * the tuple's source leaf partition's rowtype.  That is, if leaf
+        * partition's rowtype is different.
+        */
+       TupleTableSlot *mt_root_tuple_slot;
 } ModifyTableState;
 
 /* ----------------

Reply via email to