From: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
---|---|
To: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
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 12:30:14 |
Message-ID: | CAKJS1f9T_32Xpb-p8cWwo5ezSfVhXviUW8QTWncP8ksPHDRK8g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 22 August 2018 at 19:08, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> +#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 did that to save 3 chars. I think putting the additional
Child/Parent in the name is not really required. It's not as if we're
going to have a ParentToParent or a ChildToChild map, so I thought it
might be okay to assume that if it's "ToParent", that it's being
converted from the child and "ToChild" seems safe to assume it's being
converted from the parent. I can change it though if you feel very
strongly that what I've got is no good.
>> 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.
Yeah.
> 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
I disagree, but perhaps we can just reword it a bit. I've now got:
+ * Every time a tuple is routed to a partition that we've yet to set the
+ * ResultRelInfo for, before we go to the trouble of making one, we check
+ * for a pre-made one in the hash table.
> + /* 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".
Thanks for noticing.
> Since the above two are minor issues, fixed them myself in the attached
> updated version; didn't touch the macro though.
I've attached a v8. The only change from your v7 is in the "go making" comment.
> Do you agree to setting this patch to "Ready for Committer" in the
> September CF?
I read through the entire patch a couple of times yesterday and saw
nothing else, so yeah, I think now is a good time for someone with
more authority to have a look at it.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachment | Content-Type | Size |
---|---|---|
v8-0001-Speed-up-INSERT-and-UPDATE-on-partitioned-tables.patch | application/octet-stream | 62.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2018-08-22 12:33:30 | Re: plan_cache_mode and postgresql.conf.sample |
Previous Message | Sandeep Thakkar | 2018-08-22 12:22:11 | Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c) |