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.

diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index e1c0ac996d58bd9258337f1e1a8c6f54fbdd70fd..49fc43bf1673ef14b24ef61706c6de1beacfbbf4 100644 (file)
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -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);

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 243e41d0cdea15f10e4ecc96510c19653ca11bca..d138728e6791316cd1bf62c3e3b1ef5a589b7378 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -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)
                                {

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 4cbaa5a88160a58e6d55b3eff20d4563cbd17b7d..64bc8a8e5977e508557c7a70ac7fde1d9cbdf518 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -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);

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 0161acf5223ec8ef2da6bd38ecc5287d9fb6f820..a2cd6deb612890ded150234f0ebb205cd5d6d2a9 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -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);
 

diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 777b273ac1701c42af1e4efbc230163f2962a58d..1c8d1052c58e0520c07130c9d0cea6966fc1cb3b 100644 (file)
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -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)
                {

diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c
index 5e11286ac7a4ef7c317eed7a350a7908256ff578..9e1c0e53333b3186d3264289205b9152abe9e04b 100644 (file)
--- a/src/backend/optimizer/util/placeholder.c
+++ b/src/backend/optimizer/util/placeholder.c
@@ -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);

diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index a389f42a22447077fea654300e3f2d1b78161c60..292e1f4aac390b87a66f6346c0950ca9d36dd2b1 100644 (file)
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -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)

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 6e75a39bf3f086b4c171d59544f337155f7de01c..d396ef142f9563ce5f620152983a873c082439c8 100644 (file)
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -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);
 
                /*

diff --git a/src/include/optimizer/var.h b/src/include/optimizer/var.h
index d1b0978b97ea83299b7edd68e3f759cf1f430728..463e75b8282f9be220ce702a4682e1df28edad3c 100644 (file)
--- a/src/include/optimizer/var.h
+++ b/src/include/optimizer/var.h
@@ -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 */