On Thu, Feb 23, 2017 at 1:38 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2017/02/23 16:48, Ashutosh Bapat wrote:
>> On Thu, Feb 23, 2017 at 1:08 PM, Amit Langote wrote:
>>> @@ -996,10 +996,20 @@ inheritance_planner(PlannerInfo *root)
>>> /*
>>> + * Partitioned tables do not have storage for themselves and should not be
>>> + * scanned.
>>>
>>> @@ -1450,6 +1451,21 @@ expand_inherited_rtentry(PlannerInfo *root,
>>> RangeTblEntry *rte, Index rti)
>>> /*
>>> + * Partitioned tables themselves do not have any storage and should not
>>> + * be scanned. So, do not create child relations for those.
>>> + */
>>>
>>> I guess we should not have to repeat "partitioned tables do not have
>>> storage" in all these places.
>>
>> Hmm, you are right. But they are two different functions and the code
>> blocks are located away from each other. So, I thought it would be
>> better to have complete comment there, instead of pointing to other
>> places. I am fine, if we can reword it without compromising
>> readability.
>
> I was saying in general. I agree that different sites in the optimizer
> may have different considerations for why partitioned tables are to be
> handled specially, common theme being that we do not have to scan the
> parent relation. If the implementation changes someday such that we don't
> manipulate child tables (especially, partitions) through most planning
> phases anymore, then maybe we will start using some different terminology
> where we don't have to stress this fact too much. We're not there yet though.
I agree.
>
>>> + * a partitioned relation as dummy. The duplicate RTE we added for the
>>> + * parent table is harmless, so we don't bother to get rid of it; ditto for
>>> + * the useless PlanRowMark node.
>>>
>>> There is no duplicate RTE in the partitioned table case, which even my
>>> original comment failed to consider. Can you, maybe?
>>
>> May be we could says "If we have added duplicate RTE for the parent
>> table, it is harmless ...". Does that sound good?
>
> Duplicate RTE added in the non-partitioned table case is harmless, so we
> don't bother to get rid of it; ditto for the useless PlanRowMark node.
Fine with me.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company