(2019/01/15 11:42), Amit Langote wrote:
> On 2019/01/11 21:50, Etsuro Fujita wrote:
>>>> (2019/01/10 10:41), Amit Langote wrote:
>>>>> That's a loaded meaning and abusing it to mean something else can be
>>>>> challenged, but we can live with that if properly documented.
>>>>> Speaking of
>>>>> which:
>>>>>
>>>>> /* used by partitionwise joins: */
>>>>> bool consider_partitionwise_join; /* consider
>>>>> partitionwise join
>>>>> * paths? (if
>>>>> partitioned
>>>>> rel) */
>>>>>
>>>>> Maybe, mention here how it will be abused in back-branches for
>>>>> non-partitioned relations?
>> I know we don't yet reach a consensus on what to do in details to address
>> this issue, but for the above, how about adding comments like this to
>> set_append_rel_size(), instead of the header file:
>>
>> /*
>> * If we consider partitionwise joins with the parent rel, do the
>> same
>> * for partitioned child rels.
>> *
>> * Note: here we abuse the consider_partitionwise_join flag for child
>> * rels that are not partitioned, to tell try_partitionwise_join()
>> * that their targetlists and EC entries have been generated.
>> */
>> if (rel->consider_partitionwise_join)
>> childrel->consider_partitionwise_join = true;
>>
>> ISTM that that would be more clearer than the header file.
>
> Thanks for updating the patch. I tend to agree that it might be better to
> add such details here than in the header as it's better to keep the latter
> more stable.
>
> About the comment you added, I think we could clarify the note further as:
>
> Note: here we abuse the consider_partitionwise_join flag by setting it
> *even* for child rels that are not partitioned. In that case, we set it
> to tell try_partitionwise_join() that it doesn't need to generate their
> targetlists and EC entries as they have already been generated here, as
> opposed to the dummy child rels for which the flag is left set to false so
> that it will generate them.
>
> Maybe it's a bit wordy, but it helps get the intention across more clearly.
I think that is well-worded, so +1 from me.
Thanks again!
Best regards,
Etsuro Fujita
