Re: Partition-wise join for join between (declaratively) partitioned tables

Here's the set of patches rebased on latest head, which also has a
commit to eliminate scans on partitioned tables. This change has
caused problems with multi-level partitioned tables, that I have not
fixed in this patch set. Also a couple of partition-wise join plans
for single-level partitioned tables have changed to non-partition-wise
joins. I haven't fixed those as well.

I have added a separate patch to fix add_paths_to_append_rel() to
collect partitioned_rels list for join relations. Please let me know
if this looks good. I think it needs to be merged into some other
patch, but I am not sure which. Probably we should just treat it as
another refactoring patch.

On Tue, Mar 21, 2017 at 5:16 PM, Ashutosh Bapat
<ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
> On Mon, Mar 20, 2017 at 11:33 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Mon, Mar 20, 2017 at 1:19 PM, Ashutosh Bapat
>> <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>>>> That seems different than what I suggested and I'm not sure what the
>>>> reason is for the difference?
>>>
>>> The patch adding macros IS_JOIN_REL() and IS_OTHER_REL() and changing
>>> the code to use it will look quite odd by itself. We are not changing
>>> all the instances of RELOPT_JOINREL or RELOPT_OTHER_MEMBER_REL to use
>>> those. There is code which needs to check those kinds, instead of "all
>>> join rels" or "all other rels" resp. So the patch will add those
>>> macros, change only few places to use those macros, which are intended
>>> to be changed while applying partition-wise join support for single
>>> level partitioned table.
>>
>> Hmm. You might be right, but I'm not convinced.
>
> Ok. changed as per your request in the latest set of patches.
>
> There are some more changes as follows
> 1. In the earlier patch set the changes related to
> calc_nestloop_required_outer() and related functions were spread
> across multiple patches. That was unintentional. This patch set has
> all those changes in a single patch.
>
> 2. Rajkumar reported a crash offlist. When one of the joining
> multi-level partitioned relations is empty, an assertion in
> try_partition_wise_join() Assert(rel1->part_rels && rel2->part_rels);
> failed since it didn't find part_rels for a subpartition. The problem
> here is set_append_rel_size() does not call set_rel_size() and hence
> set_append_rel_size() if a child is found to be empty, a scenario
> described in [1]. It's the later one which sets the part_rels for a
> partitioned relation and hence the subpartitions do not get part_rels
> since set_append_rel_size() is never called for those. Generally, if a
> partitioned relation is found to be empty before we set part_rels, we
> may not want to spend time in creating/collecting child RelOptInfos,
> since they will be empty anyway. If part_rels isn't present,
> part_scheme doesn't make sense. So an empty partitioned table without
> any partitions can be treated as unpartitioned. So, I have fixed
> set_dummy_rel_pathlist() and mark_dummy_rel(), the functions setting a
> relation empty, to reset partition scheme when those conditions are
> met. This fix is included as a separate patch. Let me know if this
> looks good to you.
>
> 3. I am in the process of completing reparameterize_paths_by_child()
> by adding all possible paths. I have restructured the function to look
> better and have one switch case instead of two. Also added more path
> types including ForeignPath, for which I have added a FDW hook, with
> documentation, for handling fdw_private. Please let me know if this
> looks good to you. I am thinking of similar hook for CustomPath. I
> will continue to add more path types to
> reparameterize_path_by_child().
>
> I am wondering whether we should bring 0007 he patche adjusting code
> to work with child-joins before 0006, partition-wise join. 0006 needs
> it, but 0007 doesn't depend upon 0006. Will that be any better?
>
> [1] CAFjFpRcdrdsCRDbBu0J2pxwWbhb_sDWQUTVznBy_4XGr-p3+wA(at)mail(dot)gmail(dot)com,
> subject "Asymmetry between parent and child wrt "false" quals"
>
> --
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company