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: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Speeding up INSERTs and UPDATEs to partitioned tables |
Date: | 2018-08-22 07:08:46 |
Message-ID: | 79e4c67d-151a-ca08-5d19-e57ad382e911@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2018/08/21 14:44, David Rowley wrote:
> On 3 August 2018 at 17:58, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> On 2018/07/31 16:03, David Rowley wrote:
>>> Maybe we can do that as a follow-on patch.
>>
>> We probably could, but I think it would be a good idea get rid of *all*
>> redundant allocations due to tuple routing in one patch, if that's the
>> mission of this thread and the patch anyway.
>
> I started looking at this patch today and I now agree that it should
> be included in the main patch.
Great, thanks.
> I changed a few things with the patch. For example, the map access
> macros you'd defined were not in CamelCase.
In the updated patch:
+#define PartitionTupRoutingGetToParentMap(p, i) \
+#define PartitionTupRoutingGetToChildMap(p, i) \
If the "Get" could be replaced by "Child" and "Parent", respectively,
they'd sound more meaningful, imho.
> I also fixed a bug where
> the child to parent map was not being initialised when on conflict
> transition capture was required. I added a test which was crashing the
> backend but fixed the code so it works correctly.
Oops, I guess you mean my omission of checking if
mtstate->mt_oc_transition_capture is non-NULL in ExecInitRoutingInfo.
Thanks for fixing it and adding the test case.
> I also got rid of
> the child_parent_map_not_required array since we now no longer need
> it. The code now always initialises the maps in cases where they're
> going to be required.
Yes, thought I had removed the field in my patch, but looks like I had
just removed the comment about it.
> I've attached a v3 version of your patch and also v6 of the main patch
> which includes the v3 patch.
I've looked at v6 and spotted some minor typos.
+ * ResultRelInfo for, before we go making one, we check for a
pre-made one
s/making/make/g
+ /* If nobody else set the per-subplan array of maps, do so ouselves. */
I guess I'm the one to blame here for misspelling "ourselves".
Since the above two are minor issues, fixed them myself in the attached
updated version; didn't touch the macro though.
Do you agree to setting this patch to "Ready for Committer" in the
September CF?
Thanks,
Amit
Attachment | Content-Type | Size |
---|---|---|
v7-0001-Speed-up-INSERT-and-UPDATE-on-partitioned-tables.patch | text/plain | 62.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | PG Bug reporting form | 2018-08-22 07:36:58 | BUG #15346: Replica fails to start after the crash |
Previous Message | Masahiko Sawada | 2018-08-22 07:05:24 | Re: Two proposed modifications to the PostgreSQL FDW |