Get rid of duplicate child RTE for a partitioned table.

We've been creating duplicate RTEs for partitioned tables just
because we do so for regular inheritance parent tables.  But unlike
regular-inheritance parents which are themselves regular tables
and thus need to be scanned, partitioned tables don't need the
extra RTE.

This makes the conditions for building a child RTE the same as those
for building an AppendRelInfo, allowing minor simplification in
expand_single_inheritance_child.  Since the planner's actual processing
is driven off the AppendRelInfo list, nothing much changes beyond that,
we just have one fewer useless RTE entry.

Amit Langote, reviewed and hacked a bit by me

Discussion: https://postgr.es/m/9d7c5112-cb99-6a47-d3be-cf1ee6862a1d@lab.ntt.co.jp

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 2be67bca02ab6b483a16a9b7af215efe947436f7..9ad9035c5df1823cfbedd7961701832e8ae34ad2 100644 (file)
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -8435,7 +8435,7 @@ SELECT t1.a,t2.b,t2.c FROM fprt1 t1 LEFT JOIN (SELECT * FROM fprt2 WHERE a < 10)
  Foreign Scan
    Output: t1.a, ftprt2_p1.b, ftprt2_p1.c
    Relations: (public.ftprt1_p1 t1) LEFT JOIN (public.ftprt2_p1 fprt2)
-   Remote SQL: SELECT r6.a, r9.b, r9.c FROM (public.fprt1_p1 r6 LEFT JOIN public.fprt2_p1 r9 ON (((r6.a = r9.b)) AND ((r6.b = r9.a)) AND ((r9.a < 10)))) WHERE ((r6.a < 10)) ORDER BY r6.a ASC NULLS LAST, r9.b ASC NULLS LAST, r9.c ASC NULLS LAST
+   Remote SQL: SELECT r5.a, r7.b, r7.c FROM (public.fprt1_p1 r5 LEFT JOIN public.fprt2_p1 r7 ON (((r5.a = r7.b)) AND ((r5.b = r7.a)) AND ((r7.a < 10)))) WHERE ((r5.a < 10)) ORDER BY r5.a ASC NULLS LAST, r7.b ASC NULLS LAST, r7.c ASC NULLS LAST
 (4 rows)
 
 SELECT t1.a,t2.b,t2.c FROM fprt1 t1 LEFT JOIN (SELECT * FROM fprt2 WHERE a < 10) t2 ON (t1.a = t2.b and t1.b = t2.a) WHERE t1.a < 10 ORDER BY 1,2,3;

diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c
index 1fa154e0cb141194d08fd1ff03beec7f5a03a375..1d1e5060e26af41d98c227862227057a3c7d0f06 100644 (file)
--- a/src/backend/optimizer/util/inherit.c
+++ b/src/backend/optimizer/util/inherit.c
@@ -90,9 +90,10 @@ expand_inherited_tables(PlannerInfo *root)
  * A childless table is never considered to be an inheritance set. For
  * regular inheritance, a parent RTE must always have at least two associated
  * AppendRelInfos: one corresponding to the parent table as a simple member of
- * inheritance set and one or more corresponding to the actual children.
- * Since a partitioned table is not scanned, it might have only one associated
- * AppendRelInfo.
+ * the inheritance set and one or more corresponding to the actual children.
+ * (But a partitioned table might have only one associated AppendRelInfo,
+ * since it's not itself scanned and hence doesn't need a second RTE to
+ * represent itself as a member of the set.)
  */
 static void
 expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
@@ -145,6 +146,9 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
        /* Scan the inheritance set and expand it */
        if (oldrelation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
        {
+               /*
+                * Partitioned table, so set up for partitioning.
+                */
                Assert(rte->relkind == RELKIND_PARTITIONED_TABLE);
 
                if (root->glob->partition_directory == NULL)
@@ -161,6 +165,11 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
        }
        else
        {
+               /*
+                * Ordinary table, so process traditional-inheritance children.  (Note
+                * that partitioned tables are not allowed to have inheritance
+                * children, so it's not possible for both cases to apply.)
+                */
                List       *appinfos = NIL;
                RangeTblEntry *childrte;
                Index           childRTindex;
@@ -182,8 +191,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
                }
 
                /*
-                * This table has no partitions.  Expand any plain inheritance
-                * children in the order the OIDs were returned by
+                * Expand inheritance children in the order the OIDs were returned by
                 * find_all_inheritors.
                 */
                foreach(l, inhOIDs)
@@ -273,11 +281,6 @@ expand_partitioned_rtentry(PlannerInfo *root, RangeTblEntry *parentrte,
                root->partColsUpdated =
                        has_partition_attrs(parentrel, parentrte->updatedCols, NULL);
 
-       /* First expand the partitioned table itself. */
-       expand_single_inheritance_child(root, parentrte, parentRTindex, parentrel,
-                                                                       top_parentrc, parentrel,
-                                                                       appinfos, &childrte, &childRTindex);
-
        /*
         * If the partitioned table has no partitions, treat this as the
         * non-inheritance case.
@@ -288,6 +291,11 @@ expand_partitioned_rtentry(PlannerInfo *root, RangeTblEntry *parentrte,
                return;
        }
 
+       /*
+        * Create a child RTE for each partition.  Note that unlike traditional
+        * inheritance, we don't need a child RTE for the partitioned table
+        * itself, because it's not going to be scanned.
+        */
        for (i = 0; i < partdesc->nparts; i++)
        {
                Oid                     childOID = partdesc->oids[i];
@@ -321,8 +329,7 @@ expand_partitioned_rtentry(PlannerInfo *root, RangeTblEntry *parentrte,
 
 /*
  * expand_single_inheritance_child
- *             Build a RangeTblEntry and an AppendRelInfo, if appropriate, plus
- *             maybe a PlanRowMark.
+ *             Build a RangeTblEntry and an AppendRelInfo, plus maybe a PlanRowMark.
  *
  * We now expand the partition hierarchy level by level, creating a
  * corresponding hierarchy of AppendRelInfos and RelOptInfos, where each
@@ -371,9 +378,11 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
        childrte->relid = childOID;
        childrte->relkind = childrel->rd_rel->relkind;
        /* A partitioned child will need to be expanded further. */
-       if (childOID != parentOID &&
-               childrte->relkind == RELKIND_PARTITIONED_TABLE)
+       if (childrte->relkind == RELKIND_PARTITIONED_TABLE)
+       {
+               Assert(childOID != parentOID);
                childrte->inh = true;
+       }
        else
                childrte->inh = false;
        childrte->requiredPerms = 0;
@@ -383,36 +392,29 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
        *childRTindex_p = childRTindex;
 
        /*
-        * We need an AppendRelInfo if paths will be built for the child RTE. If
-        * childrte->inh is true, then we'll always need to generate append paths
-        * for it.  If childrte->inh is false, we must scan it if it's not a
-        * partitioned table; but if it is a partitioned table, then it never has
-        * any data of its own and need not be scanned.
+        * Build an AppendRelInfo struct for each parent/child pair.
         */
-       if (childrte->relkind != RELKIND_PARTITIONED_TABLE || childrte->inh)
-       {
-               appinfo = make_append_rel_info(parentrel, childrel,
-                                                                          parentRTindex, childRTindex);
-               *appinfos = lappend(*appinfos, appinfo);
+       appinfo = make_append_rel_info(parentrel, childrel,
+                                                                  parentRTindex, childRTindex);
+       *appinfos = lappend(*appinfos, appinfo);
 
-               /*
-                * Translate the column permissions bitmaps to the child's attnums (we
-                * have to build the translated_vars list before we can do this). But
-                * if this is the parent table, leave copyObject's result alone.
-                *
-                * Note: we need to do this even though the executor won't run any
-                * permissions checks on the child RTE.  The insertedCols/updatedCols
-                * bitmaps may be examined for trigger-firing purposes.
-                */
-               if (childOID != parentOID)
-               {
-                       childrte->selectedCols = translate_col_privs(parentrte->selectedCols,
-                                                                                                                appinfo->translated_vars);
-                       childrte->insertedCols = translate_col_privs(parentrte->insertedCols,
-                                                                                                                appinfo->translated_vars);
-                       childrte->updatedCols = translate_col_privs(parentrte->updatedCols,
-                                                                                                               appinfo->translated_vars);
-               }
+       /*
+        * Translate the column permissions bitmaps to the child's attnums (we
+        * have to build the translated_vars list before we can do this).  But if
+        * this is the parent table, we can leave copyObject's result alone.
+        *
+        * Note: we need to do this even though the executor won't run any
+        * permissions checks on the child RTE.  The insertedCols/updatedCols
+        * bitmaps may be examined for trigger-firing purposes.
+        */
+       if (childOID != parentOID)
+       {
+               childrte->selectedCols = translate_col_privs(parentrte->selectedCols,
+                                                                                                        appinfo->translated_vars);
+               childrte->insertedCols = translate_col_privs(parentrte->insertedCols,
+                                                                                                        appinfo->translated_vars);
+               childrte->updatedCols = translate_col_privs(parentrte->updatedCols,
+                                                                                                       appinfo->translated_vars);
        }
 
        /*