Push target list evaluation through Gather Merge.

We already do this for Gather, but it got overlooked for Gather Merge.

Amit Kapila, with review and minor revisions by Rushabh Lathia
and by me.

Discussion: http://postgr.es/m/CAA4eK1KUC5Uyu7qaifxrjpHxbSeoQh3yzwN3bThnJsmJcZ-qtA@mail.gmail.com

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 607f7cd25187ae29999a0658199c201864d76158..90fd9cc9598a8fd36c218174de9f8aa7c59a870c 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -5060,7 +5060,8 @@ create_ordered_paths(PlannerInfo *root,
                        path = (Path *)
                                create_gather_merge_path(root, ordered_rel,
                                                                                 path,
-                                                                                target, root->sort_pathkeys, NULL,
+                                                                                path->pathtarget,
+                                                                                root->sort_pathkeys, NULL,
                                                                                 &total_groups);
 
                        /* Add projection step if needed */

diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 36ec025b05ba5b894d3ac3149e5fa141fbc2f52d..68dee0f5010b3923f13dd8f4832ac805d1b254ae 100644 (file)
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -2368,9 +2368,9 @@ create_projection_path(PlannerInfo *root,
  * knows that the given path isn't referenced elsewhere and so can be modified
  * in-place.
  *
- * If the input path is a GatherPath, we try to push the new target down to
- * its input as well; this is a yet more invasive modification of the input
- * path, which create_projection_path() can't do.
+ * If the input path is a GatherPath or GatherMergePath, we try to push the
+ * new target down to its input as well; this is a yet more invasive
+ * modification of the input path, which create_projection_path() can't do.
  *
  * Note that we mustn't change the source path's parent link; so when it is
  * add_path'd to "rel" things will be a bit inconsistent.  So far that has
@@ -2407,31 +2407,44 @@ apply_projection_to_path(PlannerInfo *root,
                (target->cost.per_tuple - oldcost.per_tuple) * path->rows;
 
        /*
-        * If the path happens to be a Gather path, we'd like to arrange for the
-        * subpath to return the required target list so that workers can help
-        * project.  But if there is something that is not parallel-safe in the
-        * target expressions, then we can't.
+        * If the path happens to be a Gather or GatherMerge path, we'd like to
+        * arrange for the subpath to return the required target list so that
+        * workers can help project.  But if there is something that is not
+        * parallel-safe in the target expressions, then we can't.
         */
-       if (IsA(path, GatherPath) &&
+       if ((IsA(path, GatherPath) || IsA(path, GatherMergePath)) &&
                is_parallel_safe(root, (Node *) target->exprs))
        {
-               GatherPath *gpath = (GatherPath *) path;
-
                /*
                 * We always use create_projection_path here, even if the subpath is
                 * projection-capable, so as to avoid modifying the subpath in place.
                 * It seems unlikely at present that there could be any other
                 * references to the subpath, but better safe than sorry.
                 *
-                * Note that we don't change the GatherPath's cost estimates; it might
+                * Note that we don't change the parallel path's cost estimates; it might
                 * be appropriate to do so, to reflect the fact that the bulk of the
                 * target evaluation will happen in workers.
                 */
-               gpath->subpath = (Path *)
-                       create_projection_path(root,
-                                                                  gpath->subpath->parent,
-                                                                  gpath->subpath,
-                                                                  target);
+               if (IsA(path, GatherPath))
+               {
+                       GatherPath *gpath = (GatherPath *) path;
+
+                       gpath->subpath = (Path *)
+                               create_projection_path(root,
+                                                                          gpath->subpath->parent,
+                                                                          gpath->subpath,
+                                                                          target);
+               }
+               else
+               {
+                       GatherMergePath *gmpath = (GatherMergePath *) path;
+
+                       gmpath->subpath = (Path *)
+                               create_projection_path(root,
+                                                                          gmpath->subpath->parent,
+                                                                          gmpath->subpath,
+                                                                          target);
+               }
        }
        else if (path->parallel_safe &&
                         !is_parallel_safe(root, (Node *) target->exprs))

diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out
index ac9ad0668d17c75ac8f4bd21f4613946f321b39c..6f04769e3ea0f50e67ceb797dc12d26bcb7d6306 100644 (file)
--- a/src/test/regress/expected/select_parallel.out
+++ b/src/test/regress/expected/select_parallel.out
@@ -375,6 +375,31 @@ select count(*) from tenk1 group by twenty;
    500
 (20 rows)
 
+--test expressions in targetlist are pushed down for gather merge
+create or replace function simple_func(var1 integer) returns integer
+as $$
+begin
+        return var1 + 10;
+end;
+$$ language plpgsql PARALLEL SAFE;
+explain (costs off, verbose)
+    select ten, simple_func(ten) from tenk1 where ten < 100 order by ten;
+                     QUERY PLAN                      
+-----------------------------------------------------
+ Gather Merge
+   Output: ten, (simple_func(ten))
+   Workers Planned: 4
+   ->  Result
+         Output: ten, simple_func(ten)
+         ->  Sort
+               Output: ten
+               Sort Key: tenk1.ten
+               ->  Parallel Seq Scan on public.tenk1
+                     Output: ten
+                     Filter: (tenk1.ten < 100)
+(11 rows)
+
+drop function simple_func(integer);
 --test rescan behavior of gather merge
 set enable_material = false;
 explain (costs off)

diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql
index 495f0335dccd1120aa73b6c69fbf601f0c3fcd76..9c1b87abdfc87109921c3b4e3dc42c033c0cb942 100644 (file)
--- a/src/test/regress/sql/select_parallel.sql
+++ b/src/test/regress/sql/select_parallel.sql
@@ -144,6 +144,19 @@ explain (costs off)
 
 select count(*) from tenk1 group by twenty;
 
+--test expressions in targetlist are pushed down for gather merge
+create or replace function simple_func(var1 integer) returns integer
+as $$
+begin
+        return var1 + 10;
+end;
+$$ language plpgsql PARALLEL SAFE;
+
+explain (costs off, verbose)
+    select ten, simple_func(ten) from tenk1 where ten < 100 order by ten;
+
+drop function simple_func(integer);
+
 --test rescan behavior of gather merge
 set enable_material = false;