Repair bogus EPQ plans generated for postgres_fdw foreign joins.

postgres_fdw's postgresGetForeignPlan() assumes without checking that the
outer_plan it's given for a join relation must have a NestLoop, MergeJoin,
or HashJoin node at the top.  That's been wrong at least since commit
4bbf6edfb (which could cause insertion of a Sort node on top) and it seems
like a pretty unsafe thing to Just Assume even without that.

Through blind good fortune, this doesn't seem to have any worse
consequences today than strange EXPLAIN output, but it's clearly trouble
waiting to happen.

To fix, test the node type explicitly before touching Join-specific
fields, and avoid jamming the new tlist into a node type that can't
do projection.  Export a new support function from createplan.c
to avoid building low-level knowledge about the latter into FDWs.

Back-patch to 9.6 where the faulty coding was added.  Note that the
associated regression test cases don't show any changes before v11,
apparently because the tests back-patched with 4bbf6edfb don't actually
exercise the problem case before then (there's no top-level Sort
in those plans).

Discussion: https://postgr.es/m/8946.1544644803@sss.pgh.pa.us

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index dfa6201b3f23426f69d2a3c1ab32dcf1c1532602..666548ba696eaaaa4e0212dc242ecd0fce651cc9 100644 (file)
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -1701,25 +1701,27 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t
                Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
                Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
                Remote SQL: SELECT r1."C 1", r2."C 1", r1.c3, CASE WHEN (r1.*)::text IS NOT NULL THEN ROW(r1."C 1", r1.c2, r1.c3, r1.c4, r1.c5, r1.c6, r1.c7, r1.c8) END, CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2."C 1", r2.c2, r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8) END FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")))) ORDER BY r1.c3 ASC NULLS LAST, r1."C 1" ASC NULLS LAST FOR UPDATE OF r1
-               ->  Sort
-                     Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
-                     Sort Key: t1.c3 USING <, t1.c1
-                     ->  Merge Join
+               ->  Result
+                     Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
+                     ->  Sort
                            Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
-                           Merge Cond: (t1.c1 = t2.c1)
-                           ->  Sort
-                                 Output: t1.c1, t1.c3, t1.*
-                                 Sort Key: t1.c1
-                                 ->  Foreign Scan on public.ft1 t1
+                           Sort Key: t1.c3, t1.c1
+                           ->  Merge Join
+                                 Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
+                                 Merge Cond: (t1.c1 = t2.c1)
+                                 ->  Sort
                                        Output: t1.c1, t1.c3, t1.*
-                                       Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR UPDATE
-                           ->  Sort
-                                 Output: t2.c1, t2.*
-                                 Sort Key: t2.c1
-                                 ->  Foreign Scan on public.ft2 t2
+                                       Sort Key: t1.c1
+                                       ->  Foreign Scan on public.ft1 t1
+                                             Output: t1.c1, t1.c3, t1.*
+                                             Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR UPDATE
+                                 ->  Sort
                                        Output: t2.c1, t2.*
-                                       Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
-(26 rows)
+                                       Sort Key: t2.c1
+                                       ->  Foreign Scan on public.ft2 t2
+                                             Output: t2.c1, t2.*
+                                             Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
+(28 rows)
 
 SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR UPDATE OF t1;
  c1  | c1  
@@ -1748,25 +1750,27 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t
                Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
                Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
                Remote SQL: SELECT r1."C 1", r2."C 1", r1.c3, CASE WHEN (r1.*)::text IS NOT NULL THEN ROW(r1."C 1", r1.c2, r1.c3, r1.c4, r1.c5, r1.c6, r1.c7, r1.c8) END, CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2."C 1", r2.c2, r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8) END FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")))) ORDER BY r1.c3 ASC NULLS LAST, r1."C 1" ASC NULLS LAST FOR UPDATE OF r1 FOR UPDATE OF r2
-               ->  Sort
-                     Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
-                     Sort Key: t1.c3 USING <, t1.c1
-                     ->  Merge Join
+               ->  Result
+                     Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
+                     ->  Sort
                            Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
-                           Merge Cond: (t1.c1 = t2.c1)
-                           ->  Sort
-                                 Output: t1.c1, t1.c3, t1.*
-                                 Sort Key: t1.c1
-                                 ->  Foreign Scan on public.ft1 t1
+                           Sort Key: t1.c3, t1.c1
+                           ->  Merge Join
+                                 Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
+                                 Merge Cond: (t1.c1 = t2.c1)
+                                 ->  Sort
                                        Output: t1.c1, t1.c3, t1.*
-                                       Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR UPDATE
-                           ->  Sort
-                                 Output: t2.c1, t2.*
-                                 Sort Key: t2.c1
-                                 ->  Foreign Scan on public.ft2 t2
+                                       Sort Key: t1.c1
+                                       ->  Foreign Scan on public.ft1 t1
+                                             Output: t1.c1, t1.c3, t1.*
+                                             Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR UPDATE
+                                 ->  Sort
                                        Output: t2.c1, t2.*
-                                       Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR UPDATE
-(26 rows)
+                                       Sort Key: t2.c1
+                                       ->  Foreign Scan on public.ft2 t2
+                                             Output: t2.c1, t2.*
+                                             Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR UPDATE
+(28 rows)
 
 SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR UPDATE;
  c1  | c1  
@@ -1796,25 +1800,27 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t
                Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
                Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
                Remote SQL: SELECT r1."C 1", r2."C 1", r1.c3, CASE WHEN (r1.*)::text IS NOT NULL THEN ROW(r1."C 1", r1.c2, r1.c3, r1.c4, r1.c5, r1.c6, r1.c7, r1.c8) END, CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2."C 1", r2.c2, r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8) END FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")))) ORDER BY r1.c3 ASC NULLS LAST, r1."C 1" ASC NULLS LAST FOR SHARE OF r1
-               ->  Sort
-                     Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
-                     Sort Key: t1.c3 USING <, t1.c1
-                     ->  Merge Join
+               ->  Result
+                     Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
+                     ->  Sort
                            Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
-                           Merge Cond: (t1.c1 = t2.c1)
-                           ->  Sort
-                                 Output: t1.c1, t1.c3, t1.*
-                                 Sort Key: t1.c1
-                                 ->  Foreign Scan on public.ft1 t1
+                           Sort Key: t1.c3, t1.c1
+                           ->  Merge Join
+                                 Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
+                                 Merge Cond: (t1.c1 = t2.c1)
+                                 ->  Sort
                                        Output: t1.c1, t1.c3, t1.*
-                                       Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR SHARE
-                           ->  Sort
-                                 Output: t2.c1, t2.*
-                                 Sort Key: t2.c1
-                                 ->  Foreign Scan on public.ft2 t2
+                                       Sort Key: t1.c1
+                                       ->  Foreign Scan on public.ft1 t1
+                                             Output: t1.c1, t1.c3, t1.*
+                                             Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR SHARE
+                                 ->  Sort
                                        Output: t2.c1, t2.*
-                                       Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
-(26 rows)
+                                       Sort Key: t2.c1
+                                       ->  Foreign Scan on public.ft2 t2
+                                             Output: t2.c1, t2.*
+                                             Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
+(28 rows)
 
 SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR SHARE OF t1;
  c1  | c1  
@@ -1843,25 +1849,27 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t
                Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
                Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
                Remote SQL: SELECT r1."C 1", r2."C 1", r1.c3, CASE WHEN (r1.*)::text IS NOT NULL THEN ROW(r1."C 1", r1.c2, r1.c3, r1.c4, r1.c5, r1.c6, r1.c7, r1.c8) END, CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2."C 1", r2.c2, r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8) END FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")))) ORDER BY r1.c3 ASC NULLS LAST, r1."C 1" ASC NULLS LAST FOR SHARE OF r1 FOR SHARE OF r2
-               ->  Sort
-                     Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
-                     Sort Key: t1.c3 USING <, t1.c1
-                     ->  Merge Join
+               ->  Result
+                     Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
+                     ->  Sort
                            Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
-                           Merge Cond: (t1.c1 = t2.c1)
-                           ->  Sort
-                                 Output: t1.c1, t1.c3, t1.*
-                                 Sort Key: t1.c1
-                                 ->  Foreign Scan on public.ft1 t1
+                           Sort Key: t1.c3, t1.c1
+                           ->  Merge Join
+                                 Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
+                                 Merge Cond: (t1.c1 = t2.c1)
+                                 ->  Sort
                                        Output: t1.c1, t1.c3, t1.*
-                                       Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR SHARE
-                           ->  Sort
-                                 Output: t2.c1, t2.*
-                                 Sort Key: t2.c1
-                                 ->  Foreign Scan on public.ft2 t2
+                                       Sort Key: t1.c1
+                                       ->  Foreign Scan on public.ft1 t1
+                                             Output: t1.c1, t1.c3, t1.*
+                                             Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR SHARE
+                                 ->  Sort
                                        Output: t2.c1, t2.*
-                                       Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR SHARE
-(26 rows)
+                                       Sort Key: t2.c1
+                                       ->  Foreign Scan on public.ft2 t2
+                                             Output: t2.c1, t2.*
+                                             Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR SHARE
+(28 rows)
 
 SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR SHARE;
  c1  | c1  

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 674eb982d06bf70ca91532d0eedd4cfc2d7db971..2ac9d7bbe7a6ba23ef92e43e5b762107974afd68 100644 (file)
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -1229,11 +1229,9 @@ postgresGetForeignPlan(PlannerInfo *root,
 
        /*
         * Ensure that the outer plan produces a tuple whose descriptor
-        * matches our scan tuple slot. This is safe because all scans and
-        * joins support projection, so we never need to insert a Result node.
-        * Also, remove the local conditions from outer plan's quals, lest
-        * they will be evaluated twice, once by the local plan and once by
-        * the scan.
+        * matches our scan tuple slot.  Also, remove the local conditions
+        * from outer plan's quals, lest they be evaluated twice, once by the
+        * local plan and once by the scan.
         */
        if (outer_plan)
        {
@@ -1246,23 +1244,42 @@ postgresGetForeignPlan(PlannerInfo *root,
             */
            Assert(!IS_UPPER_REL(foreignrel));
 
-           outer_plan->targetlist = fdw_scan_tlist;
-
+           /*
+            * First, update the plan's qual list if possible.  In some cases
+            * the quals might be enforced below the topmost plan level, in
+            * which case we'll fail to remove them; it's not worth working
+            * harder than this.
+            */
            foreach(lc, local_exprs)
            {
-               Join       *join_plan = (Join *) outer_plan;
                Node       *qual = lfirst(lc);
 
                outer_plan->qual = list_delete(outer_plan->qual, qual);
 
                /*
                 * For an inner join the local conditions of foreign scan plan
-                * can be part of the joinquals as well.
+                * can be part of the joinquals as well.  (They might also be
+                * in the mergequals or hashquals, but we can't touch those
+                * without breaking the plan.)
                 */
-               if (join_plan->jointype == JOIN_INNER)
-                   join_plan->joinqual = list_delete(join_plan->joinqual,
-                                                     qual);
+               if (IsA(outer_plan, NestLoop) ||
+                   IsA(outer_plan, MergeJoin) ||
+                   IsA(outer_plan, HashJoin))
+               {
+                   Join       *join_plan = (Join *) outer_plan;
+
+                   if (join_plan->jointype == JOIN_INNER)
+                       join_plan->joinqual = list_delete(join_plan->joinqual,
+                                                         qual);
+               }
            }
+
+           /*
+            * Now fix the subplan's tlist --- this might result in inserting
+            * a Result node atop the plan tree.
+            */
+           outer_plan = change_plan_targetlist(outer_plan, fdw_scan_tlist,
+                                               best_path->path.parallel_safe);
        }
    }
 

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index da7a92081aff2277e5324dba6b5d8a027c953e06..91cf78233d592bbd63c6bb1b2f21c4e748d2f826 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -1407,20 +1407,10 @@ create_unique_plan(PlannerInfo *root, UniquePath *best_path, int flags)
        }
    }
 
+   /* Use change_plan_targetlist in case we need to insert a Result node */
    if (newitems || best_path->umethod == UNIQUE_PATH_SORT)
-   {
-       /*
-        * If the top plan node can't do projections and its existing target
-        * list isn't already what we need, we need to add a Result node to
-        * help it along.
-        */
-       if (!is_projection_capable_plan(subplan) &&
-           !tlist_same_exprs(newtlist, subplan->targetlist))
-           subplan = inject_projection_plan(subplan, newtlist,
-                                            best_path->path.parallel_safe);
-       else
-           subplan->targetlist = newtlist;
-   }
+       subplan = change_plan_targetlist(subplan, newtlist,
+                                        best_path->path.parallel_safe);
 
    /*
     * Build control information showing which subplan output columns are to
@@ -1762,6 +1752,40 @@ inject_projection_plan(Plan *subplan, List *tlist, bool parallel_safe)
    return plan;
 }
 
+/*
+ * change_plan_targetlist
+ *   Externally available wrapper for inject_projection_plan.
+ *
+ * This is meant for use by FDW plan-generation functions, which might
+ * want to adjust the tlist computed by some subplan tree.  In general,
+ * a Result node is needed to compute the new tlist, but we can optimize
+ * some cases.
+ *
+ * In most cases, tlist_parallel_safe can just be passed as the parallel_safe
+ * flag of the FDW's own Path node.
+ */
+Plan *
+change_plan_targetlist(Plan *subplan, List *tlist, bool tlist_parallel_safe)
+{
+   /*
+    * If the top plan node can't do projections and its existing target list
+    * isn't already what we need, we need to add a Result node to help it
+    * along.
+    */
+   if (!is_projection_capable_plan(subplan) &&
+       !tlist_same_exprs(tlist, subplan->targetlist))
+       subplan = inject_projection_plan(subplan, tlist,
+                                        subplan->parallel_safe &&
+                                        tlist_parallel_safe);
+   else
+   {
+       /* Else we can just replace the plan node's tlist */
+       subplan->targetlist = tlist;
+       subplan->parallel_safe &= tlist_parallel_safe;
+   }
+   return subplan;
+}
+
 /*
  * create_sort_plan
  *

diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index c8ab0280d2209cc45dd6042ed173b2bfd6f2cba5..a081ca689a3c1d615db9ddd9512fa0f1f6c7c411 100644 (file)
--- a/src/include/optimizer/planmain.h
+++ b/src/include/optimizer/planmain.h
@@ -53,6 +53,8 @@ extern ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
                 Index scanrelid, List *fdw_exprs, List *fdw_private,
                 List *fdw_scan_tlist, List *fdw_recheck_quals,
                 Plan *outer_plan);
+extern Plan *change_plan_targetlist(Plan *subplan, List *tlist,
+                      bool tlist_parallel_safe);
 extern Plan *materialize_finished_plan(Plan *subplan);
 extern bool is_projection_capable_path(Path *path);
 extern bool is_projection_capable_plan(Plan *plan);