On 2018/11/14 0:32, Jesper Pedersen wrote: > Hi, > > On 11/12/18 6:17 PM, David Rowley wrote: >> On 9 November 2018 at 19:18, Amit Langote >> <langote_amit...@lab.ntt.co.jp> wrote: >>> I have a comment regarding how you chose to make >>> PartitionTupleRouting private. >>> >>> Using the v14_to_v15 diff, I could quickly see that there are many diffs >>> changing PartitionTupleRouting to struct PartitionTupleRouting, but they >>> would be unnecessary if you had added the following in execPartition.h, as >>> my upthread had done. >>> >>> -/* See execPartition.c for the definition. */ >>> +/* See execPartition.c for the definitions. */ >>> typedef struct PartitionDispatchData *PartitionDispatch; >>> +typedef struct PartitionTupleRouting PartitionTupleRouting; >> >> Okay, done that way. v16 attached.
Thank you. > Still passes check-world. I looked at v16 and noticed a few typos: + * partition_dispatch_info Array of 'dispatch_allocsize' elements containing + * a pointer to a PartitionDispatch objects for a PartitionDispatch objects -> a PartitionDispatch object + * partitions Array of 'partitions_allocsize' elements + * containing pointers to a ResultRelInfos of all + * leaf partitions touched by tuple routing. a ResultRelInfos -> ResultRelInfos + * PartitionDispatch and ResultRelInfo pointers the 'partitions' array. "in" the 'partitions' array. + /* Setup the PartitionRoutingInfo for it */ Setup -> Set up + * Ensure there's enough space in the 'partitions' array of 'proute' + * and store it in the next empty slot in proute's partitions array. Not a typo, but maybe just write proute->partitions instead of "partitions array of proute" and "proute's partition array". + * the next available slot in the 'proute' partition_dispatch_info[] + * array. Also, record the index into this array in the 'parent_pd' Similarly, here: proute->partition_dipatch_info array + * array. Also, record the index into this array in the 'parent_pd' + * indexes[] array in the partidx element so that we can properly Similarly, parent_pd->indexes array + if (dispatchidx >= proute->dispatch_allocsize) + { + /* Expand allocated space. */ + proute->dispatch_allocsize *= 2; + proute->partition_dispatch_info = (PartitionDispatchData **) + repalloc(proute->partition_dispatch_info, + sizeof(PartitionDispatchData *) * + proute->dispatch_allocsize); + } Sorry, I forgot to point this out before, but can this code in ExecInitPartitionDispatchInfo be accommodated in ExecCheckPartitionArraySpace() for consistency? Thanks, Amit