Re: non-bulk inserts and tuple routing

(2018/02/07 19:36), Etsuro Fujita wrote:
> (2018/02/05 19:43), Etsuro Fujita wrote:
>> (2018/02/05 14:34), Amit Langote wrote:
>>> Here is the updated version that contains two patches as described
>>> above.

Here are some minor comments:

o On changes to ExecInsert

* This might be just my taste, but I think it would be better to (1) 
change ExecInitPartitionInfo so that it creates and returns a 
newly-initialized ResultRelInfo for an initialized partition, and (2) 
rewrite this bit:

+       /* 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);

to something like this (I would say the same thing to the copy.c changes):

     /*
      * Save the old ResultRelInfo and switch to the one corresponding to
      * the selected partition.
      */
     saved_resultRelInfo = resultRelInfo;
     resultRelInfo = proute->partitions[leaf_part_index];
     if (resultRelInfo == NULL);
     {
         /* Initialize partition info. */
         resultRelInfo = ExecInitPartitionInfo(mtstate,
                                               saved_resultRelInfo,
                                               proute,
                                               estate,
                                               leaf_part_index);
     }

This would make ExecInitPartitionInfo more simple because it can assume 
that the given partition has not been initialized yet.

o On changes to execPartition.h

* Please add a brief decsription about partition_oids to the comments 
for this struct.

@@ -91,6 +91,7 @@ typedef struct PartitionTupleRouting
  {
     PartitionDispatch *partition_dispatch_info;
     int         num_dispatch;
+   Oid        *partition_oids;

That's it.

Best regards,
Etsuro Fujita