Message-ID: <5C3EC4EC.5070404@lab.ntt.co.jp>
Date: Wed, 16 Jan 2019 14:45:16 +0900
From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:11.0) Gecko/20120327 Thunderbird/11.0.1
MIME-Version: 1.0
Subject: Re: Query with high planning time at version 11.1 compared versions
 10.5 and 11.0
References: <8db4eca8-6e76-70ff-f934-e0228a1cc298@lab.ntt.co.jp>
 <20181206075726.pig32e6thltb56hr@alvherre.pgsql>
 <CAExHW5umxU5FOoT8FBcqp-GWKMtZhnuEy7U6bNH8QfzvYwUgEQ@mail.gmail.com>
 <CAExHW5uFwP_1N7RV6s8piQBvygGH+rppS60nFNHB9DKEM04ShA@mail.gmail.com>
 <5C34846E.9090600@lab.ntt.co.jp>
 <96720c99-ffa0-01ad-c594-0504c8eda708@lab.ntt.co.jp>
 <5C35D912.8040200@lab.ntt.co.jp>
 <7354b5bf-5932-4c0c-81ee-d9550de18995@lab.ntt.co.jp>
 <5C36E105.2020902@lab.ntt.co.jp>
 <dd1de042-dca5-cfb9-bc66-2e39020fbf6a@lab.ntt.co.jp>
 <5C38910B.8010901@lab.ntt.co.jp>
 <a5c087d0-9ecc-1315-24d2-6303d14a6116@lab.ntt.co.jp>
In-Reply-To: <a5c087d0-9ecc-1315-24d2-6303d14a6116@lab.ntt.co.jp>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, alvherre@2ndquadrant.com,
        pryzby@telsasoft.com, sanyo.moura@tatic.net,
        Jeff Janes <jeff.janes@gmail.com>, david.rowley@2ndquadrant.com,
        Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@postgresql.org
Precedence: bulk

(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