Hi Ashutosh,
On 2017/08/17 21:39, Ashutosh Bapat wrote:
> On Thu, Aug 17, 2017 at 12:59 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>
>> Attached rest of the patches. 0001 has changes per Ashutosh's review
>> comments [2]:
>>
>> 0001: Relieve RelationGetPartitionDispatchInfo() of doing any locking
>
> [2] had a patch with some changes to the original patch you posted. I
> didn't describe those changes in my mail, since they rearranged the
> comments. Those changes are not part of this patch and you haven't
> comments about those changes as well. If you have intentionally
> excluded those changes, it's fine. In case, you haven't reviewed them,
> please see if they are good to be incorporated.
Sorry, I thought the ones you mentioned in the email were the only changes
you made to the original patch. I noted only those and included them when
editing the relevant commit in my local repository in an interactive
rebase session. I didn't actually take your patch and try to merge it
with the commit in my local repository. IMHO, simply commenting in the
email which parts of the patch you would like to see changed would be
helpful. Then we can discuss those changes and proceed with them (or not)
per the result of that discussion.
>> 0002: Teach expand_inherited_rtentry to use partition bound order
>
> 0004 in [1] expands a multi-level partition hierarchy into similar
> inheritance hierarchy. That patch doesn't need all OIDs in one go. It
> will have to handle the partition hierarchy level by level, so most of
> the code added by this patch will need to be changed by that patch. Is
> there a way we can somehow change this patch so that the delta in 0004
> is reduced? That may need rethinking about using
> RelationGetPartitionDispatchInfo().
Regarding that, I have a question:
Does the multi-level partition-wise join planning logic depend on the
inheritance itself to be expanded in a multi-level aware manner. That is,
expanding the partitioned table inheritance in multi-level aware manner in
expan_inherited_rtentry()?
Wouldn't it suffice to just have the resulting Append paths be nested per
multi-level partitioning hierarchy? Creating such nested Append paths
doesn't necessarily require that the inheritance be expanded that way in
the first place (as I am finding out when working on another patch).
Thanks,
Amit