Re: Partitioned tables and relfilenode

On Mon, Mar 6, 2017 at 12:48 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Thanks for the review.
>
> On 2017/03/06 15:41, Michael Paquier wrote:
>> On Fri, Mar 3, 2017 at 10:02 AM, Amit Langote
>> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> Thanks.  I noticed that 'and' is duplicated in a line added by the commit
>>> to analyze.sgml.  Attached 0001 fixes that.  0002 and 0003 same as the
>>> last version.
>>
>>     /*
>> -    * If all the children were temp tables, pretend it's a non-inheritance
>> -    * situation.  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.
>> +    * If all the children were temp tables or if the parent is a partitioned
>> +    * table without any leaf partitions, pretend it's a non-inheritance
>> +    * situation.  The duplicate RTE for the parent table we 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.
>>      */
>> -   if (list_length(appinfos) < 2)
>> +   if (!has_child)
>> This comment is not completely correct. Children can be temp tables,
>> they just cannot be temp tables of other backends. It seems to me that
>> you could still keep this code simple and remove has_child..
>
> I updated the comment.  I recall having posted a patch for that once, but
> perhaps went unnoticed.

The existing comment only specifies "temp tables" and not "temp table
of other backends". The new comment keeps that part same and adds
partitioned table case. So, I don't see any reason to change the "temp
tables" to "temp table of other backends" in this patch.

>
> About has_child, the other option is to make the minimum length of
> appinfos list relkind-based, but  the condition quickly becomes ugly. Do
> you have a suggestion?
>
+1.


-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company