Message-ID: <5C41CCC0.5030702@lab.ntt.co.jp>
Date: Fri, 18 Jan 2019 21:55:28 +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>
 <5C3EC4EC.5070404@lab.ntt.co.jp>
In-Reply-To: <5C3EC4EC.5070404@lab.ntt.co.jp>
Content-Type: multipart/mixed;
 boundary="------------070705030501070105080003"
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

This is a multi-part message in MIME format.
--------------070705030501070105080003
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

(2019/01/16 14:45), Etsuro Fujita wrote:
> (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.

I updated the patch as such and rebased it to the latest HEAD.  I also 
added the commit message.  Attached is an updated patch.  Does that make 
sense?  If there are no objections, I'll push that patch early next week.

Best regards,
Etsuro Fujita

--------------070705030501070105080003
Content-Type: text/x-patch;
 name="0001-Postpone-generating-tlists-and-EC-members-for-inheri.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0001-Postpone-generating-tlists-and-EC-members-for-inheri.pa";
 filename*1="tch"