On 2018/08/22 21:30, David Rowley wrote: > On 22 August 2018 at 19:08, Amit Langote <langote_amit...@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.
No strong preference as such. Maybe, let's defer to committer. >> 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. Sure. I guess "to the trouble of" was missing then. :) >> + /* 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. Thanks. >> 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. Okay, doing it now. Thanks, Amit
