From 370897446063682e7a51b5585a3604bd27997ecf Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 12 Mar 2018 13:16:30 -0400 Subject: [PATCH 3/5] Add new upper rel to represent projecting toplevel scan/join rel. UPPERREL_TLIST represents the result of applying the scan/join target to the final scan/join relation. This requires us to use create_projection_path() rather than apply_projection_to_path() when projection non-partial paths from the scan/join relation, but it also enables us to avoid needing to modify those paths in place. It also avoids the need to clear the topmost scan/join rel's partial_pathlist when the scan/join target is not parallel-safe, which is sort of hack; see commit 3bf05e096b9f8375e640c5d7996aa57efd7f240c for an example of a previous fix that eliminated a similar hack. Patch by me. --- src/backend/optimizer/plan/planner.c | 69 +++++++++++++----------------------- src/include/nodes/relation.h | 1 + 2 files changed, 25 insertions(+), 45 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 24e6c46396..7532990548 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -1694,6 +1694,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, List *scanjoin_targets_contain_srfs; bool scanjoin_target_parallel_safe; bool have_grouping; + RelOptInfo *tlist_rel; AggClauseCosts agg_costs; WindowFuncLists *wflists = NULL; List *activeWindows = NIL; @@ -1876,6 +1877,14 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, scanjoin_target_parallel_safe = grouping_target_parallel_safe; } + /* + * Create a new upper relation to represent the result of scan/join + * projection. + */ + tlist_rel = fetch_upper_rel(root, UPPERREL_TLIST, NULL); + tlist_rel->consider_parallel = current_rel->consider_parallel && + scanjoin_target_parallel_safe; + /* * If there are any SRFs in the targetlist, we must separate each of * these PathTargets into SRF-computing and SRF-free targets. Replace @@ -1919,15 +1928,8 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, } /* - * Forcibly apply SRF-free scan/join target to all the Paths for the - * scan/join rel. - * - * In principle we should re-run set_cheapest() here to identify the - * cheapest path, but it seems unlikely that adding the same tlist - * eval costs to all the paths would change that, so we don't bother. - * Instead, just assume that the cheapest-startup and cheapest-total - * paths remain so. (There should be no parameterized paths anymore, - * so we needn't worry about updating cheapest_parameterized_paths.) + * Apply SRF-free scan/join target to all Paths for the scanjoin rel + * to produce paths for the tlist rel. */ foreach(lc, current_rel->pathlist) { @@ -1935,28 +1937,16 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, Path *path; Assert(subpath->param_info == NULL); - path = apply_projection_to_path(root, current_rel, - subpath, scanjoin_target); - /* If we had to add a Result, path is different from subpath */ - if (path != subpath) - { - lfirst(lc) = path; - if (subpath == current_rel->cheapest_startup_path) - current_rel->cheapest_startup_path = path; - if (subpath == current_rel->cheapest_total_path) - current_rel->cheapest_total_path = path; - } + path = (Path *) create_projection_path(root, tlist_rel, + subpath, scanjoin_target); + add_path(tlist_rel, path); } /* - * Upper planning steps which make use of the top scan/join rel's - * partial pathlist will expect partial paths for that rel to produce - * the same output as complete paths ... and we just changed the - * output for the complete paths, so we'll need to do the same thing - * for partial paths. But only parallel-safe expressions can be - * computed by partial paths. + * If we can produce partial paths for the tlist rel, for possible use + * by upper planning stages, do so. */ - if (current_rel->partial_pathlist && scanjoin_target_parallel_safe) + if (tlist_rel->consider_parallel) { /* Apply the scan/join target to each partial path */ foreach(lc, current_rel->partial_pathlist) @@ -1967,35 +1957,24 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, /* Shouldn't have any parameterized paths anymore */ Assert(subpath->param_info == NULL); - /* - * Don't use apply_projection_to_path() here, because there - * could be other pointers to these paths, and therefore we - * mustn't modify them in place. - */ newpath = (Path *) create_projection_path(root, - current_rel, + tlist_rel, subpath, scanjoin_target); - lfirst(lc) = newpath; + add_partial_path(tlist_rel, newpath); } } - else - { - /* - * In the unfortunate event that scanjoin_target is not - * parallel-safe, we can't apply it to the partial paths; in that - * case, we'll need to forget about the partial paths, which - * aren't valid input for upper planning steps. - */ - current_rel->partial_pathlist = NIL; - } /* Now fix things up if scan/join target contains SRFs */ if (parse->hasTargetSRFs) - adjust_paths_for_srfs(root, current_rel, + adjust_paths_for_srfs(root, tlist_rel, scanjoin_targets, scanjoin_targets_contain_srfs); + /* Now consider the tlist_rel to be the current upper relation. */ + set_cheapest(tlist_rel); + current_rel = tlist_rel; + /* * Save the various upper-rel PathTargets we just computed into * root->upper_targets[]. The core code doesn't use this, but it diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index d576aa7350..8d873bdee1 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -71,6 +71,7 @@ typedef struct AggClauseCosts typedef enum UpperRelationKind { UPPERREL_SETOP, /* result of UNION/INTERSECT/EXCEPT, if any */ + UPPERREL_TLIST, /* result of projecting final scan/join rel */ UPPERREL_PARTIAL_GROUP_AGG, /* result of partial grouping/aggregation, if * any */ UPPERREL_GROUP_AGG, /* result of grouping/aggregation, if any */ -- 2.14.3 (Apple Git-98)