Horton

Thread

Commits

  1. Update comment to account for table partitioning.

  1. Prologue of set_append_rel_size() and partitioned tables

    Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> — 2017-03-29T06:20:48Z

    The prologue of set_append_rel_size() mentions

 * ....  Note that in the inheritance case,
 * the first member relation is actually the same table as is mentioned in
 * the parent RTE ... but it has a different RTE and RelOptInfo.

This isn't true about partitioned tables anymore. We do not create
RelOptInfo of the partitioned table and thus is not first member
relation. We could argue that inheritance in case of partitioned
tables is just an implementation detail and partitioned table is not
"inherited" in true sense. So "inheritance case" referred to here does
not cover partitioning and so the sentence still holds. But I guess,
this needs some change so that we do not expect first member to be
same as partitioned table. I am not able to craft an elegant sentence
but how about something like attached?
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

  2. Re: Prologue of set_append_rel_size() and partitioned tables

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2017-03-29T06:53:53Z

    On 2017/03/29 15:20, Ashutosh Bapat wrote:
> The prologue of set_append_rel_size() mentions
> 
>  * ....  Note that in the inheritance case,
>  * the first member relation is actually the same table as is mentioned in
>  * the parent RTE ... but it has a different RTE and RelOptInfo.
> 
> This isn't true about partitioned tables anymore. We do not create
> RelOptInfo of the partitioned table and thus is not first member
> relation.

My bad.

> We could argue that inheritance in case of partitioned
> tables is just an implementation detail and partitioned table is not
> "inherited" in true sense. So "inheritance case" referred to here does
> not cover partitioning and so the sentence still holds. But I guess,
> this needs some change so that we do not expect first member to be
> same as partitioned table. I am not able to craft an elegant sentence
> but how about something like attached?

I think we *should* update the comment somwhow.  Since now there are a few
places using "non-partitioned inheritance" to refer to regular parent
tables, why not use that term here too?  So:

  * The passed-in rel and RTE represent the entire append relation.  The
- * relation's contents are computed by appending together the output of
- * the individual member relations.  Note that in the inheritance case,
- * the first member relation is actually the same table as is mentioned in
- * the parent RTE ... but it has a different RTE and RelOptInfo.  This is
+ * relation's contents are computed by appending together the output of the
+ * individual member relations.  Note that in the non-partitioned inheritance
+ * case, the first member relation is actually the same table as is mentioned
+ * in the parent RTE ... but it has a different RTE and RelOptInfo.  This is

Update patch attached.

Thanks,
Amit

  3. Re: Prologue of set_append_rel_size() and partitioned tables

    Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> — 2017-03-29T07:18:03Z

    On Wed, Mar 29, 2017 at 12:23 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2017/03/29 15:20, Ashutosh Bapat wrote:
>> The prologue of set_append_rel_size() mentions
>>
>>  * ....  Note that in the inheritance case,
>>  * the first member relation is actually the same table as is mentioned in
>>  * the parent RTE ... but it has a different RTE and RelOptInfo.
>>
>> This isn't true about partitioned tables anymore. We do not create
>> RelOptInfo of the partitioned table and thus is not first member
>> relation.
>
> My bad.
>
>> We could argue that inheritance in case of partitioned
>> tables is just an implementation detail and partitioned table is not
>> "inherited" in true sense. So "inheritance case" referred to here does
>> not cover partitioning and so the sentence still holds. But I guess,
>> this needs some change so that we do not expect first member to be
>> same as partitioned table. I am not able to craft an elegant sentence
>> but how about something like attached?
>
> I think we *should* update the comment somwhow.  Since now there are a few
> places using "non-partitioned inheritance" to refer to regular parent
> tables, why not use that term here too? So:
>
>   * The passed-in rel and RTE represent the entire append relation.  The
> - * relation's contents are computed by appending together the output of
> - * the individual member relations.  Note that in the inheritance case,
> - * the first member relation is actually the same table as is mentioned in
> - * the parent RTE ... but it has a different RTE and RelOptInfo.  This is
> + * relation's contents are computed by appending together the output of the
> + * individual member relations.  Note that in the non-partitioned inheritance
> + * case, the first member relation is actually the same table as is mentioned
> + * in the parent RTE ... but it has a different RTE and RelOptInfo.  This is
>
> Update patch attached.

Looks good to me.

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

  4. Re: Prologue of set_append_rel_size() and partitioned tables

    Robert Haas <robertmhaas@gmail.com> — 2017-06-22T14:54:52Z

    On Wed, Mar 29, 2017 at 3:18 AM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
>> Update patch attached.
>
> Looks good to me.

Committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company