On 2017/09/14 1:42, Robert Haas wrote:
> On Wed, Sep 13, 2017 at 6:02 AM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> It seems to me we don't really need the first patch all that much. That
>> is, let's keep PartitionDispatchData the way it is for now, since we don't
>> really have any need for it beside tuple-routing (EIBO as committed didn't
>> need it for one). So, let's forget about "decoupling
>> RelationGetPartitionDispatchInfo() from the executor" thing for now and
>> move on.
>>
>> So, attached is just the patch to make RelationGetPartitionDispatchInfo()
>> traverse the partition tree in depth-first manner to be applied on HEAD.
>
> I like this patch. Not only does it improve the behavior, but it's
> actually less code than we have now, and in my opinion, the new code
> is easier to understand, too.
>
> A few suggestions:
Thanks for the review.
> - I think get_partition_dispatch_recurse() get a check_stack_depth()
> call just like expand_partitioned_rtentry() did, and for the same
> reasons: if the catalog contents are corrupted so that we have an
> infinite loop in the partitioning hierarchy, we want to error out, not
> crash.
Ah, missed that. Done.
> - I think we should add a comment explaining that we're careful to do
> this in the same order as expand_partitioned_rtentry() so that callers
> can assume that the N'th entry in the leaf_part_oids array will also
> be the N'th child of an Append node.
Done. Since the Append/ModifyTable may skip some leaf partitions due to
pruning, added a note about that too.
> + * For every partitioned table other than root, we must store a
>
> other than the root
>
> + * partitioned table. The value multiplied back by -1 is returned as the
>
> multiplied by -1, not multiplied back by -1
>
> + * tables in the tree, using which, search is continued further down the
> + * partition tree.
>
> Period after "in the tree". Then continue: "This value is used to
> continue the search in the next level of the partition tree."
Fixed.
Attached updated patch.
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers