From: Tom Lane Date: Fri, 8 Jan 2016 01:23:57 +0000 (-0500) Subject: Delay creation of subplan tlist until after create_plan(). X-Git-Tag: REL9_6_BETA1~884 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c44d013835049053d19bc1795f0d169f3d1d6ff0;p=postgresql Delay creation of subplan tlist until after create_plan(). Once upon a time it was necessary for grouping_planner() to determine the tlist it wanted from the scan/join plan subtree before it called query_planner(), because query_planner() would actually make a Plan using that. But we refactored things a long time ago to delay construction of the Plan tree till later, so there's no need to build that tlist until (and indeed unless) we're ready to plaster it onto the Plan. The only thing query_planner() cares about is what Vars are going to be needed for the tlist, and it can perfectly well get that by looking at the real tlist rather than some masticated version. Well, actually, there is one minor glitch in that argument, which is that make_subplanTargetList also adds Vars appearing only in HAVING to the tlist it produces. So now we have to account for HAVING explicitly in build_base_rel_tlists. But that just adds a few lines of code, and I doubt it moves the needle much on processing time; we might be doing pull_var_clause() twice on the havingQual, but before we had it scanning dummy tlist entries instead. This is a very small down payment on rationalizing grouping_planner enough so it can be refactored. --- diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index efd40b2022..4a906a88a8 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -137,7 +137,7 @@ add_base_rels_to_query(PlannerInfo *root, Node *jtnode) /* * build_base_rel_tlists * Add targetlist entries for each var needed in the query's final tlist - * to the appropriate base relations. + * (and HAVING clause, if any) to the appropriate base relations. * * We mark such vars as needed by "relation 0" to ensure that they will * propagate up through all join plan steps. @@ -154,6 +154,23 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist) add_vars_to_targetlist(root, tlist_vars, bms_make_singleton(0), true); list_free(tlist_vars); } + + /* + * If there's a HAVING clause, we'll need the Vars it uses, too. + */ + if (root->parse->havingQual) + { + List *having_vars = pull_var_clause(root->parse->havingQual, + PVC_RECURSE_AGGREGATES, + PVC_INCLUDE_PLACEHOLDERS); + + if (having_vars != NIL) + { + add_vars_to_targetlist(root, having_vars, + bms_make_singleton(0), true); + list_free(having_vars); + } + } } /* diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 7f51d8ce4d..a4357639dc 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -1415,9 +1415,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) else { /* No set operations, do regular planning */ - List *sub_tlist; - AttrNumber *groupColIdx = NULL; - bool need_tlist_eval = true; long numGroups = 0; AggClauseCosts agg_costs; int numGroupCols; @@ -1548,13 +1545,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) parse->hasWindowFuncs = false; } - /* - * Generate appropriate target list for subplan; may be different from - * tlist if grouping or aggregation is needed. - */ - sub_tlist = make_subplanTargetList(root, tlist, - &groupColIdx, &need_tlist_eval); - /* * Do aggregate preprocessing, if the query has any aggs. * @@ -1612,7 +1602,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) * standard_qp_callback) pathkey representations of the query's sort * clause, distinct clause, etc. */ - final_rel = query_planner(root, sub_tlist, + final_rel = query_planner(root, tlist, standard_qp_callback, &qp_extra); /* @@ -1888,6 +1878,9 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) * Normal case --- create a plan according to query_planner's * results. */ + List *sub_tlist; + AttrNumber *groupColIdx = NULL; + bool need_tlist_eval = true; bool need_sort_for_grouping = false; result_plan = create_plan(root, best_path); @@ -1896,23 +1889,27 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) /* Detect if we'll need an explicit sort for grouping */ if (parse->groupClause && !use_hashed_grouping && !pathkeys_contained_in(root->group_pathkeys, current_pathkeys)) - { need_sort_for_grouping = true; - /* - * Always override create_plan's tlist, so that we don't sort - * useless data from a "physical" tlist. - */ - need_tlist_eval = true; - } + /* + * Generate appropriate target list for scan/join subplan; may be + * different from tlist if grouping or aggregation is needed. + */ + sub_tlist = make_subplanTargetList(root, tlist, + &groupColIdx, + &need_tlist_eval); /* * create_plan returns a plan with just a "flat" tlist of required * Vars. Usually we need to insert the sub_tlist as the tlist of * the top plan node. However, we can skip that if we determined * that whatever create_plan chose to return will be good enough. + * + * If we need_sort_for_grouping, always override create_plan's + * tlist, so that we don't sort useless data from a "physical" + * tlist. */ - if (need_tlist_eval) + if (need_tlist_eval || need_sort_for_grouping) { /* * If the top-level plan node is one that cannot do expression