Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit c44d013

Browse files
committed
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.
1 parent f81c966 commit c44d013

File tree

2 files changed

+34
-20
lines changed

2 files changed

+34
-20
lines changed

src/backend/optimizer/plan/initsplan.c

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ add_base_rels_to_query(PlannerInfo *root, Node *jtnode)
137137
/*
138138
* build_base_rel_tlists
139139
* Add targetlist entries for each var needed in the query's final tlist
140-
* to the appropriate base relations.
140+
* (and HAVING clause, if any) to the appropriate base relations.
141141
*
142142
* We mark such vars as needed by "relation 0" to ensure that they will
143143
* propagate up through all join plan steps.
@@ -154,6 +154,23 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
154154
add_vars_to_targetlist(root, tlist_vars, bms_make_singleton(0), true);
155155
list_free(tlist_vars);
156156
}
157+
158+
/*
159+
* If there's a HAVING clause, we'll need the Vars it uses, too.
160+
*/
161+
if (root->parse->havingQual)
162+
{
163+
List *having_vars = pull_var_clause(root->parse->havingQual,
164+
PVC_RECURSE_AGGREGATES,
165+
PVC_INCLUDE_PLACEHOLDERS);
166+
167+
if (having_vars != NIL)
168+
{
169+
add_vars_to_targetlist(root, having_vars,
170+
bms_make_singleton(0), true);
171+
list_free(having_vars);
172+
}
173+
}
157174
}
158175

159176
/*

src/backend/optimizer/plan/planner.c

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,9 +1415,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
14151415
else
14161416
{
14171417
/* No set operations, do regular planning */
1418-
List *sub_tlist;
1419-
AttrNumber *groupColIdx = NULL;
1420-
bool need_tlist_eval = true;
14211418
long numGroups = 0;
14221419
AggClauseCosts agg_costs;
14231420
int numGroupCols;
@@ -1548,13 +1545,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
15481545
parse->hasWindowFuncs = false;
15491546
}
15501547

1551-
/*
1552-
* Generate appropriate target list for subplan; may be different from
1553-
* tlist if grouping or aggregation is needed.
1554-
*/
1555-
sub_tlist = make_subplanTargetList(root, tlist,
1556-
&groupColIdx, &need_tlist_eval);
1557-
15581548
/*
15591549
* Do aggregate preprocessing, if the query has any aggs.
15601550
*
@@ -1612,7 +1602,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
16121602
* standard_qp_callback) pathkey representations of the query's sort
16131603
* clause, distinct clause, etc.
16141604
*/
1615-
final_rel = query_planner(root, sub_tlist,
1605+
final_rel = query_planner(root, tlist,
16161606
standard_qp_callback, &qp_extra);
16171607

16181608
/*
@@ -1888,6 +1878,9 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
18881878
* Normal case --- create a plan according to query_planner's
18891879
* results.
18901880
*/
1881+
List *sub_tlist;
1882+
AttrNumber *groupColIdx = NULL;
1883+
bool need_tlist_eval = true;
18911884
bool need_sort_for_grouping = false;
18921885

18931886
result_plan = create_plan(root, best_path);
@@ -1896,23 +1889,27 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
18961889
/* Detect if we'll need an explicit sort for grouping */
18971890
if (parse->groupClause && !use_hashed_grouping &&
18981891
!pathkeys_contained_in(root->group_pathkeys, current_pathkeys))
1899-
{
19001892
need_sort_for_grouping = true;
19011893

1902-
/*
1903-
* Always override create_plan's tlist, so that we don't sort
1904-
* useless data from a "physical" tlist.
1905-
*/
1906-
need_tlist_eval = true;
1907-
}
1894+
/*
1895+
* Generate appropriate target list for scan/join subplan; may be
1896+
* different from tlist if grouping or aggregation is needed.
1897+
*/
1898+
sub_tlist = make_subplanTargetList(root, tlist,
1899+
&groupColIdx,
1900+
&need_tlist_eval);
19081901

19091902
/*
19101903
* create_plan returns a plan with just a "flat" tlist of required
19111904
* Vars. Usually we need to insert the sub_tlist as the tlist of
19121905
* the top plan node. However, we can skip that if we determined
19131906
* that whatever create_plan chose to return will be good enough.
1907+
*
1908+
* If we need_sort_for_grouping, always override create_plan's
1909+
* tlist, so that we don't sort useless data from a "physical"
1910+
* tlist.
19141911
*/
1915-
if (need_tlist_eval)
1912+
if (need_tlist_eval || need_sort_for_grouping)
19161913
{
19171914
/*
19181915
* If the top-level plan node is one that cannot do expression

0 commit comments

Comments
 (0)