Fujita-san,

Thank you for the review.

On 2018/02/02 19:56, Etsuro Fujita wrote:
> (2018/01/30 18:52), Etsuro Fujita wrote:
>> (2018/01/30 18:39), Amit Langote wrote:
>>> Will wait for your comments before sending a new version then.
>>
>> Ok, I'll post my comments as soon as possible.
> 
> * ExecInitPartitionResultRelInfo is called from ExecFindPartition, but we
> could call that another way; in ExecInsert/CopyFrom we call that after
> ExecFindPartition if the partition chosen by ExecFindPartition has not
> been initialized yet.  Maybe either would be OK, but I like that because I
> think that would not only better divide that labor but better fit into the
> existing code in ExecInsert/CopyFrom IMO.

I see no problem with that, so done that way.

> * In ExecInitPartitionResultRelInfo:
> +       /*
> +        * Note that the entries in this list appear in no predetermined
> +        * order as result of initializing partition result rels as and when
> +        * they're needed.
> +        */
> +       estate->es_leaf_result_relations =
> + lappend(estate->es_leaf_result_relations,
> +                                           leaf_part_rri);
> 
> Is it OK to put this within the "if (leaf_part_rri == NULL)" block?

Good catch.  I moved it outside the block.  I was under the impression
that leaf result relations that were reused from the
mtstate->resultRelInfo arrary would have already been added to the list,
but it seems they are not.

> * In the same function:
> +   /*
> +    * Verify result relation is a valid target for INSERT.
> +    */
> +   CheckValidResultRel(leaf_part_rri, CMD_INSERT);
> 
> I think it would be better to leave the previous comments as-is here:
> 
>         /*
>          * Verify result relation is a valid target for an INSERT.  An UPDATE
>          * of a partition-key becomes a DELETE+INSERT operation, so this
> check
>          * is still required when the operation is CMD_UPDATE.
>          */

Oops, my bad.  Didn't notice that I had ended up removing the part about
UPDATE.

> * ExecInitPartitionResultRelInfo does the work other than the
> initialization of ResultRelInfo for the chosen partition (eg, create a
> tuple conversion map to convert a tuple routed to the partition from the
> parent's type to the partition's).  So I'd propose to rename that function
> to eg, ExecInitPartition.

I went with ExevInitPartitionInfo.

> * CopyFrom is modified so that it calls ExecSetupPartitionTupleRouting and
> ExecFindPartition with a mostly-dummy ModifyTableState node.  I'm not sure
> that is a good idea.  My concern about that is that might be something
> like a headache in future development.

OK, I removed those changes.

> * The patch 0001 and 0002 are pretty small but can't be reviewed without
> the patch 0003.  The total size of the three patches aren't that large, so
> I think it would be better to put those patches together into a single patch.

As I said above, I got rid of 0001.  Then, I merged the
ExecFindPartition() refactoring patch 0002 into 0003.

The code in tupconv_map_for_subplan() currently assumes that it can rely
on all leaf partitions having been initialized.  Since we're breaking that
assumption with this proposal, that needed to be changed.  So the patch
contained some refactoring to make it not rely on that assumption.  I
carved that out into a separate patch which can be applied and tested
before the main patch.

> That's all for now.  I'll continue to review the patches!

Here is the updated version that contains two patches as described above.

Thanks,
Amit
From 03ec63385251f31bdf95006258617d10eda94709 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 5 Feb 2018 13:38:05 +0900
Subject: [PATCH v24 1/2] Refactor partition tuple conversion maps handling
 code

tupconv_map_for_subplan() currently assumes that it gets to use
the Relation pointer for *all* leaf partitions.  That is, both those
that exist in mtstate->resultRelInfo array and those that don't and
hence would be initialized by ExecSetupPartitionTupleRouting().
However, an upcoming patch will change ExecSetupPartitionTupleRouting
such that leaf partitions' ResultRelInfo are no longer initialized
there.  So make it stop relying on the latter.
---
 src/backend/executor/execPartition.c   | 87 ++++++++++++++++++++++++++--------
 src/backend/executor/nodeModifyTable.c | 23 ++-------
 src/include/executor/execPartition.h   |  2 +
 3 files changed, 73 insertions(+), 39 deletions(-)

diff --git a/src/backend/executor/execPartition.c 
b/src/backend/executor/execPartition.c
index 106a96d910..f2a920f4c3 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -373,40 +373,89 @@ ExecSetupChildParentMapForLeaf(PartitionTupleRouting 
*proute)
 }
 
 /*
- * TupConvMapForLeaf -- Get the tuple conversion map for a given leaf partition
- * index.
+ * ChildParentTupConvMap -- Return tuple conversion map to convert tuples of
+ *                                                     'partrel' into those of 
'rootrel'
+ *
+ * If the function was previously called for this partition, we would've
+ * either already created the map and stored the same at
+ * proute->child_parent_tupconv_maps[index] or found out that such a map
+ * is not needed and thus set proute->child_parent_map_not_required[index].
  */
-TupleConversionMap *
-TupConvMapForLeaf(PartitionTupleRouting *proute,
-                                 ResultRelInfo *rootRelInfo, int leaf_index)
+static TupleConversionMap *
+ChildParentTupConvMap(Relation partrel, Relation rootrel,
+                                         PartitionTupleRouting *proute, int 
index)
 {
-       ResultRelInfo **resultRelInfos = proute->partitions;
-       TupleConversionMap **map;
-       TupleDesc       tupdesc;
+       TupleConversionMap *map;
 
        /* Don't call this if we're not supposed to be using this type of map. 
*/
        Assert(proute->child_parent_tupconv_maps != NULL);
 
        /* If it's already known that we don't need a map, return NULL. */
-       if (proute->child_parent_map_not_required[leaf_index])
+       if (proute->child_parent_map_not_required[index])
                return NULL;
 
        /* If we've already got a map, return it. */
-       map = &proute->child_parent_tupconv_maps[leaf_index];
-       if (*map != NULL)
-               return *map;
+       map = proute->child_parent_tupconv_maps[index];
+       if (map != NULL)
+               return map;
 
        /* No map yet; try to create one. */
-       tupdesc = RelationGetDescr(resultRelInfos[leaf_index]->ri_RelationDesc);
-       *map =
-               convert_tuples_by_name(tupdesc,
-                                                          
RelationGetDescr(rootRelInfo->ri_RelationDesc),
-                                                          gettext_noop("could 
not convert row type"));
+       map = convert_tuples_by_name(RelationGetDescr(partrel),
+                                                                
RelationGetDescr(rootrel),
+                                                                
gettext_noop("could not convert row type"));
 
        /* If it turns out no map is needed, remember for next time. */
-       proute->child_parent_map_not_required[leaf_index] = (*map == NULL);
+       proute->child_parent_map_not_required[index] = (map == NULL);
+
+       return map;
+}
+
+/*
+ * TupConvMapForLeaf -- Get the tuple conversion map for a given leaf partition
+ * index.
+ *
+ * Call this only if it's known that the partition at leaf_index has been
+ * initialized.
+ */
+TupleConversionMap *
+TupConvMapForLeaf(PartitionTupleRouting *proute,
+                                 ResultRelInfo *rootRelInfo, int leaf_index)
+{
+       ResultRelInfo **resultrels = proute->partitions;
+
+       Assert(resultrels[leaf_index] != NULL);
 
-       return *map;
+       return ChildParentTupConvMap(resultrels[leaf_index]->ri_RelationDesc,
+                                                                
rootRelInfo->ri_RelationDesc, proute,
+                                                                leaf_index);
+}
+
+/*
+ * TupConvMapForSubplan -- Get the tuple conversion map for a partition given
+ * its subplan index.
+ *
+ * Call this if it's unclear whether the partition's ResultRelInfo has been
+ * initialized in mtstate->mt_partition_tuple_routing.
+ */
+TupleConversionMap *
+TupConvMapForSubplan(ModifyTableState *mtstate, int subplan_index)
+{
+       int             leaf_index;
+       PartitionTupleRouting *proute = mtstate->mt_partition_tuple_routing;
+       ResultRelInfo *resultrels = mtstate->resultRelInfo,
+                                 *rootRelInfo = (mtstate->rootResultRelInfo != 
NULL)
+                                                                       ? 
mtstate->rootResultRelInfo
+                                                                       : 
mtstate->resultRelInfo;
+
+       Assert(proute != NULL &&
+                  proute->subplan_partition_offsets != NULL &&
+                  subplan_index < proute->num_subplan_partition_offsets);
+       leaf_index = proute->subplan_partition_offsets[subplan_index];
+
+       Assert(subplan_index >= 0 && subplan_index < mtstate->mt_nplans);
+       return ChildParentTupConvMap(resultrels[subplan_index].ri_RelationDesc,
+                                                                
rootRelInfo->ri_RelationDesc,
+                                                                proute, 
leaf_index);
 }
 
 /*
diff --git a/src/backend/executor/nodeModifyTable.c 
b/src/backend/executor/nodeModifyTable.c
index 2a8ecbd830..d054da5330 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1804,27 +1804,10 @@ tupconv_map_for_subplan(ModifyTableState *mtstate, int 
whichplan)
         * array *only* if partition-indexed array is not required.
         */
        if (mtstate->mt_per_subplan_tupconv_maps == NULL)
-       {
-               int                     leaf_index;
-               PartitionTupleRouting *proute = 
mtstate->mt_partition_tuple_routing;
-
-               /*
-                * If subplan-indexed array is NULL, things should have been 
arranged
-                * to convert the subplan index to partition index.
-                */
-               Assert(proute && proute->subplan_partition_offsets != NULL &&
-                          whichplan < proute->num_subplan_partition_offsets);
+               return TupConvMapForSubplan(mtstate, whichplan);
 
-               leaf_index = proute->subplan_partition_offsets[whichplan];
-
-               return TupConvMapForLeaf(proute, 
getTargetResultRelInfo(mtstate),
-                                                                leaf_index);
-       }
-       else
-       {
-               Assert(whichplan >= 0 && whichplan < mtstate->mt_nplans);
-               return mtstate->mt_per_subplan_tupconv_maps[whichplan];
-       }
+       Assert(whichplan >= 0 && whichplan < mtstate->mt_nplans);
+       return mtstate->mt_per_subplan_tupconv_maps[whichplan];
 }
 
 /* ----------------------------------------------------------------
diff --git a/src/include/executor/execPartition.h 
b/src/include/executor/execPartition.h
index 3df9c498bb..a75a37060a 100644
--- a/src/include/executor/execPartition.h
+++ b/src/include/executor/execPartition.h
@@ -112,6 +112,8 @@ extern int ExecFindPartition(ResultRelInfo *resultRelInfo,
 extern void ExecSetupChildParentMapForLeaf(PartitionTupleRouting *proute);
 extern TupleConversionMap *TupConvMapForLeaf(PartitionTupleRouting *proute,
                                  ResultRelInfo *rootRelInfo, int leaf_index);
+extern TupleConversionMap *TupConvMapForSubplan(ModifyTableState *mtstate,
+                               int subplan_index);
 extern HeapTuple ConvertPartitionTupleSlot(TupleConversionMap *map,
                                                  HeapTuple tuple,
                                                  TupleTableSlot *new_slot,
-- 
2.11.0

From 316c54eb48d9973a2859a31a993d065efa791299 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Tue, 19 Dec 2017 16:20:09 +0900
Subject: [PATCH v24 2/2] During tuple-routing, initialize per-partition
 objects lazily

Those objects include ResultRelInfo, tuple conversion map,
WITH CHECK OPTION quals and RETURNING projections.

This means we don't allocate these objects for partitions that are
never inserted into.
---
 src/backend/commands/copy.c            |   7 +-
 src/backend/executor/execPartition.c   | 409 +++++++++++++++++++++++----------
 src/backend/executor/nodeModifyTable.c | 128 +----------
 src/include/executor/execPartition.h   |   8 +-
 4 files changed, 302 insertions(+), 250 deletions(-)

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index b3933df9af..d69d88c8a8 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2470,7 +2470,7 @@ CopyFrom(CopyState cstate)
                PartitionTupleRouting *proute;
 
                proute = cstate->partition_tuple_routing =
-                       ExecSetupPartitionTupleRouting(NULL, cstate->rel, 1, 
estate);
+                       ExecSetupPartitionTupleRouting(NULL, cstate->rel);
 
                /*
                 * If we are capturing transition tuples, they may need to be
@@ -2590,6 +2590,10 @@ CopyFrom(CopyState cstate)
                        Assert(leaf_part_index >= 0 &&
                                   leaf_part_index < proute->num_partitions);
 
+                       /* Initialize partition info, if not done already. */
+                       ExecInitPartitionInfo(NULL, resultRelInfo, proute, 
estate,
+                                                                 
leaf_part_index);
+
                        /*
                         * If this tuple is mapped to a partition that is not 
same as the
                         * previous one, we'd better make the bulk insert 
mechanism gets a
@@ -2607,6 +2611,7 @@ CopyFrom(CopyState cstate)
                         */
                        saved_resultRelInfo = resultRelInfo;
                        resultRelInfo = proute->partitions[leaf_part_index];
+                       Assert(resultRelInfo != NULL);
 
                        /* We do not yet have a way to insert into a foreign 
partition */
                        if (resultRelInfo->ri_FdwRoutine)
diff --git a/src/backend/executor/execPartition.c 
b/src/backend/executor/execPartition.c
index f2a920f4c3..6386dea5fb 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -44,22 +44,23 @@ static char *ExecBuildSlotPartitionKeyDescription(Relation 
rel,
  *
  * Note that all the relations in the partition tree are locked using the
  * RowExclusiveLock mode upon return from this function.
+ *
+ * While we allocate the arrays of pointers of various objects for all
+ * partitions here, the objects themselves are lazily allocated and
+ * initialized for a given partition if a tuple is actually routed to it;
+ * see ExecInitPartitionInfo.
  */
 PartitionTupleRouting *
-ExecSetupPartitionTupleRouting(ModifyTableState *mtstate,
-                                                          Relation rel, Index 
resultRTindex,
-                                                          EState *estate)
+ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel)
 {
-       TupleDesc       tupDesc = RelationGetDescr(rel);
+       PartitionTupleRouting *proute;
        List       *leaf_parts;
        ListCell   *cell;
-       int                     i;
-       ResultRelInfo *leaf_part_arr = NULL,
-                          *update_rri = NULL;
-       int                     num_update_rri = 0,
-                               update_rri_index = 0;
+       int                     leaf_index,
+                               update_rri_index,
+                               num_update_rri;
        bool            is_update = false;
-       PartitionTupleRouting *proute;
+       ResultRelInfo *update_rri;
 
        /*
         * Get the information about the partition tree after locking all the
@@ -71,20 +72,24 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate,
                RelationGetPartitionDispatchInfo(rel, &proute->num_dispatch,
                                                                                
 &leaf_parts);
        proute->num_partitions = list_length(leaf_parts);
-       proute->partitions = (ResultRelInfo **) palloc(proute->num_partitions *
-                                                                               
                   sizeof(ResultRelInfo *));
+       /*
+        * Actual ResultRelInfo's and TupleConversionMap's are allocated in
+        * ExecInitResultRelInfo().
+        */
+       proute->partitions = (ResultRelInfo **) palloc0(proute->num_partitions *
+                                                                               
                        sizeof(ResultRelInfo *));
        proute->parent_child_tupconv_maps =
                (TupleConversionMap **) palloc0(proute->num_partitions *
                                                                                
sizeof(TupleConversionMap *));
+       proute->partition_oids = (Oid *) palloc0(proute->num_partitions *
+                                                                               
         sizeof(Oid));
 
        /* Set up details specific to the type of tuple routing we are doing. */
        if (mtstate && mtstate->operation == CMD_UPDATE)
        {
-               ModifyTable *node = (ModifyTable *) mtstate->ps.plan;
-
                is_update = true;
                update_rri = mtstate->resultRelInfo;
-               num_update_rri = list_length(node->plans);
+               num_update_rri = mtstate->mt_nplans;
                proute->subplan_partition_offsets =
                        palloc(num_update_rri * sizeof(int));
                proute->num_subplan_partition_offsets = num_update_rri;
@@ -95,15 +100,29 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate,
                 */
                proute->root_tuple_slot = MakeTupleTableSlot();
        }
-       else
+
+       leaf_index = update_rri_index = 0;
+       foreach (cell, leaf_parts)
        {
+               Oid             leaf_oid = lfirst_oid(cell);
+
+               proute->partition_oids[leaf_index] = leaf_oid;
+
                /*
-                * Since we are inserting tuples, we need to create all new 
result
-                * rels. Avoid repeated pallocs by allocating memory for all the
-                * result rels in bulk.
+                * The per-subplan resultrels and the resultrels of the leaf
+                * partitions are both in the same canonical order.  So while 
going
+                * through the leaf partition oids, we need to keep track of the
+                * next per-subplan result rel to be looked for in the leaf
+                * partition resultrels.
                 */
-               leaf_part_arr = (ResultRelInfo *) 
palloc0(proute->num_partitions *
-                                                                               
                  sizeof(ResultRelInfo));
+               if (is_update && update_rri_index < num_update_rri &&
+                       
RelationGetRelid(update_rri[update_rri_index].ri_RelationDesc) == leaf_oid)
+               {
+                       proute->subplan_partition_offsets[update_rri_index] = 
leaf_index;
+                       update_rri_index++;
+               }
+
+               leaf_index++;
        }
 
        /*
@@ -114,109 +133,6 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate,
         */
        proute->partition_tuple_slot = MakeTupleTableSlot();
 
-       i = 0;
-       foreach(cell, leaf_parts)
-       {
-               ResultRelInfo *leaf_part_rri;
-               Relation        partrel = NULL;
-               TupleDesc       part_tupdesc;
-               Oid                     leaf_oid = lfirst_oid(cell);
-
-               if (is_update)
-               {
-                       /*
-                        * If the leaf partition is already present in the 
per-subplan
-                        * result rels, we re-use that rather than initialize a 
new result
-                        * rel. The per-subplan resultrels and the resultrels 
of the leaf
-                        * partitions are both in the same canonical order. So 
while going
-                        * through the leaf partition oids, we need to keep 
track of the
-                        * next per-subplan result rel to be looked for in the 
leaf
-                        * partition resultrels.
-                        */
-                       if (update_rri_index < num_update_rri &&
-                               
RelationGetRelid(update_rri[update_rri_index].ri_RelationDesc) == leaf_oid)
-                       {
-                               leaf_part_rri = &update_rri[update_rri_index];
-                               partrel = leaf_part_rri->ri_RelationDesc;
-
-                               /*
-                                * This is required in order to we convert the 
partition's
-                                * tuple to be compatible with the root 
partitioned table's
-                                * tuple descriptor.  When generating the 
per-subplan result
-                                * rels, this was not set.
-                                */
-                               leaf_part_rri->ri_PartitionRoot = rel;
-
-                               /* Remember the subplan offset for this 
ResultRelInfo */
-                               
proute->subplan_partition_offsets[update_rri_index] = i;
-
-                               update_rri_index++;
-                       }
-                       else
-                               leaf_part_rri = (ResultRelInfo *) 
palloc0(sizeof(ResultRelInfo));
-               }
-               else
-               {
-                       /* For INSERTs, we already have an array of result rels 
allocated */
-                       leaf_part_rri = &leaf_part_arr[i];
-               }
-
-               /*
-                * If we didn't open the partition rel, it means we haven't
-                * initialized the result rel either.
-                */
-               if (!partrel)
-               {
-                       /*
-                        * We locked all the partitions above including the leaf
-                        * partitions. Note that each of the newly opened 
relations in
-                        * proute->partitions are eventually closed by the 
caller.
-                        */
-                       partrel = heap_open(leaf_oid, NoLock);
-                       InitResultRelInfo(leaf_part_rri,
-                                                         partrel,
-                                                         resultRTindex,
-                                                         rel,
-                                                         
estate->es_instrument);
-               }
-
-               part_tupdesc = RelationGetDescr(partrel);
-
-               /*
-                * Save a tuple conversion map to convert a tuple routed to this
-                * partition from the parent's type to the partition's.
-                */
-               proute->parent_child_tupconv_maps[i] =
-                       convert_tuples_by_name(tupDesc, part_tupdesc,
-                                                                  
gettext_noop("could not convert row type"));
-
-               /*
-                * Verify result relation is a valid target for an INSERT.  An 
UPDATE
-                * of a partition-key becomes a DELETE+INSERT operation, so 
this check
-                * is still required when the operation is CMD_UPDATE.
-                */
-               CheckValidResultRel(leaf_part_rri, CMD_INSERT);
-
-               /*
-                * Open partition indices.  The user may have asked to check for
-                * conflicts within this leaf partition and do "nothing" 
instead of
-                * throwing an error.  Be prepared in that case by initializing 
the
-                * index information needed by ExecInsert() to perform 
speculative
-                * insertions.
-                */
-               if (leaf_part_rri->ri_RelationDesc->rd_rel->relhasindex &&
-                       leaf_part_rri->ri_IndexRelationDescs == NULL)
-                       ExecOpenIndices(leaf_part_rri,
-                                                       mtstate != NULL &&
-                                                       mtstate->mt_onconflict 
!= ONCONFLICT_NONE);
-
-               estate->es_leaf_result_relations =
-                       lappend(estate->es_leaf_result_relations, 
leaf_part_rri);
-
-               proute->partitions[i] = leaf_part_rri;
-               i++;
-       }
-
        /*
         * For UPDATE, we should have found all the per-subplan resultrels in 
the
         * leaf partitions.
@@ -345,6 +261,244 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, 
PartitionDispatch *pd,
        return result;
 }
 
+static int
+leafpart_index_cmp(const void *arg1, const void *arg2)
+{
+       int             leafidx1 = *(const int *) arg1;
+       int             leafidx2 = *(const int *) arg2;
+
+       if (leafidx1 > leafidx2)
+               return 1;
+       else if (leafidx1 < leafidx2)
+               return -1;
+       return 0;
+}
+
+/*
+ * ExecInitPartitionInfo
+ *             Initialize ResultRelInfo and other information for a partition 
if not
+ *             already done
+ */
+void
+ExecInitPartitionInfo(ModifyTableState *mtstate,
+                                         ResultRelInfo *resultRelInfo,
+                                         PartitionTupleRouting *proute,
+                                         EState *estate, int partidx)
+{
+       Relation        partrel,
+                               rootrel;
+       ResultRelInfo *leaf_part_rri;
+
+       /* Nothing to do if already set. */
+       if (proute->partitions[partidx])
+               return;
+
+       leaf_part_rri = NULL;
+       rootrel = resultRelInfo->ri_RelationDesc;
+
+       /*
+        * If we are doing tuple routing for update, try to reuse the
+        * per-subplan resultrel for this partition that ExecInitModifyTable()
+        * might already have created.
+        */
+       if (mtstate && mtstate->operation == CMD_UPDATE)
+       {
+               int   *partidx_entry;
+
+               /*
+                * If the partition's ResultRelInfo has already been created, 
we'd
+                * find its index in proute->subplan_partition_offsets.
+                */
+               partidx_entry = (int *) bsearch(&partidx,
+                                                                               
proute->subplan_partition_offsets,
+                                                                               
mtstate->mt_nplans, sizeof(int),
+                                                                               
leafpart_index_cmp);
+               if (partidx_entry)
+               {
+                       int             update_rri_index =
+                                                       partidx_entry - 
proute->subplan_partition_offsets;
+
+                       Assert (update_rri_index < mtstate->mt_nplans);
+                       leaf_part_rri = 
&mtstate->resultRelInfo[update_rri_index];
+                       partrel = leaf_part_rri->ri_RelationDesc;
+
+                       /*
+                        * This is required in order to we convert the 
partition's
+                        * tuple to be compatible with the root partitioned 
table's
+                        * tuple descriptor.  When generating the per-subplan 
result
+                        * rels, this was not set.
+                        */
+                       leaf_part_rri->ri_PartitionRoot = rootrel;
+               }
+       }
+
+       /*
+        * Create new result rel, either if we are *inserting* the new tuple, or
+        * if we didn't find the result rel above for the update tuple routing
+        * case.
+        */
+       if (leaf_part_rri == NULL)
+       {
+               int                     firstVarno;
+               Relation        firstResultRel;
+               ModifyTable *node = NULL;
+
+               if (mtstate)
+               {
+                       node = (ModifyTable *) mtstate->ps.plan;
+                       firstVarno = 
mtstate->resultRelInfo[0].ri_RangeTableIndex;
+                       firstResultRel = 
mtstate->resultRelInfo[0].ri_RelationDesc;
+               }
+
+               leaf_part_rri = (ResultRelInfo *) 
palloc0(sizeof(ResultRelInfo));
+
+               /*
+                * We locked all the partitions in 
ExecSetupPartitionTupleRouting
+                * including the leaf partitions.
+                */
+               partrel = heap_open(proute->partition_oids[partidx], NoLock);
+               InitResultRelInfo(leaf_part_rri,
+                                                 partrel,
+                                                 node ? node->nominalRelation 
: 1,
+                                                 rootrel,
+                                                 estate->es_instrument);
+
+               /*
+                * Build WITH CHECK OPTION constraints for this partition rel.  
Note
+                * that we didn't build the withCheckOptionList for each 
partition
+                * within the planner, but simple translation of the varattnos 
will
+                * suffice.  This only occurs for the INSERT case or in the 
case of
+                * UPDATE for which we didn't find a result rel above to reuse.
+                */
+               if (node && node->withCheckOptionLists != NIL)
+               {
+                       List       *wcoList;
+                       List       *mapped_wcoList;
+                       List       *wcoExprs = NIL;
+                       ListCell   *ll;
+
+                       /*
+                        * In the case of INSERT on partitioned tables, there 
is only one
+                        * plan.  Likewise, there is only one WCO list, not one 
per
+                        * partition.  For UPDATE, there would be as many WCO 
lists as
+                        * there are plans, but we use the first one as 
reference.  Note
+                        * that if there are SubPlans in there, they all end up 
attached
+                        * to the one parent Plan node.
+                        */
+                       Assert((node->operation == CMD_INSERT &&
+                                       list_length(node->withCheckOptionLists) 
== 1 &&
+                                       list_length(node->plans) == 1) ||
+                                  (node->operation == CMD_UPDATE &&
+                                       list_length(node->withCheckOptionLists) 
==
+                                       list_length(node->plans)));
+                       wcoList = linitial(node->withCheckOptionLists);
+
+                       mapped_wcoList = map_partition_varattnos(wcoList,
+                                                                               
                         firstVarno,
+                                                                               
                         partrel, firstResultRel,
+                                                                               
                         NULL);
+                       foreach(ll, mapped_wcoList)
+                       {
+                               WithCheckOption *wco = 
castNode(WithCheckOption, lfirst(ll));
+                               ExprState  *wcoExpr = 
ExecInitQual(castNode(List, wco->qual),
+                                                                               
                   mtstate->mt_plans[0]);
+                               wcoExprs = lappend(wcoExprs, wcoExpr);
+                       }
+
+                       leaf_part_rri->ri_WithCheckOptions = mapped_wcoList;
+                       leaf_part_rri->ri_WithCheckOptionExprs = wcoExprs;
+               }
+
+               /*
+                * Build the RETURNING projection if any for the partition.  
Note that
+                * we didn't build the returningList for each partition within 
the
+                * planner, but simple translation of the varattnos will 
suffice.
+                * This only occurs for the INSERT case; in the UPDATE/DELETE 
cases,
+                * ExecInitModifyTable() would've initialized this.
+                */
+               if (node && node->returningLists != NIL)
+               {
+                       TupleTableSlot *slot;
+                       ExprContext *econtext;
+                       List       *returningList;
+                       List       *rlist;
+                       TupleDesc       tupDesc;
+
+                       /* See the comment written above for WCO lists. */
+                       Assert((node->operation == CMD_INSERT &&
+                                       list_length(node->returningLists) == 1 
&&
+                                       list_length(node->plans) == 1) ||
+                                  (node->operation == CMD_UPDATE &&
+                                       list_length(node->returningLists) ==
+                                       list_length(node->plans)));
+                       returningList = linitial(node->returningLists);
+
+                       /*
+                        * Initialize result tuple slot and assign its rowtype 
using the first
+                        * RETURNING list.  We assume the rest will look the 
same.
+                        */
+                       tupDesc = ExecTypeFromTL(returningList, false);
+
+                       /* Set up a slot for the output of the RETURNING 
projection(s) */
+                       ExecInitResultTupleSlot(estate, &mtstate->ps);
+                       ExecAssignResultType(&mtstate->ps, tupDesc);
+                       slot = mtstate->ps.ps_ResultTupleSlot;
+
+                       /* Need an econtext too */
+                       if (mtstate->ps.ps_ExprContext == NULL)
+                               ExecAssignExprContext(estate, &mtstate->ps);
+                       econtext = mtstate->ps.ps_ExprContext;
+
+                       rlist = map_partition_varattnos(returningList,
+                                                                               
        firstVarno,
+                                                                               
        partrel, firstResultRel, NULL);
+                       leaf_part_rri->ri_projectReturning =
+                               ExecBuildProjectionInfo(rlist, econtext, slot, 
&mtstate->ps,
+                                                                               
RelationGetDescr(partrel));
+               }
+
+               /*
+                * Open partition indices.  The user may have asked to check for
+                * conflicts within this leaf partition and do "nothing" 
instead of
+                * throwing an error.  Be prepared in that case by initializing 
the
+                * index information needed by ExecInsert() to perform 
speculative
+                * insertions.
+                */
+               if (partrel->rd_rel->relhasindex &&
+                       leaf_part_rri->ri_IndexRelationDescs == NULL)
+                       ExecOpenIndices(leaf_part_rri,
+                                                       mtstate != NULL &&
+                                                       mtstate->mt_onconflict 
!= ONCONFLICT_NONE);
+       }
+
+       /*
+        * Verify result relation is a valid target for an INSERT.  An UPDATE
+        * of a partition-key becomes a DELETE+INSERT operation, so this check
+        * is still required when the operation is CMD_UPDATE.
+        */
+       CheckValidResultRel(leaf_part_rri, CMD_INSERT);
+
+       proute->partitions[partidx] = leaf_part_rri;
+
+       /*
+        * Note that the entries in this list appear in no predetermined order,
+        * because partition result rels are initialized as and when they're
+        * needed.
+        */
+       estate->es_leaf_result_relations =
+                                                               
lappend(estate->es_leaf_result_relations,
+                                                                               
leaf_part_rri);
+
+       /*
+        * Save a tuple conversion map to convert a tuple routed to this
+        * partition from the parent's type to the partition's.
+        */
+       proute->parent_child_tupconv_maps[partidx] =
+                                                       
convert_tuples_by_name(RelationGetDescr(rootrel),
+                                                                               
                   RelationGetDescr(partrel),
+                                                                               
           gettext_noop("could not convert row type"));
+}
+
 /*
  * ExecSetupChildParentMapForLeaf -- Initialize the per-leaf-partition
  * child-to-root tuple conversion map array.
@@ -538,8 +692,11 @@ ExecCleanupTupleRouting(PartitionTupleRouting *proute)
                        continue;
                }
 
-               ExecCloseIndices(resultRelInfo);
-               heap_close(resultRelInfo->ri_RelationDesc, NoLock);
+               if (resultRelInfo)
+               {
+                       ExecCloseIndices(resultRelInfo);
+                       heap_close(resultRelInfo->ri_RelationDesc, NoLock);
+               }
        }
 
        /* Release the standalone partition tuple descriptors, if any */
diff --git a/src/backend/executor/nodeModifyTable.c 
b/src/backend/executor/nodeModifyTable.c
index d054da5330..15ca3281c9 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -304,12 +304,17 @@ ExecInsert(ModifyTableState *mtstate,
                Assert(leaf_part_index >= 0 &&
                           leaf_part_index < proute->num_partitions);
 
+               /* Initialize partition info, if not done already. */
+               ExecInitPartitionInfo(mtstate, resultRelInfo, proute, estate,
+                                                         leaf_part_index);
+
                /*
                 * Save the old ResultRelInfo and switch to the one 
corresponding to
                 * the selected partition.
                 */
                saved_resultRelInfo = resultRelInfo;
                resultRelInfo = proute->partitions[leaf_part_index];
+               Assert(resultRelInfo != NULL);
 
                /* We do not yet have a way to insert into a foreign partition 
*/
                if (resultRelInfo->ri_FdwRoutine)
@@ -2081,14 +2086,10 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, 
int eflags)
        ResultRelInfo *saved_resultRelInfo;
        ResultRelInfo *resultRelInfo;
        Plan       *subplan;
-       int                     firstVarno = 0;
-       Relation        firstResultRel = NULL;
        ListCell   *l;
        int                     i;
        Relation        rel;
        bool            update_tuple_routing_needed = node->partColsUpdated;
-       PartitionTupleRouting *proute = NULL;
-       int                     num_partitions = 0;
 
        /* check for unsupported flags */
        Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK)));
@@ -2211,20 +2212,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, 
int eflags)
         */
        if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
                (operation == CMD_INSERT || update_tuple_routing_needed))
-       {
-               proute = mtstate->mt_partition_tuple_routing =
-                       ExecSetupPartitionTupleRouting(mtstate,
-                                                                               
   rel, node->nominalRelation,
-                                                                               
   estate);
-               num_partitions = proute->num_partitions;
-
-               /*
-                * Below are required as reference objects for mapping partition
-                * attno's in expressions such as WithCheckOptions and 
RETURNING.
-                */
-               firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex;
-               firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc;
-       }
+               mtstate->mt_partition_tuple_routing =
+                                               
ExecSetupPartitionTupleRouting(mtstate, rel);
 
        /*
         * Build state for collecting transition tuples.  This requires having a
@@ -2271,77 +2260,12 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, 
int eflags)
        }
 
        /*
-        * Build WITH CHECK OPTION constraints for each leaf partition rel. Note
-        * that we didn't build the withCheckOptionList for each partition 
within
-        * the planner, but simple translation of the varattnos for each 
partition
-        * will suffice.  This only occurs for the INSERT case or for UPDATE row
-        * movement. DELETEs and local UPDATEs are handled above.
-        */
-       if (node->withCheckOptionLists != NIL && num_partitions > 0)
-       {
-               List       *first_wcoList;
-
-               /*
-                * In case of INSERT on partitioned tables, there is only one 
plan.
-                * Likewise, there is only one WITH CHECK OPTIONS list, not one 
per
-                * partition. Whereas for UPDATE, there are as many WCOs as 
there are
-                * plans. So in either case, use the WCO expression of the first
-                * resultRelInfo as a reference to calculate attno's for the WCO
-                * expression of each of the partitions. We make a copy of the 
WCO
-                * qual for each partition. Note that, if there are SubPlans in 
there,
-                * they all end up attached to the one parent Plan node.
-                */
-               Assert(update_tuple_routing_needed ||
-                          (operation == CMD_INSERT &&
-                               list_length(node->withCheckOptionLists) == 1 &&
-                               mtstate->mt_nplans == 1));
-
-               first_wcoList = linitial(node->withCheckOptionLists);
-               for (i = 0; i < num_partitions; i++)
-               {
-                       Relation        partrel;
-                       List       *mapped_wcoList;
-                       List       *wcoExprs = NIL;
-                       ListCell   *ll;
-
-                       resultRelInfo = proute->partitions[i];
-
-                       /*
-                        * If we are referring to a resultRelInfo from one of 
the update
-                        * result rels, that result rel would already have
-                        * WithCheckOptions initialized.
-                        */
-                       if (resultRelInfo->ri_WithCheckOptions)
-                               continue;
-
-                       partrel = resultRelInfo->ri_RelationDesc;
-
-                       mapped_wcoList = map_partition_varattnos(first_wcoList,
-                                                                               
                         firstVarno,
-                                                                               
                         partrel, firstResultRel,
-                                                                               
                         NULL);
-                       foreach(ll, mapped_wcoList)
-                       {
-                               WithCheckOption *wco = 
castNode(WithCheckOption, lfirst(ll));
-                               ExprState  *wcoExpr = 
ExecInitQual(castNode(List, wco->qual),
-                                                                               
                   &mtstate->ps);
-
-                               wcoExprs = lappend(wcoExprs, wcoExpr);
-                       }
-
-                       resultRelInfo->ri_WithCheckOptions = mapped_wcoList;
-                       resultRelInfo->ri_WithCheckOptionExprs = wcoExprs;
-               }
-       }
-
-       /*
         * Initialize RETURNING projections if needed.
         */
        if (node->returningLists)
        {
                TupleTableSlot *slot;
                ExprContext *econtext;
-               List       *firstReturningList;
 
                /*
                 * Initialize result tuple slot and assign its rowtype using 
the first
@@ -2372,44 +2296,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, 
int eflags)
                                                                                
resultRelInfo->ri_RelationDesc->rd_att);
                        resultRelInfo++;
                }
-
-               /*
-                * Build a projection for each leaf partition rel.  Note that we
-                * didn't build the returningList for each partition within the
-                * planner, but simple translation of the varattnos for each 
partition
-                * will suffice.  This only occurs for the INSERT case or for 
UPDATE
-                * row movement. DELETEs and local UPDATEs are handled above.
-                */
-               firstReturningList = linitial(node->returningLists);
-               for (i = 0; i < num_partitions; i++)
-               {
-                       Relation        partrel;
-                       List       *rlist;
-
-                       resultRelInfo = proute->partitions[i];
-
-                       /*
-                        * If we are referring to a resultRelInfo from one of 
the update
-                        * result rels, that result rel would already have a 
returningList
-                        * built.
-                        */
-                       if (resultRelInfo->ri_projectReturning)
-                               continue;
-
-                       partrel = resultRelInfo->ri_RelationDesc;
-
-                       /*
-                        * Use the returning expression of the first 
resultRelInfo as a
-                        * reference to calculate attno's for the returning 
expression of
-                        * each of the partitions.
-                        */
-                       rlist = map_partition_varattnos(firstReturningList,
-                                                                               
        firstVarno,
-                                                                               
        partrel, firstResultRel, NULL);
-                       resultRelInfo->ri_projectReturning =
-                               ExecBuildProjectionInfo(rlist, econtext, slot, 
&mtstate->ps,
-                                                                               
resultRelInfo->ri_RelationDesc->rd_att);
-               }
        }
        else
        {
diff --git a/src/include/executor/execPartition.h 
b/src/include/executor/execPartition.h
index a75a37060a..4a62b9f1ff 100644
--- a/src/include/executor/execPartition.h
+++ b/src/include/executor/execPartition.h
@@ -91,6 +91,7 @@ typedef struct PartitionTupleRouting
 {
        PartitionDispatch *partition_dispatch_info;
        int                     num_dispatch;
+       Oid                *partition_oids;
        ResultRelInfo **partitions;
        int                     num_partitions;
        TupleConversionMap **parent_child_tupconv_maps;
@@ -103,12 +104,15 @@ typedef struct PartitionTupleRouting
 } PartitionTupleRouting;
 
 extern PartitionTupleRouting *ExecSetupPartitionTupleRouting(ModifyTableState 
*mtstate,
-                                                          Relation rel, Index 
resultRTindex,
-                                                          EState *estate);
+                                                          Relation rel);
 extern int ExecFindPartition(ResultRelInfo *resultRelInfo,
                                  PartitionDispatch *pd,
                                  TupleTableSlot *slot,
                                  EState *estate);
+extern void ExecInitPartitionInfo(ModifyTableState *mtstate,
+                                       ResultRelInfo *resultRelInfo,
+                                       PartitionTupleRouting *proute,
+                                       EState *estate, int partidx);
 extern void ExecSetupChildParentMapForLeaf(PartitionTupleRouting *proute);
 extern TupleConversionMap *TupConvMapForLeaf(PartitionTupleRouting *proute,
                                  ResultRelInfo *rootRelInfo, int leaf_index);
-- 
2.11.0

Reply via email to