Push recursive paths to remote nodes when construction the paths

Until now, the pushdown happened in make_recursive_union(), but that
apparently fails to update subroots (and possibly other fields), causing
segfaults in SS_finalize_plan().

Instead move the push-down logic to generate_recursion_path(), which
fixes the issue and is consistent with the overall pathification of
the planner.

With this change XL 9.6 now generates the same plans as XL 9.5.

There remains a lot to be desired, though. Firstly, it fails for queries
with nested recursive unions, because it produces a plan like this:

     ->  Remote Subquery Scan on all (dn1)
           ->  Recursive Union
                 ->  Seq Scan on department
                 ->  CTE Scan on x
                       CTE x
                         ->  Remote Subquery Scan on all (dn1)
                               ->  Recursive Union
                                     ->  Seq Scan on department
                                     ->  Append
                                           ->  WorkTable Scan on q
                                           ->  WorkTable Scan on x

This however fails with 'ERROR: unrecognized node type: 120' error,
because determine_param_types() in execRemote.c does not expect
T_WorkTableScan.

After resolving that issue, it however starts failing with a segfault, as
WorkTableScan nodes try to access the top recursive union node, which is
however executed in a different process, and possibly even on a different
data node (note the "Remote Subquery").

Those issues are however not resolved by this commit, and is left for
the future.

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 182c81f06eab79ab71f4ddaa00048137f01c0cc8..637926ff3a46500eb84ca145a696471cdab10a71 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -308,8 +308,6 @@ static int add_sort_column(AttrNumber colIdx, Oid sortOp, Oid coll,
 #endif
 
 static RemoteSubplan *find_push_down_plan(Plan *plan, bool force);
-static RemoteSubplan *find_delete_push_down_plan(PlannerInfo *root, Plan *plan,
-               bool force, Plan **parent);
 
 /*
  * create_plan
@@ -2530,14 +2528,6 @@ find_push_down_plan(Plan *plan, bool force)
        return find_push_down_plan_int(NULL, plan, force, false, NULL);
 }
 
-static RemoteSubplan *
-find_delete_push_down_plan(PlannerInfo *root,
-               Plan *plan,
-               bool force,
-               Plan **parent)
-{
-       return find_push_down_plan_int(root, plan, force, true, parent);
-}
 #endif
 
 
@@ -5631,9 +5621,6 @@ make_recursive_union(PlannerInfo *root,
        RecursiveUnion *node = makeNode(RecursiveUnion);
        Plan       *plan = &node->plan;
        int                     numCols = list_length(distinctList);
-#ifdef XCP     
-       RemoteSubplan *left_pushdown, *right_pushdown;
-#endif 
 
        plan->targetlist = tlist;
        plan->qual = NIL;
@@ -5672,44 +5659,6 @@ make_recursive_union(PlannerInfo *root,
        }
        node->numGroups = numGroups;
 
-#ifdef XCP     
-       /*
-        * For recursive CTE, we have already checked that all tables involved in
-        * the query are replicated tables (or coordinator local tables such as
-        * catalog tables). So drill down the left and right plan trees and find
-        * the corresponding remote subplan(s). If both sides contain a
-        * RemoteSubplan then its possible that they are marked for execution on
-        * different nodes, but that does not matter since tables are replicated
-        * and nodes are picked randomly for replicated tables. So just reuse
-        * either of the RemoteSubplan and pin the RecursiveUnion plan generated
-        * above to the RemoteSubplan. They must have been already removed from the
-        * subtree by find_delete_push_down_plan function
-        *
-        * XXX For tables replicated on different subsets of nodes, this may not
-        * work. In fact, we probably can't support recursive queries for such
-        * tables.
-        */
-       left_pushdown = find_delete_push_down_plan(root, lefttree, true, &plan->lefttree);
-       right_pushdown = find_delete_push_down_plan(root, righttree, true, &plan->righttree);
-       if (left_pushdown || right_pushdown)
-       {
-               /* Pick either one */
-               if (!left_pushdown)
-                       left_pushdown = right_pushdown;
-
-               /*
-                * Push the RecursiveUnion to the remote node
-                */
-               left_pushdown->scan.plan.lefttree = plan;
-
-               /*
-                * The only caller for this function does not really care if the
-                * returned node is RecursiveUnion or not. So we just return the
-                * RemoteSubplan as it
-                */
-               return (RecursiveUnion *) left_pushdown;
-       }
-#endif 
        return node;
 }
 

diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index 785cf5a99fa74032be49446887301592bf3d9a90..25226363920ed9772175534d69c4336f38ec321b 100644 (file)
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -418,6 +418,41 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
        }
 }
 
+/*
+ * remove RemoteSubquery from the top of the path
+ *
+ * Essentially find_push_down_plan() but applied when constructing the path,
+ * not when creating the plan. Compared to find_push_down_plan it only deals
+ * with a subset of node types, however.
+ *
+ * XXX Does this need to handle additional node types?
+ */
+static Path *
+strip_remote_subquery(PlannerInfo *root, Path *path)
+{
+       /* if there's RemoteSubplan at the top, we're trivially done */
+       if (IsA(path, RemoteSubPath))
+               return ((RemoteSubPath *)path)->subpath;
+
+       /* for subquery, we tweak the subpath (and descend into it) */
+       if (IsA(path, SubqueryScanPath))
+       {
+               SubqueryScanPath *subquery = (SubqueryScanPath *)path;
+               subquery->subpath = strip_remote_subquery(root, subquery->subpath);
+
+               subquery->path.param_info = subquery->subpath->param_info;
+               subquery->path.pathkeys = subquery->subpath->pathkeys;
+
+               /* also update the distribution */
+               subquery->path.distribution = copyObject(subquery->subpath->distribution);
+
+               /* recompute costs */
+               cost_subqueryscan(subquery, root, path->parent, subquery->path.param_info);
+       }
+
+       return path;
+}
+
 /*
  * Generate path for a recursive UNION node
  */
@@ -498,6 +533,32 @@ generate_recursion_path(SetOperationStmt *setOp, PlannerInfo *root,
                dNumGroups = lpath->rows + rpath->rows * 10;
        }
 
+       /*
+        * Push the resursive union (CTE) below Remote Subquery.
+        *
+        * We have already checked that all tables involved in the recursive CTE
+        * are replicated tables (or coordinator local tables such as catalogs).
+        * See subquery_planner for details. So here we search the left and right
+        * subpaths, and search for those remote subqueries.
+        *
+        * If either side contains a remote subquery, we remove those, and instead
+        * add a remote subquery on top of the recursive union later (we don't need
+        * to do that manually, it'll happen automatically).
+        *
+        * XXX The tables may be marked for execution on different nodes, but that
+        * does not matter since tables are replicated, and execution nodes are
+        * picked randomly.
+        *
+        * XXX For tables replicated on different groups of nodes, this may not
+        * work. We either need to pick a node from an intersection of the groups,
+        * or simply disable recursive queries on such tables.
+        *
+        * XXX This obviously breaks costing, because we're removing nodes that
+        * affected the cost (network transfers).
+        */
+       rpath = strip_remote_subquery(root, rpath);
+       lpath = strip_remote_subquery(root, lpath);
+
        /*
         * And make the path node.
         */

diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index c0ff99829cd6ad80ed14ee70ae5e902e3913a9e8..971ffa882268ef1899c5a899336c10329abb1b39 100644 (file)
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -4338,6 +4338,17 @@ create_recursiveunion_path(PlannerInfo *root,
        /* RecursiveUnion result is always unsorted */
        pathnode->path.pathkeys = NIL;
 
+       /*
+        * FIXME This assumes left/right path have the same distribution, or one
+        * of them is NULL. This is related to the subquery_planner() assuming all
+        * tables are replicated on the same group of nodes, which may or may not
+        * be the case, and we need to be more careful about it.
+        */
+       if (leftpath->distribution)
+               pathnode->path.distribution = copyObject(leftpath->distribution);
+       else
+               pathnode->path.distribution = copyObject(rightpath->distribution);
+
        pathnode->leftpath = leftpath;
        pathnode->rightpath = rightpath;
        pathnode->distinctList = distinctList;