Re: Speeding up INSERTs and UPDATEs to partitioned tables - Mailing list pgsql-hackers

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_f8@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