Re: Speeding up INSERTs and UPDATEs to partitioned tables

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Jesper Pedersen <jesper(dot)pedersen(at)redhat(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Speeding up INSERTs and UPDATEs to partitioned tables
Date: 2018-11-14 04:30:27
Message-ID: 44bb0f2f-fa68-cfdc-f474-5f27b0759451@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for updating the patch.

On 2018/11/14 13:16, David Rowley wrote:
> Thanks for looking at this again.
>
> On 14 November 2018 at 13:47, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> + 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?
>
> I don't really want to put that code in ExecCheckPartitionArraySpace()
> as the way the function is now, it makes quite a lot of sense for the
> compiler to inline it. If we add redundant work in there, then it
> makes less sense. There's never any need to check both arrays at once
> as we're only adding the new item to one array at a time.
>
> Instead, I've written a new function named
> ExecCheckDispatchArraySpace() and put the resize code inside that.

Okay, seems fine.

> I've fixed the typos you mentioned. The only other thing I changed was
> to only allocate the PartitionDispatch->tupslot if a conversion is
> required. The previous code allocated this regardless if it was going
> to be used or not. This saves both the redundant allocation and also
> very slightly reduces the cost of the if test in ExecFindPartition().
> There's now no need to check if the map ! NULL as if the slot is there

Also makes sense.

Although it seems that Alvaro has already started at looking at this, I'll
mark the CF entry as Ready for Committer anyway, because I don't have any
more comments. :)

Thanks,
Amit

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kuntal Ghosh 2018-11-14 04:45:43 In-place updates and serializable transactions
Previous Message David Rowley 2018-11-14 04:16:02 Re: Speeding up INSERTs and UPDATEs to partitioned tables