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

On Mon, Mar 20, 2017 at 11:33 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Mar 20, 2017 at 1:19 PM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.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@mail.gmail.com,
subject "Asymmetry between parent and child wrt "false" quals"

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