Fix estimate_num_groups() to not fail on PlaceHolderVars, per report from

Stefan Kaltenbrunner.  The most reasonable behavior (at least for the near
term) seems to be to ignore the PlaceHolderVar and examine its argument
instead.  In support of this, change the API of pull_var_clause() to allow
callers to request recursion into PlaceHolderVars.  Currently
estimate_num_groups() is the only customer for that behavior, but where
there's one there may be others.

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index f6b52109fe96bfe5e85223712c810e5a071de457..bc03ef60331b362f0c69a1b94b30f75ded65dbb8 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.352 2009/03/31 17:59:56 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.353 2009/04/19 19:46:32 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1621,7 +1621,7 @@ StoreRelCheck(Relation rel, char *ccname, Node *expr,
     * in check constraints; it would fail to examine the contents of
     * subselects.
     */
-   varList = pull_var_clause(expr, false);
+   varList = pull_var_clause(expr, PVC_REJECT_PLACEHOLDERS);
    keycount = list_length(varList);
 
    if (keycount > 0)
@@ -1915,7 +1915,7 @@ AddRelationNewConstraints(Relation rel,
            List       *vars;
            char       *colname;
 
-           vars = pull_var_clause(expr, false);
+           vars = pull_var_clause(expr, PVC_REJECT_PLACEHOLDERS);
 
            /* eliminate duplicates */
            vars = list_union(NIL, vars);

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index a172c5de7ab2bb6e6a96fe90669a8bbe893774f1..fee9b8fac8626284583838382521deac4a7cea58 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.181 2009/03/10 20:58:26 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.182 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1122,7 +1122,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
     * Examine all Vars used in clause; since it's a restriction clause, all
     * such Vars must refer to subselect output columns.
     */
-   vars = pull_var_clause(qual, true);
+   vars = pull_var_clause(qual, PVC_INCLUDE_PLACEHOLDERS);
    foreach(vl, vars)
    {
        Var        *var = (Var *) lfirst(vl);

diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index bc4544e5e063bd93940f8d75311310332bf84d4f..17c9539679723cce12ad9a2aae875727eb8a471b 100644 (file)
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -10,7 +10,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.17 2009/02/06 23:43:23 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.18 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -689,7 +689,8 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
    foreach(lc, ec->ec_members)
    {
        EquivalenceMember *cur_em = (EquivalenceMember *) lfirst(lc);
-       List       *vars = pull_var_clause((Node *) cur_em->em_expr, true);
+       List       *vars = pull_var_clause((Node *) cur_em->em_expr,
+                                          PVC_INCLUDE_PLACEHOLDERS);
 
        add_vars_to_targetlist(root, vars, ec->ec_relids);
        list_free(vars);

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index cff0424c6c7826616917bdebff2c59cf9cd7fa85..1a957ac39698edfe794d143f564016f7c839cbe6 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.257 2009/03/26 17:15:35 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.258 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3022,7 +3022,8 @@ make_sort_from_pathkeys(PlannerInfo *root, Plan *lefttree, List *pathkeys,
                    if (em->em_is_const || em->em_is_child)
                        continue;
                    sortexpr = em->em_expr;
-                   exprvars = pull_var_clause((Node *) sortexpr, true);
+                   exprvars = pull_var_clause((Node *) sortexpr,
+                                              PVC_INCLUDE_PLACEHOLDERS);
                    foreach(k, exprvars)
                    {
                        if (!tlist_member_ignore_relabel(lfirst(k), tlist))

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 952fd7649fd708b1ae8b92ec078156027a383f5a..dd73beb759f1df23df8bc36384c637954b39ac35 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.150 2009/04/16 20:42:16 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.151 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -130,7 +130,8 @@ add_base_rels_to_query(PlannerInfo *root, Node *jtnode)
 void
 build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
 {
-   List       *tlist_vars = pull_var_clause((Node *) final_tlist, true);
+   List       *tlist_vars = pull_var_clause((Node *) final_tlist,
+                                            PVC_INCLUDE_PLACEHOLDERS);
 
    if (tlist_vars != NIL)
    {
@@ -981,7 +982,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
     */
    if (bms_membership(relids) == BMS_MULTIPLE)
    {
-       List       *vars = pull_var_clause(clause, true);
+       List       *vars = pull_var_clause(clause, PVC_INCLUDE_PLACEHOLDERS);
 
        add_vars_to_targetlist(root, vars, relids);
        list_free(vars);

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index d7a793ba769687ed44d230ab362dfc5599259ab3..bf1142aec09a42263d48d73ebfd890176b83781a 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.253 2009/03/30 17:30:44 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.254 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2236,7 +2236,7 @@ make_subplanTargetList(PlannerInfo *root,
     * and window specifications.
     */
    sub_tlist = flatten_tlist(tlist);
-   extravars = pull_var_clause(parse->havingQual, true);
+   extravars = pull_var_clause(parse->havingQual, PVC_INCLUDE_PLACEHOLDERS);
    sub_tlist = add_to_flat_tlist(sub_tlist, extravars);
    list_free(extravars);
    *need_tlist_eval = false;   /* only eval if not flat tlist */

diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index e6bd43f7b3a20a09540606c2bd57b1433784cc77..73a158e5d894fa8e0aa8b098fa7510458ae859cb 100644 (file)
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -16,7 +16,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/prep/preptlist.c,v 1.95 2009/01/01 17:23:44 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/prep/preptlist.c,v 1.96 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -193,7 +193,8 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
        List       *vars;
        ListCell   *l;
 
-       vars = pull_var_clause((Node *) parse->returningList, true);
+       vars = pull_var_clause((Node *) parse->returningList,
+                              PVC_INCLUDE_PLACEHOLDERS);
        foreach(l, vars)
        {
            Var        *var = (Var *) lfirst(l);

diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c
index c1834a12914906238f4f06fe9068d35c5e715e4f..019352158d2876e9a55af429938f873db49803b1 100644 (file)
--- a/src/backend/optimizer/util/placeholder.c
+++ b/src/backend/optimizer/util/placeholder.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/util/placeholder.c,v 1.3 2009/01/01 17:23:45 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/util/placeholder.c,v 1.4 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -162,7 +162,7 @@ fix_placeholder_eval_levels(PlannerInfo *root)
        if (bms_membership(eval_at) == BMS_MULTIPLE)
        {
            List       *vars = pull_var_clause((Node *) phinfo->ph_var->phexpr,
-                                              true);
+                                              PVC_INCLUDE_PLACEHOLDERS);
 
            add_vars_to_targetlist(root, vars, eval_at);
            list_free(vars);

diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c
index 5cf2839b37ea32509ea2a7efd5d0a89252447a25..9526aefd9ac9bf36a073c01cbccdd66dd7eb6583 100644 (file)
--- a/src/backend/optimizer/util/tlist.c
+++ b/src/backend/optimizer/util/tlist.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/util/tlist.c,v 1.85 2009/01/01 17:23:45 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/util/tlist.c,v 1.86 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -91,7 +91,8 @@ tlist_member_ignore_relabel(Node *node, List *targetlist)
 List *
 flatten_tlist(List *tlist)
 {
-   List       *vlist = pull_var_clause((Node *) tlist, true);
+   List       *vlist = pull_var_clause((Node *) tlist,
+                                       PVC_INCLUDE_PLACEHOLDERS);
    List       *new_tlist;
 
    new_tlist = add_to_flat_tlist(NIL, vlist);

diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index 7768712b569012db9c0163cc9dcfcef6ec2a9d14..cd88c337f1a3ccc91da7583752042a88cc34d09c 100644 (file)
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/util/var.c,v 1.84 2009/02/25 03:30:37 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/util/var.c,v 1.85 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -56,7 +56,7 @@ typedef struct
 typedef struct
 {
    List       *varlist;
-   bool        includePlaceHolderVars;
+   PVCPlaceHolderBehavior behavior;
 } pull_var_clause_context;
 
 typedef struct
@@ -614,11 +614,13 @@ find_minimum_var_level_walker(Node *node,
  * pull_var_clause
  *   Recursively pulls all Var nodes from an expression clause.
  *
- *   PlaceHolderVars are included too, if includePlaceHolderVars is true.
- *   If it isn't true, an error is thrown if any are found.
- *   Note that Vars within a PHV's expression are *not* included.
+ *   PlaceHolderVars are handled according to 'behavior':
+ *     PVC_REJECT_PLACEHOLDERS     throw error if PlaceHolderVar found
+ *     PVC_INCLUDE_PLACEHOLDERS    include PlaceHolderVars in output list
+ *     PVC_RECURSE_PLACEHOLDERS    recurse into PlaceHolderVar argument
+ *   Vars within a PHV's expression are included only in the last case.
  *
- *   CurrentOfExpr nodes are *not* included.
+ *   CurrentOfExpr nodes are ignored in all cases.
  *
  *   Upper-level vars (with varlevelsup > 0) are not included.
  *   (These probably represent errors too, but we don't complain.)
@@ -630,12 +632,12 @@ find_minimum_var_level_walker(Node *node,
  * of sublinks to subplans!
  */
 List *
-pull_var_clause(Node *node, bool includePlaceHolderVars)
+pull_var_clause(Node *node, PVCPlaceHolderBehavior behavior)
 {
    pull_var_clause_context context;
 
    context.varlist = NIL;
-   context.includePlaceHolderVars = includePlaceHolderVars;
+   context.behavior = behavior;
 
    pull_var_clause_walker(node, &context);
    return context.varlist;
@@ -654,12 +656,20 @@ pull_var_clause_walker(Node *node, pull_var_clause_context *context)
    }
    if (IsA(node, PlaceHolderVar))
    {
-       if (!context->includePlaceHolderVars)
-           elog(ERROR, "PlaceHolderVar found where not expected");
-       if (((PlaceHolderVar *) node)->phlevelsup == 0)
-           context->varlist = lappend(context->varlist, node);
-       /* we do NOT descend into the contained expression */
-       return false;
+       switch (context->behavior)
+       {
+           case PVC_REJECT_PLACEHOLDERS:
+               elog(ERROR, "PlaceHolderVar found where not expected");
+               break;
+           case PVC_INCLUDE_PLACEHOLDERS:
+               if (((PlaceHolderVar *) node)->phlevelsup == 0)
+                   context->varlist = lappend(context->varlist, node);
+               /* we do NOT descend into the contained expression */
+               return false;
+           case PVC_RECURSE_PLACEHOLDERS:
+               /* ignore the placeholder, look at its argument instead */
+               break;
+       }
    }
    return expression_tree_walker(node, pull_var_clause_walker,
                                  (void *) context);

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index e4b1acb1f8389d9567dace9fb510e1fc1a74d3d3..3f07db6857fff6eed71fc60099d082dba2f8e1e0 100644 (file)
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/adt/selfuncs.c,v 1.259 2009/02/15 20:16:21 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/adt/selfuncs.c,v 1.260 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2984,9 +2984,12 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows)
        ReleaseVariableStats(vardata);
 
        /*
-        * Else pull out the component Vars
+        * Else pull out the component Vars.  Handle PlaceHolderVars by
+        * recursing into their arguments (effectively assuming that the
+        * PlaceHolderVar doesn't change the number of groups, which boils
+        * down to ignoring the possible addition of nulls to the result set).
         */
-       varshere = pull_var_clause(groupexpr, true);
+       varshere = pull_var_clause(groupexpr, PVC_RECURSE_PLACEHOLDERS);
 
        /*
         * If we find any variable-free GROUP BY item, then either it is a

diff --git a/src/include/optimizer/var.h b/src/include/optimizer/var.h
index 83b181932e5a6e2ca5a1abf672c682d91a4b49b9..5d19572e0d97225fab442db4b90f1fae8e0a8073 100644 (file)
--- a/src/include/optimizer/var.h
+++ b/src/include/optimizer/var.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/optimizer/var.h,v 1.40 2009/01/01 17:24:00 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/var.h,v 1.41 2009/04/19 19:46:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -16,6 +16,12 @@
 
 #include "nodes/relation.h"
 
+typedef enum
+{
+   PVC_REJECT_PLACEHOLDERS,    /* throw error if PlaceHolderVar found */
+   PVC_INCLUDE_PLACEHOLDERS,   /* include PlaceHolderVars in output list */
+   PVC_RECURSE_PLACEHOLDERS    /* recurse into PlaceHolderVar argument */
+} PVCPlaceHolderBehavior;
 
 extern Relids pull_varnos(Node *node);
 extern void pull_varattnos(Node *node, Bitmapset **varattnos);
@@ -24,7 +30,7 @@ extern bool contain_vars_of_level(Node *node, int levelsup);
 extern int locate_var_of_level(Node *node, int levelsup);
 extern int locate_var_of_relation(Node *node, int relid, int levelsup);
 extern int find_minimum_var_level(Node *node);
-extern List *pull_var_clause(Node *node, bool includePlaceHolderVars);
+extern List *pull_var_clause(Node *node, PVCPlaceHolderBehavior behavior);
 extern Node *flatten_join_alias_vars(PlannerInfo *root, Node *node);
 
 #endif   /* VAR_H */