On 30 July 2018 at 20:26, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> I couldn't find much to complain about in the latest v3, except I noticed
> a few instances of the word "setup" where I think what's really meant is
> "set up".
>
> + * must be setup, but any sub-partitioned tables can be setup lazily as
>
> + * A ResultRelInfo has not been setup for this partition yet,
>
Great. I've fixed those and also fixed a few other comments. I found
the comments on PartitionTupleRouting didn't really explain how the
arrays were indexed. I've made an attempt to make that clear.
I've attached a complete v4 patch.
> By the way, when going over the updated code, I noticed that the code
> around child_parent_tupconv_maps could use some refactoring too.
> Especially, I noticed that ExecSetupChildParentMapForLeaf() allocates
> child-to-parent map array needed for transition tuple capture even if not
> needed by any of the leaf partitions. I'm attaching here a patch that
> applies on top of your v3 to show what I'm thinking we could do.
Maybe we can do that as a follow-on patch. I think what we have so far
is already ended up quite complex to review. What do you think?
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services