On 22 August 2018 at 19:08, Amit Langote <Langote_Amit_f8@lab.ntt.co.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
