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);
    }
 
    /*