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.
This commit is contained in:
Tom Lane 2016-03-10 16:23:40 -05:00
parent fd31cd2651
commit c82c92b111
9 changed files with 47 additions and 7 deletions

View File

@ -911,6 +911,7 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
EquivalenceMember *cur_em = (EquivalenceMember *) lfirst(lc);
List *vars = pull_var_clause((Node *) cur_em->em_expr,
PVC_RECURSE_AGGREGATES |
PVC_RECURSE_WINDOWFUNCS |
PVC_INCLUDE_PLACEHOLDERS);
add_vars_to_targetlist(root, vars, ec->ec_relids, false);

View File

@ -5307,7 +5307,8 @@ prepare_sort_from_pathkeys(Plan *lefttree, List *pathkeys,
* that we treat Aggrefs as if they were variables; this is
* necessary when attempting to sort the output from an Agg node
* for use in a WindowFunc (since grouping_planner will have
* treated the Aggrefs as variables, too).
* treated the Aggrefs as variables, too). Likewise, if we find a
* WindowFunc in a sort expression, treat it as a variable.
*/
Expr *sortexpr = NULL;
@ -5336,6 +5337,7 @@ prepare_sort_from_pathkeys(Plan *lefttree, List *pathkeys,
sortexpr = em->em_expr;
exprvars = pull_var_clause((Node *) sortexpr,
PVC_INCLUDE_AGGREGATES |
PVC_INCLUDE_WINDOWFUNCS |
PVC_INCLUDE_PLACEHOLDERS);
foreach(k, exprvars)
{

View File

@ -147,6 +147,7 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
{
List *tlist_vars = pull_var_clause((Node *) final_tlist,
PVC_RECURSE_AGGREGATES |
PVC_RECURSE_WINDOWFUNCS |
PVC_INCLUDE_PLACEHOLDERS);
if (tlist_vars != NIL)
@ -156,7 +157,8 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
}
/*
* If there's a HAVING clause, we'll need the Vars it uses, too.
* If there's a HAVING clause, we'll need the Vars it uses, too. Note
* that HAVING can contain Aggrefs but not WindowFuncs.
*/
if (root->parse->havingQual)
{
@ -1788,6 +1790,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
{
List *vars = pull_var_clause(clause,
PVC_RECURSE_AGGREGATES |
PVC_RECURSE_WINDOWFUNCS |
PVC_INCLUDE_PLACEHOLDERS);
add_vars_to_targetlist(root, vars, relids, false);

View File

@ -3837,11 +3837,12 @@ make_group_input_target(PlannerInfo *root, List *tlist)
* add them to the result tlist if not already present. (A Var used
* directly as a GROUP BY item will be present already.) Note this
* includes Vars used in resjunk items, so we are covering the needs of
* ORDER BY and window specifications. Vars used within Aggrefs will be
* pulled out here, too.
* ORDER BY and window specifications. Vars used within Aggrefs and
* WindowFuncs will be pulled out here, too.
*/
non_group_vars = pull_var_clause((Node *) non_group_cols,
PVC_RECURSE_AGGREGATES |
PVC_RECURSE_WINDOWFUNCS |
PVC_INCLUDE_PLACEHOLDERS);
sub_tlist = add_to_flat_tlist(sub_tlist, non_group_vars);
@ -4086,10 +4087,12 @@ make_window_input_target(PlannerInfo *root,
*
* Note: it's essential to use PVC_INCLUDE_AGGREGATES here, so that the
* Aggrefs are placed in the Agg node's tlist and not left to be computed
* at higher levels.
* at higher levels. On the other hand, we should recurse into
* WindowFuncs to make sure their input expressions are available.
*/
flattenable_vars = pull_var_clause((Node *) flattenable_cols,
PVC_INCLUDE_AGGREGATES |
PVC_RECURSE_WINDOWFUNCS |
PVC_INCLUDE_PLACEHOLDERS);
new_tlist = add_to_flat_tlist(new_tlist, flattenable_vars);

View File

@ -169,6 +169,7 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
vars = pull_var_clause((Node *) parse->returningList,
PVC_RECURSE_AGGREGATES |
PVC_RECURSE_WINDOWFUNCS |
PVC_INCLUDE_PLACEHOLDERS);
foreach(l, vars)
{

View File

@ -221,6 +221,7 @@ find_placeholders_in_expr(PlannerInfo *root, Node *expr)
*/
vars = pull_var_clause(expr,
PVC_RECURSE_AGGREGATES |
PVC_RECURSE_WINDOWFUNCS |
PVC_INCLUDE_PLACEHOLDERS);
foreach(vl, vars)
{
@ -355,6 +356,7 @@ fix_placeholder_input_needed_levels(PlannerInfo *root)
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc);
List *vars = pull_var_clause((Node *) phinfo->ph_var->phexpr,
PVC_RECURSE_AGGREGATES |
PVC_RECURSE_WINDOWFUNCS |
PVC_INCLUDE_PLACEHOLDERS);
add_vars_to_targetlist(root, vars, phinfo->ph_eval_at, false);

View File

@ -503,6 +503,13 @@ locate_var_of_level_walker(Node *node,
* Vars within an Aggref's expression are included in the result only
* when PVC_RECURSE_AGGREGATES is specified.
*
* WindowFuncs are handled according to these bits in 'flags':
* PVC_INCLUDE_WINDOWFUNCS include WindowFuncs in output list
* PVC_RECURSE_WINDOWFUNCS recurse into WindowFunc arguments
* neither flag throw error if WindowFunc found
* Vars within a WindowFunc's expression are included in the result only
* when PVC_RECURSE_WINDOWFUNCS is specified.
*
* PlaceHolderVars are handled according to these bits in 'flags':
* PVC_INCLUDE_PLACEHOLDERS include PlaceHolderVars in output list
* PVC_RECURSE_PLACEHOLDERS recurse into PlaceHolderVar arguments
@ -532,6 +539,8 @@ pull_var_clause(Node *node, int flags)
/* Assert that caller has not specified inconsistent flags */
Assert((flags & (PVC_INCLUDE_AGGREGATES | PVC_RECURSE_AGGREGATES))
!= (PVC_INCLUDE_AGGREGATES | PVC_RECURSE_AGGREGATES));
Assert((flags & (PVC_INCLUDE_WINDOWFUNCS | PVC_RECURSE_WINDOWFUNCS))
!= (PVC_INCLUDE_WINDOWFUNCS | PVC_RECURSE_WINDOWFUNCS));
Assert((flags & (PVC_INCLUDE_PLACEHOLDERS | PVC_RECURSE_PLACEHOLDERS))
!= (PVC_INCLUDE_PLACEHOLDERS | PVC_RECURSE_PLACEHOLDERS));
@ -594,6 +603,22 @@ pull_var_clause_walker(Node *node, pull_var_clause_context *context)
else
elog(ERROR, "GROUPING found where not expected");
}
else if (IsA(node, WindowFunc))
{
/* WindowFuncs have no levelsup field to check ... */
if (context->flags & PVC_INCLUDE_WINDOWFUNCS)
{
context->varlist = lappend(context->varlist, node);
/* we do NOT descend into the contained expressions */
return false;
}
else if (context->flags & PVC_RECURSE_WINDOWFUNCS)
{
/* fall through to recurse into the windowfunc's arguments */
}
else
elog(ERROR, "WindowFunc found where not expected");
}
else if (IsA(node, PlaceHolderVar))
{
if (((PlaceHolderVar *) node)->phlevelsup != 0)

View File

@ -3329,6 +3329,7 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows,
*/
varshere = pull_var_clause(groupexpr,
PVC_RECURSE_AGGREGATES |
PVC_RECURSE_WINDOWFUNCS |
PVC_RECURSE_PLACEHOLDERS);
/*

View File

@ -19,9 +19,11 @@
/* Bits that can be OR'd into the flags argument of pull_var_clause() */
#define PVC_INCLUDE_AGGREGATES 0x0001 /* include Aggrefs in output list */
#define PVC_RECURSE_AGGREGATES 0x0002 /* recurse into Aggref arguments */
#define PVC_INCLUDE_PLACEHOLDERS 0x0004 /* include PlaceHolderVars in
#define PVC_INCLUDE_WINDOWFUNCS 0x0004 /* include WindowFuncs in output list */
#define PVC_RECURSE_WINDOWFUNCS 0x0008 /* recurse into WindowFunc arguments */
#define PVC_INCLUDE_PLACEHOLDERS 0x0010 /* include PlaceHolderVars in
* output list */
#define PVC_RECURSE_PLACEHOLDERS 0x0008 /* recurse into PlaceHolderVar
#define PVC_RECURSE_PLACEHOLDERS 0x0020 /* recurse into PlaceHolderVar
* arguments */