(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

Reply via email to