Prologue of set_append_rel_size() and partitioned tables
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
Attachments:
set_append_rel_size_prologue.patchapplication/octet-stream; name=set_append_rel_size_prologue.patchDownload
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index a1e1a87..bf959ab 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -834,11 +834,12 @@ set_foreign_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
* Set size estimates for an "append relation"
*
* 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
- * a good thing because their outputs are not the same size.
+ * relation's contents are computed by appending together the output of the
+ * individual member relations. Note that in the inheritance case, except for
+ * that representing a partitioned table, 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 a good thing because their outputs are not the
+ * same size.
*/
static void
set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
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 isUpdate patch attached.
Thanks,
Amit
Attachments:
set_append_rel_size_prologue-v2.patchtext/x-diff; name=set_append_rel_size_prologue-v2.patchDownload
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index a1e1a87c29..9fce9437f8 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -834,10 +834,10 @@ set_foreign_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
* Set size estimates for an "append relation"
*
* 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
* a good thing because their outputs are not the same size.
*/
static void
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 isUpdate patch attached.
Looks good to me.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers