Fujita-san,

Thanks for the updates and sorry I couldn't reply sooner.

On 2018/03/06 21:26, Etsuro Fujita wrote:
> One thing I notice while working on this is this in ExecInsert/CopyFrom,
> which I moved to ExecPrepareTupleRouting as-is for the former:
>
>     /*
>      * If we're capturing transition tuples, we might need to convert from
> the
>      * partition rowtype to parent rowtype.
>      */
>     if (mtstate->mt_transition_capture != NULL)
>     {
>         if (resultRelInfo->ri_TrigDesc &&
>             (resultRelInfo->ri_TrigDesc->trig_insert_before_row ||
>              resultRelInfo->ri_TrigDesc->trig_insert_instead_row))
>         {
>             /*
>              * If there are any BEFORE or INSTEAD triggers on the partition,
>              * we'll have 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 =
>                 TupConvMapForLeaf(proute, rootRelInfo, leaf_part_index);
>         }
>
> Do we need to consider INSTEAD triggers here?  The partition is either a
> plain table or a foreign table, so I don't think it can have those
> triggers.  Am I missing something?

I think you're right.  We don't need to consider INSTEAD OF triggers here
if we know we're dealing with a partition which cannot have those.

Just to make sure, a word from Thomas would help.

On 2018/03/12 12:25, Etsuro Fujita wrote:
> (2018/03/09 20:18), Etsuro Fujita wrote:
>> Here are updated patches for PG10 and HEAD.
>>
>> Other changes:
>> * Add regression tests based on your test cases shown upthread
> 
> I added a little bit more regression tests and revised comments.  Please
> find attached an updated patch.

Thanks for adding the test.

I was concerned that ExecMaterializeSlot will be called twice now -- first
in ExecPrepareTupleRouting and then in ExecInsert -- instead of only once
in ExecInsert with the existing code, but perhaps it doesn't matter much
because most of the work would be done in the 1st call anyway.

Sorry that this may be nitpicking that I should've brought up before, but
doesn't ExecPrepareTupleRouting do all the work that's needed for routing
a tuple and hence isn't the name a bit misleading?  Maybe,
ExecPerformTupleRouting?

Btw, I noticed that the patches place ExecPrepareTupleRouting (both the
declaration and the definition) at different relative locations within
nodeModifyTable.c in the HEAD branch vs. in the 10 branch.  It might be a
good idea to bring them to the same relative location to avoid hassle when
back-patching relevant patches in the future.  I did that in the attached
updated version (v4) of the patch for HEAD, which does not make any other
changes.  Although, the patch for PG-10 is unchanged, I still changed its
file name to contain v4.

Regards,
Amit
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
***************
*** 2656,2668 **** CopyFrom(CopyState cstate)
                        if (cstate->transition_capture != NULL)
                        {
                                if (resultRelInfo->ri_TrigDesc &&
!                                       
(resultRelInfo->ri_TrigDesc->trig_insert_before_row ||
!                                        
resultRelInfo->ri_TrigDesc->trig_insert_instead_row))
                                {
                                        /*
!                                        * If there are any BEFORE or INSTEAD 
triggers on the
!                                        * partition, we'll have to be ready to 
convert their
!                                        * result back to tuplestore format.
                                         */
                                        
cstate->transition_capture->tcs_original_insert_tuple = NULL;
                                        cstate->transition_capture->tcs_map =
--- 2656,2667 ----
                        if (cstate->transition_capture != NULL)
                        {
                                if (resultRelInfo->ri_TrigDesc &&
!                                       
resultRelInfo->ri_TrigDesc->trig_insert_before_row)
                                {
                                        /*
!                                        * If there are any BEFORE triggers on 
the partition,
!                                        * we'll have to be ready to convert 
their result back to
!                                        * tuplestore format.
                                         */
                                        
cstate->transition_capture->tcs_original_insert_tuple = NULL;
                                        cstate->transition_capture->tcs_map =
***************
*** 2803,2814 **** CopyFrom(CopyState cstate)
                         * tuples inserted by an INSERT command.
                         */
                        processed++;
  
!                       if (saved_resultRelInfo)
!                       {
!                               resultRelInfo = saved_resultRelInfo;
!                               estate->es_result_relation_info = resultRelInfo;
!                       }
                }
        }
  
--- 2802,2814 ----
                         * tuples inserted by an INSERT command.
                         */
                        processed++;
+               }
  
!               /* Restore the saved ResultRelInfo */
!               if (saved_resultRelInfo)
!               {
!                       resultRelInfo = saved_resultRelInfo;
!                       estate->es_result_relation_info = resultRelInfo;
                }
        }
  
*** a/src/backend/executor/nodeModifyTable.c
--- b/src/backend/executor/nodeModifyTable.c
***************
*** 62,67 **** static bool ExecOnConflictUpdate(ModifyTableState *mtstate,
--- 62,71 ----
                                         EState *estate,
                                         bool canSetTag,
                                         TupleTableSlot **returning);
+ static TupleTableSlot *ExecPrepareTupleRouting(ModifyTableState *mtstate,
+                                               EState *estate,
+                                               ResultRelInfo *targetRelInfo,
+                                               TupleTableSlot *slot);
  
  /*
   * Verify that the tuples to be produced by INSERT or UPDATE match the
***************
*** 259,265 **** ExecInsert(ModifyTableState *mtstate,
  {
        HeapTuple       tuple;
        ResultRelInfo *resultRelInfo;
-       ResultRelInfo *saved_resultRelInfo = NULL;
        Relation        resultRelationDesc;
        Oid                     newId;
        List       *recheckIndexes = NIL;
--- 263,268 ----
***************
*** 275,374 **** ExecInsert(ModifyTableState *mtstate,
         * get information on the (current) result relation
         */
        resultRelInfo = estate->es_result_relation_info;
- 
-       /* Determine the partition to heap_insert the tuple into */
-       if (mtstate->mt_partition_dispatch_info)
-       {
-               int                     leaf_part_index;
-               TupleConversionMap *map;
- 
-               /*
-                * Away we go ... If we end up not finding a partition after 
all,
-                * ExecFindPartition() does not return and errors out instead.
-                * Otherwise, the returned value is to be used as an index into 
arrays
-                * mt_partitions[] and mt_partition_tupconv_maps[] that will 
get us
-                * the ResultRelInfo and TupleConversionMap for the partition,
-                * respectively.
-                */
-               leaf_part_index = ExecFindPartition(resultRelInfo,
-                                                                               
        mtstate->mt_partition_dispatch_info,
-                                                                               
        slot,
-                                                                               
        estate);
-               Assert(leaf_part_index >= 0 &&
-                          leaf_part_index < mtstate->mt_num_partitions);
- 
-               /*
-                * Save the old ResultRelInfo and switch to the one 
corresponding to
-                * the selected partition.
-                */
-               saved_resultRelInfo = resultRelInfo;
-               resultRelInfo = mtstate->mt_partitions + leaf_part_index;
- 
-               /* We do not yet have a way to insert into a foreign partition 
*/
-               if (resultRelInfo->ri_FdwRoutine)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                        errmsg("cannot route inserted tuples 
to a foreign table")));
- 
-               /* For ExecInsertIndexTuples() to work on the partition's 
indexes */
-               estate->es_result_relation_info = resultRelInfo;
- 
-               /*
-                * If we're capturing transition tuples, we might need to 
convert from
-                * the partition rowtype to parent rowtype.
-                */
-               if (mtstate->mt_transition_capture != NULL)
-               {
-                       if (resultRelInfo->ri_TrigDesc &&
-                               
(resultRelInfo->ri_TrigDesc->trig_insert_before_row ||
-                                
resultRelInfo->ri_TrigDesc->trig_insert_instead_row))
-                       {
-                               /*
-                                * If there are any BEFORE or INSTEAD triggers 
on the
-                                * partition, we'll have 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 =
-                                       
mtstate->mt_transition_tupconv_maps[leaf_part_index];
-                       }
-                       else
-                       {
-                               /*
-                                * Otherwise, just remember the original 
unconverted tuple, to
-                                * avoid a needless round trip conversion.
-                                */
-                               
mtstate->mt_transition_capture->tcs_original_insert_tuple = tuple;
-                               mtstate->mt_transition_capture->tcs_map = NULL;
-                       }
-               }
-               if (mtstate->mt_oc_transition_capture != NULL)
-                       mtstate->mt_oc_transition_capture->tcs_map =
-                               
mtstate->mt_transition_tupconv_maps[leaf_part_index];
- 
-               /*
-                * We might need to convert from the parent rowtype to the 
partition
-                * rowtype.
-                */
-               map = mtstate->mt_partition_tupconv_maps[leaf_part_index];
-               if (map)
-               {
-                       Relation        partrel = 
resultRelInfo->ri_RelationDesc;
- 
-                       tuple = do_convert_tuple(tuple, map);
- 
-                       /*
-                        * We must use the partition's tuple descriptor from 
this point
-                        * on, until we're finished dealing with the partition. 
Use the
-                        * dedicated slot for that.
-                        */
-                       slot = mtstate->mt_partition_tuple_slot;
-                       Assert(slot != NULL);
-                       ExecSetSlotDescriptor(slot, RelationGetDescr(partrel));
-                       ExecStoreTuple(tuple, slot, InvalidBuffer, true);
-               }
-       }
- 
        resultRelationDesc = resultRelInfo->ri_RelationDesc;
  
        /*
--- 278,283 ----
***************
*** 477,483 **** ExecInsert(ModifyTableState *mtstate,
                 * No need though if the tuple has been routed, and a BR trigger
                 * doesn't exist.
                 */
!               if (saved_resultRelInfo != NULL &&
                        !(resultRelInfo->ri_TrigDesc &&
                          resultRelInfo->ri_TrigDesc->trig_insert_before_row))
                        check_partition_constr = false;
--- 386,392 ----
                 * No need though if the tuple has been routed, and a BR trigger
                 * doesn't exist.
                 */
!               if (resultRelInfo->ri_PartitionRoot != NULL &&
                        !(resultRelInfo->ri_TrigDesc &&
                          resultRelInfo->ri_TrigDesc->trig_insert_before_row))
                        check_partition_constr = false;
***************
*** 645,653 **** ExecInsert(ModifyTableState *mtstate,
        if (resultRelInfo->ri_projectReturning)
                result = ExecProcessReturning(resultRelInfo, slot, planSlot);
  
-       if (saved_resultRelInfo)
-               estate->es_result_relation_info = saved_resultRelInfo;
- 
        return result;
  }
  
--- 554,559 ----
***************
*** 1545,1550 **** ExecSetupTransitionCaptureState(ModifyTableState *mtstate, 
EState *estate)
--- 1451,1563 ----
        }
  }
  
+ /*
+  * ExecPrepareTupleRouting --- prepare for tuple routing of one tuple
+  *
+  * Returns a slot holding the routed tuple of the partition rowtype
+  */
+ static TupleTableSlot *
+ ExecPrepareTupleRouting(ModifyTableState *mtstate,
+                                               EState *estate,
+                                               ResultRelInfo *targetRelInfo,
+                                               TupleTableSlot *slot)
+ {
+       int                     leaf_part_index;
+       ResultRelInfo *resultRelInfo;
+       HeapTuple       tuple;
+       TupleConversionMap *map;
+ 
+       /*
+        * Away we go ... If we end up not finding a partition after all,
+        * ExecFindPartition() does not return and errors out instead.
+        * Otherwise, the returned value is to be used as an index into arrays
+        * mt_partitions[] and mt_partition_tupconv_maps[] that will get us
+        * the ResultRelInfo and TupleConversionMap for the partition,
+        * respectively.
+        */
+       leaf_part_index = ExecFindPartition(targetRelInfo,
+                                                                               
mtstate->mt_partition_dispatch_info,
+                                                                               
slot,
+                                                                               
estate);
+       Assert(leaf_part_index >= 0 &&
+                  leaf_part_index < mtstate->mt_num_partitions);
+ 
+       /* Get the ResultRelInfo corresponding to the selected partition. */
+       resultRelInfo = &mtstate->mt_partitions[leaf_part_index];
+ 
+       /* We do not yet have a way to insert into a foreign partition */
+       if (resultRelInfo->ri_FdwRoutine)
+               ereport(ERROR,
+                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                errmsg("cannot route inserted tuples to a 
foreign table")));
+ 
+       /*
+        * For ExecInsert(), make it look like we are inserting into the 
partition.
+        */
+       estate->es_result_relation_info = resultRelInfo;
+ 
+       /* Get the heap tuple out of the given slot. */
+       tuple = ExecMaterializeSlot(slot);
+ 
+       /*
+        * If we're capturing transition tuples, we might need to convert from 
the
+        * partition rowtype to parent rowtype.
+        */
+       if (mtstate->mt_transition_capture != NULL)
+       {
+               if (resultRelInfo->ri_TrigDesc &&
+                       resultRelInfo->ri_TrigDesc->trig_insert_before_row)
+               {
+                       /*
+                        * If there are any BEFORE triggers on the partition, 
we'll have
+                        * 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 =
+                               
mtstate->mt_transition_tupconv_maps[leaf_part_index];
+               }
+               else
+               {
+                       /*
+                        * Otherwise, just remember the original unconverted 
tuple, to
+                        * avoid a needless round trip conversion.
+                        */
+                       
mtstate->mt_transition_capture->tcs_original_insert_tuple = tuple;
+                       mtstate->mt_transition_capture->tcs_map = NULL;
+               }
+       }
+       if (mtstate->mt_oc_transition_capture != NULL)
+       {
+               mtstate->mt_oc_transition_capture->tcs_map =
+                       mtstate->mt_transition_tupconv_maps[leaf_part_index];
+       }
+ 
+       /*
+        * We might need to convert from the parent rowtype to the partition
+        * rowtype.
+        */
+       map = mtstate->mt_partition_tupconv_maps[leaf_part_index];
+       if (map)
+       {
+               Relation        partrel = resultRelInfo->ri_RelationDesc;
+ 
+               tuple = do_convert_tuple(tuple, map);
+ 
+               /*
+                * We must use the partition's tuple descriptor from this point 
on,
+                * until we're finished dealing with the partition.  Use the
+                * dedicated slot for that.
+                */
+               slot = mtstate->mt_partition_tuple_slot;
+               Assert(slot != NULL);
+               ExecSetSlotDescriptor(slot, RelationGetDescr(partrel));
+               ExecStoreTuple(tuple, slot, InvalidBuffer, true);
+       }
+ 
+       return slot;
+ }
+ 
+ 
  /* ----------------------------------------------------------------
   *       ExecModifyTable
   *
***************
*** 1763,1771 **** ExecModifyTable(PlanState *pstate)
--- 1776,1794 ----
                switch (operation)
                {
                        case CMD_INSERT:
+                               /* Prepare for tuple routing of the tuple if 
needed. */
+                               if (node->mt_partition_dispatch_info)
+                                       slot = ExecPrepareTupleRouting(node, 
estate,
+                                                                               
                   resultRelInfo, slot);
                                slot = ExecInsert(node, slot, planSlot,
                                                                  
node->mt_arbiterindexes, node->mt_onconflict,
                                                                  estate, 
node->canSetTag);
+                               /*
+                                * Revert back the active result relation that
+                                * ExecPrepareTupleRouting would have changed.
+                                */
+                               if (node->mt_partition_dispatch_info)
+                                       estate->es_result_relation_info = 
resultRelInfo;
                                break;
                        case CMD_UPDATE:
                                slot = ExecUpdate(node, tupleid, oldtuple, 
slot, planSlot,
*** a/src/test/regress/expected/insert.out
--- b/src/test/regress/expected/insert.out
***************
*** 554,559 **** drop role regress_coldesc_role;
--- 554,578 ----
  drop table inserttest3;
  drop table brtrigpartcon;
  drop function brtrigpartcon1trigf();
+ -- check that "do nothing" BR triggers work with tuple-routing
+ -- (this ensures that estate->es_result_relation_info is set properly
+ -- for every routed tuple)
+ create or replace function donothingbrtrig_func() returns trigger as $$begin 
return NULL; end$$ language plpgsql;
+ create table donothingbrtrig_test (a int, b text) partition by list (a);
+ create table donothingbrtrig_test1 (b text, a int);
+ create trigger donothingbrtrig before insert on donothingbrtrig_test1 for 
each row execute procedure donothingbrtrig_func();
+ alter table donothingbrtrig_test attach partition donothingbrtrig_test1 for 
values in (1);
+ create table donothingbrtrig_test2 partition of donothingbrtrig_test for 
values in (2);
+ insert into donothingbrtrig_test values (1, 'foo'), (2, 'bar');
+ select tableoid::regclass, * from donothingbrtrig_test;
+        tableoid        | a |  b  
+ -----------------------+---+-----
+  donothingbrtrig_test2 | 2 | bar
+ (1 row)
+ 
+ -- cleanup
+ drop table donothingbrtrig_test;
+ drop function donothingbrtrig_func();
  -- check multi-column range partitioning with minvalue/maxvalue constraints
  create table mcrparted (a text, b int) partition by range(a, b);
  create table mcrparted1_lt_b partition of mcrparted for values from 
(minvalue, minvalue) to ('b', minvalue);
*** a/src/test/regress/sql/insert.sql
--- b/src/test/regress/sql/insert.sql
***************
*** 378,383 **** drop table inserttest3;
--- 378,402 ----
  drop table brtrigpartcon;
  drop function brtrigpartcon1trigf();
  
+ -- check that "do nothing" BR triggers work with tuple-routing
+ -- (this ensures that estate->es_result_relation_info is set properly
+ -- for every routed tuple)
+ create or replace function donothingbrtrig_func() returns trigger as $$begin 
return NULL; end$$ language plpgsql;
+ 
+ create table donothingbrtrig_test (a int, b text) partition by list (a);
+ create table donothingbrtrig_test1 (b text, a int);
+ create trigger donothingbrtrig before insert on donothingbrtrig_test1 for 
each row execute procedure donothingbrtrig_func();
+ alter table donothingbrtrig_test attach partition donothingbrtrig_test1 for 
values in (1);
+ create table donothingbrtrig_test2 partition of donothingbrtrig_test for 
values in (2);
+ 
+ insert into donothingbrtrig_test values (1, 'foo'), (2, 'bar');
+ 
+ select tableoid::regclass, * from donothingbrtrig_test;
+ 
+ -- cleanup
+ drop table donothingbrtrig_test;
+ drop function donothingbrtrig_func();
+ 
  -- check multi-column range partitioning with minvalue/maxvalue constraints
  create table mcrparted (a text, b int) partition by range(a, b);
  create table mcrparted1_lt_b partition of mcrparted for values from 
(minvalue, minvalue) to ('b', minvalue);
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 4562a5121d..a42861da0d 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2633,13 +2633,12 @@ CopyFrom(CopyState cstate)
                        if (cstate->transition_capture != NULL)
                        {
                                if (resultRelInfo->ri_TrigDesc &&
-                                       
(resultRelInfo->ri_TrigDesc->trig_insert_before_row ||
-                                        
resultRelInfo->ri_TrigDesc->trig_insert_instead_row))
+                                       
resultRelInfo->ri_TrigDesc->trig_insert_before_row)
                                {
                                        /*
-                                        * If there are any BEFORE or INSTEAD 
triggers on the
-                                        * partition, we'll have to be ready to 
convert their
-                                        * result back to tuplestore format.
+                                        * If there are any BEFORE triggers on 
the partition,
+                                        * we'll have to be ready to convert 
their result back to
+                                        * tuplestore format.
                                         */
                                        
cstate->transition_capture->tcs_original_insert_tuple = NULL;
                                        cstate->transition_capture->tcs_map =
@@ -2768,12 +2767,13 @@ CopyFrom(CopyState cstate)
                         * tuples inserted by an INSERT command.
                         */
                        processed++;
+               }
 
-                       if (saved_resultRelInfo)
-                       {
-                               resultRelInfo = saved_resultRelInfo;
-                               estate->es_result_relation_info = resultRelInfo;
-                       }
+               /* Restore the saved ResultRelInfo */
+               if (saved_resultRelInfo)
+               {
+                       resultRelInfo = saved_resultRelInfo;
+                       estate->es_result_relation_info = resultRelInfo;
                }
        }
 
diff --git a/src/backend/executor/nodeModifyTable.c 
b/src/backend/executor/nodeModifyTable.c
index c32928d9bd..30af4b11eb 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -62,6 +62,11 @@ static bool ExecOnConflictUpdate(ModifyTableState *mtstate,
                                         EState *estate,
                                         bool canSetTag,
                                         TupleTableSlot **returning);
+static TupleTableSlot *ExecPrepareTupleRouting(ModifyTableState *mtstate,
+                                               PartitionTupleRouting *proute,
+                                               EState *estate,
+                                               ResultRelInfo *targetRelInfo,
+                                               TupleTableSlot *slot);
 static ResultRelInfo *getTargetResultRelInfo(ModifyTableState *node);
 static void ExecSetupChildParentMapForTcs(ModifyTableState *mtstate);
 static void ExecSetupChildParentMapForSubplan(ModifyTableState *mtstate);
@@ -265,7 +270,6 @@ ExecInsert(ModifyTableState *mtstate,
 {
        HeapTuple       tuple;
        ResultRelInfo *resultRelInfo;
-       ResultRelInfo *saved_resultRelInfo = NULL;
        Relation        resultRelationDesc;
        Oid                     newId;
        List       *recheckIndexes = NIL;
@@ -282,100 +286,6 @@ ExecInsert(ModifyTableState *mtstate,
         * get information on the (current) result relation
         */
        resultRelInfo = estate->es_result_relation_info;
-
-       /* Determine the partition to heap_insert the tuple into */
-       if (mtstate->mt_partition_tuple_routing)
-       {
-               int                     leaf_part_index;
-               PartitionTupleRouting *proute = 
mtstate->mt_partition_tuple_routing;
-
-               /*
-                * Away we go ... If we end up not finding a partition after 
all,
-                * ExecFindPartition() does not return and errors out instead.
-                * Otherwise, the returned value is to be used as an index into 
arrays
-                * proute->partitions[] and proute->partition_tupconv_maps[] 
that will
-                * get us the ResultRelInfo and TupleConversionMap for the 
partition,
-                * respectively.
-                */
-               leaf_part_index = ExecFindPartition(resultRelInfo,
-                                                                               
        proute->partition_dispatch_info,
-                                                                               
        slot,
-                                                                               
        estate);
-               Assert(leaf_part_index >= 0 &&
-                          leaf_part_index < proute->num_partitions);
-
-               /*
-                * Save the old ResultRelInfo and switch to the one 
corresponding to
-                * the selected partition.  (We might need to initialize it 
first.)
-                */
-               saved_resultRelInfo = resultRelInfo;
-               resultRelInfo = proute->partitions[leaf_part_index];
-               if (resultRelInfo == NULL)
-               {
-                       resultRelInfo = ExecInitPartitionInfo(mtstate,
-                                                                               
                  saved_resultRelInfo,
-                                                                               
                  proute, estate,
-                                                                               
                  leaf_part_index);
-                       Assert(resultRelInfo != NULL);
-               }
-
-               /* We do not yet have a way to insert into a foreign partition 
*/
-               if (resultRelInfo->ri_FdwRoutine)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                        errmsg("cannot route inserted tuples 
to a foreign table")));
-
-               /* For ExecInsertIndexTuples() to work on the partition's 
indexes */
-               estate->es_result_relation_info = resultRelInfo;
-
-               /*
-                * If we're capturing transition tuples, we might need to 
convert from
-                * the partition rowtype to parent rowtype.
-                */
-               if (mtstate->mt_transition_capture != NULL)
-               {
-                       if (resultRelInfo->ri_TrigDesc &&
-                               
(resultRelInfo->ri_TrigDesc->trig_insert_before_row ||
-                                
resultRelInfo->ri_TrigDesc->trig_insert_instead_row))
-                       {
-                               /*
-                                * If there are any BEFORE or INSTEAD triggers 
on the
-                                * partition, we'll have 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 =
-                                       TupConvMapForLeaf(proute, 
saved_resultRelInfo,
-                                                                         
leaf_part_index);
-                       }
-                       else
-                       {
-                               /*
-                                * Otherwise, just remember the original 
unconverted tuple, to
-                                * avoid a needless round trip conversion.
-                                */
-                               
mtstate->mt_transition_capture->tcs_original_insert_tuple = tuple;
-                               mtstate->mt_transition_capture->tcs_map = NULL;
-                       }
-               }
-               if (mtstate->mt_oc_transition_capture != NULL)
-               {
-                       mtstate->mt_oc_transition_capture->tcs_map =
-                               TupConvMapForLeaf(proute, saved_resultRelInfo,
-                                                                 
leaf_part_index);
-               }
-
-               /*
-                * We might need to convert from the parent rowtype to the 
partition
-                * rowtype.
-                */
-               tuple = 
ConvertPartitionTupleSlot(proute->parent_child_tupconv_maps[leaf_part_index],
-                                                                               
  tuple,
-                                                                               
  proute->partition_tuple_slot,
-                                                                               
  &slot);
-       }
-
        resultRelationDesc = resultRelInfo->ri_RelationDesc;
 
        /*
@@ -495,7 +405,7 @@ ExecInsert(ModifyTableState *mtstate,
                 * No need though if the tuple has been routed, and a BR trigger
                 * doesn't exist.
                 */
-               if (saved_resultRelInfo != NULL &&
+               if (resultRelInfo->ri_PartitionRoot != NULL &&
                        !(resultRelInfo->ri_TrigDesc &&
                          resultRelInfo->ri_TrigDesc->trig_insert_before_row))
                        check_partition_constr = false;
@@ -686,9 +596,6 @@ ExecInsert(ModifyTableState *mtstate,
        if (resultRelInfo->ri_projectReturning)
                result = ExecProcessReturning(resultRelInfo, slot, planSlot);
 
-       if (saved_resultRelInfo)
-               estate->es_result_relation_info = saved_resultRelInfo;
-
        return result;
 }
 
@@ -1209,22 +1116,24 @@ lreplace:;
                                                                                
          proute->root_tuple_slot,
                                                                                
          &slot);
 
-
-                       /*
-                        * For ExecInsert(), make it look like we are inserting 
into the
-                        * root.
-                        */
+                       /* Prepare for tuple routing of the tuple */
                        Assert(mtstate->rootResultRelInfo != NULL);
-                       estate->es_result_relation_info = 
mtstate->rootResultRelInfo;
+                       slot = ExecPrepareTupleRouting(mtstate, proute, estate,
+                                                                               
   mtstate->rootResultRelInfo, slot);
 
                        ret_slot = ExecInsert(mtstate, slot, planSlot, NULL,
                                                                  
ONCONFLICT_NONE, estate, canSetTag);
 
                        /*
-                        * Revert back the active result relation and the active
-                        * transition capture map that we changed above.
+                        * Revert back the active result relation that
+                        * ExecPrepareTupleRouting would have changed.
                         */
                        estate->es_result_relation_info = resultRelInfo;
+
+                       /*
+                        * Also, revert back the active transition capture map 
that we
+                        * changed above.
+                        */
                        if (mtstate->mt_transition_capture)
                        {
                                
mtstate->mt_transition_capture->tcs_original_insert_tuple = NULL;
@@ -1575,6 +1484,106 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
        return true;
 }
 
+/*
+ * ExecPrepareTupleRouting --- prepare for tuple routing of one tuple
+ *
+ * Returns a slot holding the routed tuple of the partition rowtype
+ */
+static TupleTableSlot *
+ExecPrepareTupleRouting(ModifyTableState *mtstate,
+                                               PartitionTupleRouting *proute,
+                                               EState *estate,
+                                               ResultRelInfo *targetRelInfo,
+                                               TupleTableSlot *slot)
+{
+       int                     leaf_part_index;
+       ResultRelInfo *resultRelInfo;
+       HeapTuple       tuple;
+
+       Assert(proute != NULL);
+
+       /*
+        * Away we go ... If we end up not finding a partition after all,
+        * ExecFindPartition() does not return and errors out instead.
+        * Otherwise, the returned value is to be used as an index into arrays
+        * proute->partitions[] and proute->partition_tupconv_maps[] that will
+        * get us the ResultRelInfo and TupleConversionMap for the partition,
+        * respectively.
+        */
+       leaf_part_index = ExecFindPartition(targetRelInfo,
+                                                                               
proute->partition_dispatch_info,
+                                                                               
slot,
+                                                                               
estate);
+       Assert(leaf_part_index >= 0 && leaf_part_index < 
proute->num_partitions);
+
+       /* Get the ResultRelInfo corresponding to the selected partition. */
+       resultRelInfo = proute->partitions[leaf_part_index];
+       if (resultRelInfo == NULL)
+       {
+               resultRelInfo = ExecInitPartitionInfo(mtstate, targetRelInfo,
+                                                                               
          proute, estate,
+                                                                               
          leaf_part_index);
+               Assert(resultRelInfo != NULL);
+       }
+
+       /* We do not yet have a way to insert into a foreign partition */
+       if (resultRelInfo->ri_FdwRoutine)
+               ereport(ERROR,
+                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                errmsg("cannot route inserted tuples to a 
foreign table")));
+
+       /*
+        * For ExecInsert(), make it look like we are inserting into the 
partition.
+        */
+       estate->es_result_relation_info = resultRelInfo;
+
+       /* Get the heap tuple out of the given slot. */
+       tuple = ExecMaterializeSlot(slot);
+
+       /*
+        * If we're capturing transition tuples, we might need to convert from 
the
+        * partition rowtype to parent rowtype.
+        */
+       if (mtstate->mt_transition_capture != NULL)
+       {
+               if (resultRelInfo->ri_TrigDesc &&
+                       resultRelInfo->ri_TrigDesc->trig_insert_before_row)
+               {
+                       /*
+                        * If there are any BEFORE triggers on the partition, 
we'll have
+                        * 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 =
+                               TupConvMapForLeaf(proute, targetRelInfo, 
leaf_part_index);
+               }
+               else
+               {
+                       /*
+                        * Otherwise, just remember the original unconverted 
tuple, to
+                        * avoid a needless round trip conversion.
+                        */
+                       
mtstate->mt_transition_capture->tcs_original_insert_tuple = tuple;
+                       mtstate->mt_transition_capture->tcs_map = NULL;
+               }
+       }
+       if (mtstate->mt_oc_transition_capture != NULL)
+       {
+               mtstate->mt_oc_transition_capture->tcs_map =
+                       TupConvMapForLeaf(proute, targetRelInfo, 
leaf_part_index);
+       }
+
+       /*
+        * We might need to convert from the parent rowtype to the partition
+        * rowtype.
+        */
+       
ConvertPartitionTupleSlot(proute->parent_child_tupconv_maps[leaf_part_index],
+                                                         tuple,
+                                                         
proute->partition_tuple_slot,
+                                                         &slot);
+
+       return slot;
+}
 
 /*
  * Process BEFORE EACH STATEMENT triggers
@@ -1835,6 +1844,7 @@ tupconv_map_for_subplan(ModifyTableState *mtstate, int 
whichplan)
        }
 }
 
+
 /* ----------------------------------------------------------------
  *        ExecModifyTable
  *
@@ -1846,6 +1856,7 @@ static TupleTableSlot *
 ExecModifyTable(PlanState *pstate)
 {
        ModifyTableState *node = castNode(ModifyTableState, pstate);
+       PartitionTupleRouting *proute = node->mt_partition_tuple_routing;
        EState     *estate = node->ps.state;
        CmdType         operation = node->operation;
        ResultRelInfo *saved_resultRelInfo;
@@ -2051,9 +2062,19 @@ ExecModifyTable(PlanState *pstate)
                switch (operation)
                {
                        case CMD_INSERT:
+                               /* Prepare for tuple routing of the tuple if 
needed. */
+                               if (proute)
+                                       slot = ExecPrepareTupleRouting(node, 
proute, estate,
+                                                                               
                   resultRelInfo, slot);
                                slot = ExecInsert(node, slot, planSlot,
                                                                  
node->mt_arbiterindexes, node->mt_onconflict,
                                                                  estate, 
node->canSetTag);
+                               /*
+                                * Revert back the active result relation that
+                                * ExecPrepareTupleRouting would have changed.
+                                */
+                               if (proute)
+                                       estate->es_result_relation_info = 
resultRelInfo;
                                break;
                        case CMD_UPDATE:
                                slot = ExecUpdate(node, tupleid, oldtuple, 
slot, planSlot,
diff --git a/src/test/regress/expected/insert.out 
b/src/test/regress/expected/insert.out
index dcbaad8e2f..cd668bde63 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -748,6 +748,25 @@ drop role regress_coldesc_role;
 drop table inserttest3;
 drop table brtrigpartcon;
 drop function brtrigpartcon1trigf();
+-- check that "do nothing" BR triggers work with tuple-routing
+-- (this ensures that estate->es_result_relation_info is set properly
+-- for every routed tuple)
+create or replace function donothingbrtrig_func() returns trigger as $$begin 
return NULL; end$$ language plpgsql;
+create table donothingbrtrig_test (a int, b text) partition by list (a);
+create table donothingbrtrig_test1 (b text, a int);
+create trigger donothingbrtrig before insert on donothingbrtrig_test1 for each 
row execute procedure donothingbrtrig_func();
+alter table donothingbrtrig_test attach partition donothingbrtrig_test1 for 
values in (1);
+create table donothingbrtrig_test2 partition of donothingbrtrig_test for 
values in (2);
+insert into donothingbrtrig_test values (1, 'foo'), (2, 'bar');
+select tableoid::regclass, * from donothingbrtrig_test;
+       tableoid        | a |  b  
+-----------------------+---+-----
+ donothingbrtrig_test2 | 2 | bar
+(1 row)
+
+-- cleanup
+drop table donothingbrtrig_test;
+drop function donothingbrtrig_func();
 -- check multi-column range partitioning with minvalue/maxvalue constraints
 create table mcrparted (a text, b int) partition by range(a, b);
 create table mcrparted1_lt_b partition of mcrparted for values from (minvalue, 
minvalue) to ('b', minvalue);
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 0150b6bb0f..7673d64f7f 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -489,6 +489,25 @@ drop table inserttest3;
 drop table brtrigpartcon;
 drop function brtrigpartcon1trigf();
 
+-- check that "do nothing" BR triggers work with tuple-routing
+-- (this ensures that estate->es_result_relation_info is set properly
+-- for every routed tuple)
+create or replace function donothingbrtrig_func() returns trigger as $$begin 
return NULL; end$$ language plpgsql;
+
+create table donothingbrtrig_test (a int, b text) partition by list (a);
+create table donothingbrtrig_test1 (b text, a int);
+create trigger donothingbrtrig before insert on donothingbrtrig_test1 for each 
row execute procedure donothingbrtrig_func();
+alter table donothingbrtrig_test attach partition donothingbrtrig_test1 for 
values in (1);
+create table donothingbrtrig_test2 partition of donothingbrtrig_test for 
values in (2);
+
+insert into donothingbrtrig_test values (1, 'foo'), (2, 'bar');
+
+select tableoid::regclass, * from donothingbrtrig_test;
+
+-- cleanup
+drop table donothingbrtrig_test;
+drop function donothingbrtrig_func();
+
 -- check multi-column range partitioning with minvalue/maxvalue constraints
 create table mcrparted (a text, b int) partition by range(a, b);
 create table mcrparted1_lt_b partition of mcrparted for values from (minvalue, 
minvalue) to ('b', minvalue);

Reply via email to