Adjust reltarget assignment for UPPERREL_PARTIAL_DISTINCT rel

A comment in grouping_planner() claimed that the PlannerInfo
upper_targets array was not used in core code.  However, the code that
generated the paths for the UPPERREL_PARTIAL_DISTINCT rel made that
comment untrue.

Here we adjust the create_distinct_paths() function signature to pass
down the PathTarget the same as is done for create_grouping_paths(),
thus making the aforementioned comment true again.

In passing adjust the order of the upper_targets[] assignments.  These
seem to be following the reverse enum order apart from
UPPERREL_PARTIAL_DISTINCT.

Also, update the header comment for generate_gather_paths() to mention
the function is also used to create gather paths for partial distinct
paths.

Author: Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAMbWs48u9VoVOouJsys1qOaC9WVGVmBa+wT1dx8KvxF5GPzezA@mail.gmail.com

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 84c4de488a6352f33c0ca2287f818c7ce4731e28..d404fbf262c218600237746088ece7b6266d1580 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -3053,10 +3053,10 @@ set_worktable_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
  *
  * If we're generating paths for a scan or join relation, override_rows will
  * be false, and we'll just use the relation's size estimate.  When we're
- * being called for a partially-grouped path, though, we need to override
- * the rowcount estimate.  (It's not clear that the particular value we're
- * using here is actually best, but the underlying rel has no estimate so
- * we must do something.)
+ * being called for a partially-grouped or partially-distinct path, though, we
+ * need to override the rowcount estimate.  (It's not clear that the
+ * particular value we're using here is actually best, but the underlying rel
+ * has no estimate so we must do something.)
  */
 void
 generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows)

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index acc324122fde92f409c678010b1ad83672cab68c..be4e182869a19f648aae55c3161a3d4444abf72a 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -190,10 +190,12 @@ static void create_one_window_path(PlannerInfo *root,
                                   WindowFuncLists *wflists,
                                   List *activeWindows);
 static RelOptInfo *create_distinct_paths(PlannerInfo *root,
-                                        RelOptInfo *input_rel);
+                                        RelOptInfo *input_rel,
+                                        PathTarget *target);
 static void create_partial_distinct_paths(PlannerInfo *root,
                                          RelOptInfo *input_rel,
-                                         RelOptInfo *final_distinct_rel);
+                                         RelOptInfo *final_distinct_rel,
+                                         PathTarget *target);
 static RelOptInfo *create_final_distinct_paths(PlannerInfo *root,
                                               RelOptInfo *input_rel,
                                               RelOptInfo *distinct_rel);
@@ -1644,8 +1646,8 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
         */
        root->upper_targets[UPPERREL_FINAL] = final_target;
        root->upper_targets[UPPERREL_ORDERED] = final_target;
-       root->upper_targets[UPPERREL_PARTIAL_DISTINCT] = sort_input_target;
        root->upper_targets[UPPERREL_DISTINCT] = sort_input_target;
+       root->upper_targets[UPPERREL_PARTIAL_DISTINCT] = sort_input_target;
        root->upper_targets[UPPERREL_WINDOW] = sort_input_target;
        root->upper_targets[UPPERREL_GROUP_AGG] = grouping_target;
 
@@ -1695,7 +1697,8 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
        if (parse->distinctClause)
        {
            current_rel = create_distinct_paths(root,
-                                               current_rel);
+                                               current_rel,
+                                               sort_input_target);
        }
    }                           /* end of if (setOperations) */
 
@@ -4568,12 +4571,14 @@ create_one_window_path(PlannerInfo *root,
  * Build a new upperrel containing Paths for SELECT DISTINCT evaluation.
  *
  * input_rel: contains the source-data Paths
+ * target: the pathtarget for the result Paths to compute
  *
  * Note: input paths should already compute the desired pathtarget, since
  * Sort/Unique won't project anything.
  */
 static RelOptInfo *
-create_distinct_paths(PlannerInfo *root, RelOptInfo *input_rel)
+create_distinct_paths(PlannerInfo *root, RelOptInfo *input_rel,
+                     PathTarget *target)
 {
    RelOptInfo *distinct_rel;
 
@@ -4601,7 +4606,7 @@ create_distinct_paths(PlannerInfo *root, RelOptInfo *input_rel)
    create_final_distinct_paths(root, input_rel, distinct_rel);
 
    /* now build distinct paths based on input_rel's partial_pathlist */
-   create_partial_distinct_paths(root, input_rel, distinct_rel);
+   create_partial_distinct_paths(root, input_rel, distinct_rel, target);
 
    /* Give a helpful error if we failed to create any paths */
    if (distinct_rel->pathlist == NIL)
@@ -4643,7 +4648,8 @@ create_distinct_paths(PlannerInfo *root, RelOptInfo *input_rel)
  */
 static void
 create_partial_distinct_paths(PlannerInfo *root, RelOptInfo *input_rel,
-                             RelOptInfo *final_distinct_rel)
+                             RelOptInfo *final_distinct_rel,
+                             PathTarget *target)
 {
    RelOptInfo *partial_distinct_rel;
    Query      *parse;
@@ -4664,7 +4670,7 @@ create_partial_distinct_paths(PlannerInfo *root, RelOptInfo *input_rel,
 
    partial_distinct_rel = fetch_upper_rel(root, UPPERREL_PARTIAL_DISTINCT,
                                           NULL);
-   partial_distinct_rel->reltarget = root->upper_targets[UPPERREL_PARTIAL_DISTINCT];
+   partial_distinct_rel->reltarget = target;
    partial_distinct_rel->consider_parallel = input_rel->consider_parallel;
 
    /*