Skip to content

Commit c82c92b

Browse files
tglsfdctglsfdc
committed
Give pull_var_clause() reject/recurse/return behavior for WindowFuncs too.
All along, this function should have treated WindowFuncs in a manner similar to Aggrefs, ie with an option whether or not to recurse into them. By not considering the case, it was always recursing, which is OK for most callers (although I suspect that the case in prepare_sort_from_pathkeys might represent a bug). But now we need return-without-recursing behavior as well. There are also more than a few callers that should never see a WindowFunc, and now we'll get some error checking on that.
1 parent fd31cd2 commit c82c92b

File tree

9 files changed

+47
-7
lines changed

9 files changed

+47
-7
lines changed

‎src/backend/optimizer/path/equivclass.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,7 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
911911
EquivalenceMember *cur_em = (EquivalenceMember *) lfirst(lc);
912912
List *vars = pull_var_clause((Node *) cur_em->em_expr,
913913
PVC_RECURSE_AGGREGATES |
914+
PVC_RECURSE_WINDOWFUNCS |
914915
PVC_INCLUDE_PLACEHOLDERS);
915916

916917
add_vars_to_targetlist(root, vars, ec->ec_relids, false);

‎src/backend/optimizer/plan/createplan.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5307,7 +5307,8 @@ prepare_sort_from_pathkeys(Plan *lefttree, List *pathkeys,
53075307
* that we treat Aggrefs as if they were variables; this is
53085308
* necessary when attempting to sort the output from an Agg node
53095309
* for use in a WindowFunc (since grouping_planner will have
5310-
* treated the Aggrefs as variables, too).
5310+
* treated the Aggrefs as variables, too). Likewise, if we find a
5311+
* WindowFunc in a sort expression, treat it as a variable.
53115312
*/
53125313
Expr *sortexpr = NULL;
53135314

@@ -5336,6 +5337,7 @@ prepare_sort_from_pathkeys(Plan *lefttree, List *pathkeys,
53365337
sortexpr = em->em_expr;
53375338
exprvars = pull_var_clause((Node *) sortexpr,
53385339
PVC_INCLUDE_AGGREGATES |
5340+
PVC_INCLUDE_WINDOWFUNCS |
53395341
PVC_INCLUDE_PLACEHOLDERS);
53405342
foreach(k, exprvars)
53415343
{

‎src/backend/optimizer/plan/initsplan.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
147147
{
148148
List *tlist_vars = pull_var_clause((Node *) final_tlist,
149149
PVC_RECURSE_AGGREGATES |
150+
PVC_RECURSE_WINDOWFUNCS |
150151
PVC_INCLUDE_PLACEHOLDERS);
151152

152153
if (tlist_vars != NIL)
@@ -156,7 +157,8 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
156157
}
157158

158159
/*
159-
* If there's a HAVING clause, we'll need the Vars it uses, too.
160+
* If there's a HAVING clause, we'll need the Vars it uses, too. Note
161+
* that HAVING can contain Aggrefs but not WindowFuncs.
160162
*/
161163
if (root->parse->havingQual)
162164
{
@@ -1788,6 +1790,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
17881790
{
17891791
List *vars = pull_var_clause(clause,
17901792
PVC_RECURSE_AGGREGATES |
1793+
PVC_RECURSE_WINDOWFUNCS |
17911794
PVC_INCLUDE_PLACEHOLDERS);
17921795

17931796
add_vars_to_targetlist(root, vars, relids, false);

‎src/backend/optimizer/plan/planner.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3837,11 +3837,12 @@ make_group_input_target(PlannerInfo *root, List *tlist)
38373837
* add them to the result tlist if not already present. (A Var used
38383838
* directly as a GROUP BY item will be present already.) Note this
38393839
* includes Vars used in resjunk items, so we are covering the needs of
3840-
* ORDER BY and window specifications. Vars used within Aggrefs will be
3841-
* pulled out here, too.
3840+
* ORDER BY and window specifications. Vars used within Aggrefs and
3841+
* WindowFuncs will be pulled out here, too.
38423842
*/
38433843
non_group_vars = pull_var_clause((Node *) non_group_cols,
38443844
PVC_RECURSE_AGGREGATES |
3845+
PVC_RECURSE_WINDOWFUNCS |
38453846
PVC_INCLUDE_PLACEHOLDERS);
38463847
sub_tlist = add_to_flat_tlist(sub_tlist, non_group_vars);
38473848

@@ -4086,10 +4087,12 @@ make_window_input_target(PlannerInfo *root,
40864087
*
40874088
* Note: it's essential to use PVC_INCLUDE_AGGREGATES here, so that the
40884089
* Aggrefs are placed in the Agg node's tlist and not left to be computed
4089-
* at higher levels.
4090+
* at higher levels. On the other hand, we should recurse into
4091+
* WindowFuncs to make sure their input expressions are available.
40904092
*/
40914093
flattenable_vars = pull_var_clause((Node *) flattenable_cols,
40924094
PVC_INCLUDE_AGGREGATES |
4095+
PVC_RECURSE_WINDOWFUNCS |
40934096
PVC_INCLUDE_PLACEHOLDERS);
40944097
new_tlist = add_to_flat_tlist(new_tlist, flattenable_vars);
40954098

‎src/backend/optimizer/prep/preptlist.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
169169

170170
vars = pull_var_clause((Node *) parse->returningList,
171171
PVC_RECURSE_AGGREGATES |
172+
PVC_RECURSE_WINDOWFUNCS |
172173
PVC_INCLUDE_PLACEHOLDERS);
173174
foreach(l, vars)
174175
{

‎src/backend/optimizer/util/placeholder.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ find_placeholders_in_expr(PlannerInfo *root, Node *expr)
221221
*/
222222
vars = pull_var_clause(expr,
223223
PVC_RECURSE_AGGREGATES |
224+
PVC_RECURSE_WINDOWFUNCS |
224225
PVC_INCLUDE_PLACEHOLDERS);
225226
foreach(vl, vars)
226227
{
@@ -355,6 +356,7 @@ fix_placeholder_input_needed_levels(PlannerInfo *root)
355356
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc);
356357
List *vars = pull_var_clause((Node *) phinfo->ph_var->phexpr,
357358
PVC_RECURSE_AGGREGATES |
359+
PVC_RECURSE_WINDOWFUNCS |
358360
PVC_INCLUDE_PLACEHOLDERS);
359361

360362
add_vars_to_targetlist(root, vars, phinfo->ph_eval_at, false);

‎src/backend/optimizer/util/var.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,13 @@ locate_var_of_level_walker(Node *node,
503503
* Vars within an Aggref's expression are included in the result only
504504
* when PVC_RECURSE_AGGREGATES is specified.
505505
*
506+
* WindowFuncs are handled according to these bits in 'flags':
507+
* PVC_INCLUDE_WINDOWFUNCS include WindowFuncs in output list
508+
* PVC_RECURSE_WINDOWFUNCS recurse into WindowFunc arguments
509+
* neither flag throw error if WindowFunc found
510+
* Vars within a WindowFunc's expression are included in the result only
511+
* when PVC_RECURSE_WINDOWFUNCS is specified.
512+
*
506513
* PlaceHolderVars are handled according to these bits in 'flags':
507514
* PVC_INCLUDE_PLACEHOLDERS include PlaceHolderVars in output list
508515
* PVC_RECURSE_PLACEHOLDERS recurse into PlaceHolderVar arguments
@@ -532,6 +539,8 @@ pull_var_clause(Node *node, int flags)
532539
/* Assert that caller has not specified inconsistent flags */
533540
Assert((flags & (PVC_INCLUDE_AGGREGATES | PVC_RECURSE_AGGREGATES))
534541
!= (PVC_INCLUDE_AGGREGATES | PVC_RECURSE_AGGREGATES));
542+
Assert((flags & (PVC_INCLUDE_WINDOWFUNCS | PVC_RECURSE_WINDOWFUNCS))
543+
!= (PVC_INCLUDE_WINDOWFUNCS | PVC_RECURSE_WINDOWFUNCS));
535544
Assert((flags & (PVC_INCLUDE_PLACEHOLDERS | PVC_RECURSE_PLACEHOLDERS))
536545
!= (PVC_INCLUDE_PLACEHOLDERS | PVC_RECURSE_PLACEHOLDERS));
537546

@@ -594,6 +603,22 @@ pull_var_clause_walker(Node *node, pull_var_clause_context *context)
594603
else
595604
elog(ERROR, "GROUPING found where not expected");
596605
}
606+
else if (IsA(node, WindowFunc))
607+
{
608+
/* WindowFuncs have no levelsup field to check ... */
609+
if (context->flags & PVC_INCLUDE_WINDOWFUNCS)
610+
{
611+
context->varlist = lappend(context->varlist, node);
612+
/* we do NOT descend into the contained expressions */
613+
return false;
614+
}
615+
else if (context->flags & PVC_RECURSE_WINDOWFUNCS)
616+
{
617+
/* fall through to recurse into the windowfunc's arguments */
618+
}
619+
else
620+
elog(ERROR, "WindowFunc found where not expected");
621+
}
597622
else if (IsA(node, PlaceHolderVar))
598623
{
599624
if (((PlaceHolderVar *) node)->phlevelsup != 0)

‎src/backend/utils/adt/selfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3329,6 +3329,7 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows,
33293329
*/
33303330
varshere = pull_var_clause(groupexpr,
33313331
PVC_RECURSE_AGGREGATES |
3332+
PVC_RECURSE_WINDOWFUNCS |
33323333
PVC_RECURSE_PLACEHOLDERS);
33333334

33343335
/*

‎src/include/optimizer/var.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@
1919
/* Bits that can be OR'd into the flags argument of pull_var_clause() */
2020
#define PVC_INCLUDE_AGGREGATES 0x0001 /* include Aggrefs in output list */
2121
#define PVC_RECURSE_AGGREGATES 0x0002 /* recurse into Aggref arguments */
22-
#define PVC_INCLUDE_PLACEHOLDERS 0x0004 /* include PlaceHolderVars in
22+
#define PVC_INCLUDE_WINDOWFUNCS 0x0004 /* include WindowFuncs in output list */
23+
#define PVC_RECURSE_WINDOWFUNCS 0x0008 /* recurse into WindowFunc arguments */
24+
#define PVC_INCLUDE_PLACEHOLDERS 0x0010 /* include PlaceHolderVars in
2325
* output list */
24-
#define PVC_RECURSE_PLACEHOLDERS 0x0008 /* recurse into PlaceHolderVar
26+
#define PVC_RECURSE_PLACEHOLDERS 0x0020 /* recurse into PlaceHolderVar
2527
* arguments */
2628

2729

0 commit comments

Comments
 (0)