On Mon, Mar 20, 2017 at 10:26 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Mar 20, 2017 at 12:52 PM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
>>> Hmm. I would kind of like to move the IS_JOIN_REL() and
>>> IS_OTHER_REL() stuff to the front of the series. In other words, I
>>> propose that we add those macros first, each testing for only the one
>>> kind of RelOptInfo that exists today, and change all the code to use
>>> them. Then, when we add child joinrels, we can modify the macros at
>>> the same time. The problem with doing it the way you have it is that
>>> those changes will have to be squashed into the main partitionwise
>>> join commit, because otherwise stuff will be broken. Doing it the
>>> other way around lets us commit that bit separately.
>>
>> I can provide a patch with adjust_appendrel_attrs_multilevel() changed
>> to child-joins, which can be applied before multi-level
>> partitioin-wise support patch but after partition-wise implementation
>> patch. You may consider applying that patch separately before
>> multi-level partition-wise support, in case we see that multi-level
>> partition-wise join support can be committed. Does that sound good?
>> That way we save changing those macros twice.
>
> 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.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company