Re: Query with high planning time at version 11.1 compared versions10.5 and 11.0

(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