On 2018/07/28 10:54, David Rowley wrote:
> On 27 July 2018 at 19:11, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> I've attached a delta patch to make the above changes. I'm leaving the
>> hash table rename up to you though.
>
> Thanks for the delta patch. I took all of it, just rewrote a comment slightly.
>
> I also renamed the hash table to your suggestion and changed a few more things.
>
> Attached a delta based on v2 and the full v3 patch.
>
> This includes another small change to make
> PartitionDispatchData->indexes an array that's allocated in the same
> memory as the PartitionDispatchData. This will save a palloc() call
> and also should be a bit more cache friendly.
>
> This also required a rebase on master again due to 3e32109049.
Thanks for the updated patch.
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,
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.
Thanks,
Amit