Skip to content

Commit 1d97c19

Browse files
tglsfdctglsfdc
committed
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.
1 parent 3a624e9 commit 1d97c19

File tree

12 files changed

+63
-39
lines changed

12 files changed

+63
-39
lines changed

‎src/backend/catalog/heap.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.352 2009/03/31 17:59:56 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.353 2009/04/19 19:46:32 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -1621,7 +1621,7 @@ StoreRelCheck(Relation rel, char *ccname, Node *expr,
16211621
* in check constraints; it would fail to examine the contents of
16221622
* subselects.
16231623
*/
1624-
varList = pull_var_clause(expr, false);
1624+
varList = pull_var_clause(expr, PVC_REJECT_PLACEHOLDERS);
16251625
keycount = list_length(varList);
16261626

16271627
if (keycount > 0)
@@ -1915,7 +1915,7 @@ AddRelationNewConstraints(Relation rel,
19151915
List *vars;
19161916
char *colname;
19171917

1918-
vars = pull_var_clause(expr, false);
1918+
vars = pull_var_clause(expr, PVC_REJECT_PLACEHOLDERS);
19191919

19201920
/* eliminate duplicates */
19211921
vars = list_union(NIL, vars);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.181 2009/03/10 20:58:26 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.182 2009/04/19 19:46:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1122,7 +1122,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
11221122
* Examine all Vars used in clause; since it's a restriction clause, all
11231123
* such Vars must refer to subselect output columns.
11241124
*/
1125-
vars = pull_var_clause(qual, true);
1125+
vars = pull_var_clause(qual, PVC_INCLUDE_PLACEHOLDERS);
11261126
foreach(vl, vars)
11271127
{
11281128
Var *var = (Var *) lfirst(vl);

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Portions Copyright (c) 1994, Regents of the University of California
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.17 2009/02/06 23:43:23 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.18 2009/04/19 19:46:33 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -689,7 +689,8 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
689689
foreach(lc, ec->ec_members)
690690
{
691691
EquivalenceMember *cur_em = (EquivalenceMember *) lfirst(lc);
692-
List *vars = pull_var_clause((Node *) cur_em->em_expr, true);
692+
List *vars = pull_var_clause((Node *) cur_em->em_expr,
693+
PVC_INCLUDE_PLACEHOLDERS);
693694

694695
add_vars_to_targetlist(root, vars, ec->ec_relids);
695696
list_free(vars);

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.257 2009/03/26 17:15:35 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.258 2009/04/19 19:46:33 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -3022,7 +3022,8 @@ make_sort_from_pathkeys(PlannerInfo *root, Plan *lefttree, List *pathkeys,
30223022
if (em->em_is_const || em->em_is_child)
30233023
continue;
30243024
sortexpr = em->em_expr;
3025-
exprvars = pull_var_clause((Node *) sortexpr, true);
3025+
exprvars = pull_var_clause((Node *) sortexpr,
3026+
PVC_INCLUDE_PLACEHOLDERS);
30263027
foreach(k, exprvars)
30273028
{
30283029
if (!tlist_member_ignore_relabel(lfirst(k), tlist))

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.150 2009/04/16 20:42:16 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.151 2009/04/19 19:46:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -130,7 +130,8 @@ add_base_rels_to_query(PlannerInfo *root, Node *jtnode)
130130
void
131131
build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
132132
{
133-
List *tlist_vars = pull_var_clause((Node *) final_tlist, true);
133+
List *tlist_vars = pull_var_clause((Node *) final_tlist,
134+
PVC_INCLUDE_PLACEHOLDERS);
134135

135136
if (tlist_vars != NIL)
136137
{
@@ -981,7 +982,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
981982
*/
982983
if (bms_membership(relids) == BMS_MULTIPLE)
983984
{
984-
List *vars = pull_var_clause(clause, true);
985+
List *vars = pull_var_clause(clause, PVC_INCLUDE_PLACEHOLDERS);
985986

986987
add_vars_to_targetlist(root, vars, relids);
987988
list_free(vars);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.253 2009/03/30 17:30:44 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.254 2009/04/19 19:46:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -2236,7 +2236,7 @@ make_subplanTargetList(PlannerInfo *root,
22362236
* and window specifications.
22372237
*/
22382238
sub_tlist = flatten_tlist(tlist);
2239-
extravars = pull_var_clause(parse->havingQual, true);
2239+
extravars = pull_var_clause(parse->havingQual, PVC_INCLUDE_PLACEHOLDERS);
22402240
sub_tlist = add_to_flat_tlist(sub_tlist, extravars);
22412241
list_free(extravars);
22422242
*need_tlist_eval = false; /* only eval if not flat tlist */

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* Portions Copyright (c) 1994, Regents of the University of California
1717
*
1818
* IDENTIFICATION
19-
* $PostgreSQL: pgsql/src/backend/optimizer/prep/preptlist.c,v 1.95 2009/01/01 17:23:44 momjian Exp $
19+
* $PostgreSQL: pgsql/src/backend/optimizer/prep/preptlist.c,v 1.96 2009/04/19 19:46:33 tgl Exp $
2020
*
2121
*-------------------------------------------------------------------------
2222
*/
@@ -193,7 +193,8 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
193193
List *vars;
194194
ListCell *l;
195195

196-
vars = pull_var_clause((Node *) parse->returningList, true);
196+
vars = pull_var_clause((Node *) parse->returningList,
197+
PVC_INCLUDE_PLACEHOLDERS);
197198
foreach(l, vars)
198199
{
199200
Var *var = (Var *) lfirst(l);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/optimizer/util/placeholder.c,v 1.3 2009/01/01 17:23:45 momjian Exp $
12+
* $PostgreSQL: pgsql/src/backend/optimizer/util/placeholder.c,v 1.4 2009/04/19 19:46:33 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -162,7 +162,7 @@ fix_placeholder_eval_levels(PlannerInfo *root)
162162
if (bms_membership(eval_at) == BMS_MULTIPLE)
163163
{
164164
List *vars = pull_var_clause((Node *) phinfo->ph_var->phexpr,
165-
true);
165+
PVC_INCLUDE_PLACEHOLDERS);
166166

167167
add_vars_to_targetlist(root, vars, eval_at);
168168
list_free(vars);

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/util/tlist.c,v 1.85 2009/01/01 17:23:45 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/util/tlist.c,v 1.86 2009/04/19 19:46:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -91,7 +91,8 @@ tlist_member_ignore_relabel(Node *node, List *targetlist)
9191
List *
9292
flatten_tlist(List *tlist)
9393
{
94-
List *vlist = pull_var_clause((Node *) tlist, true);
94+
List *vlist = pull_var_clause((Node *) tlist,
95+
PVC_INCLUDE_PLACEHOLDERS);
9596
List *new_tlist;
9697

9798
new_tlist = add_to_flat_tlist(NIL, vlist);

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

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*
1515
*
1616
* IDENTIFICATION
17-
* $PostgreSQL: pgsql/src/backend/optimizer/util/var.c,v 1.84 2009/02/25 03:30:37 tgl Exp $
17+
* $PostgreSQL: pgsql/src/backend/optimizer/util/var.c,v 1.85 2009/04/19 19:46:33 tgl Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -56,7 +56,7 @@ typedef struct
5656
typedef struct
5757
{
5858
List *varlist;
59-
bool includePlaceHolderVars;
59+
PVCPlaceHolderBehavior behavior;
6060
} pull_var_clause_context;
6161

6262
typedef struct
@@ -614,11 +614,13 @@ find_minimum_var_level_walker(Node *node,
614614
* pull_var_clause
615615
* Recursively pulls all Var nodes from an expression clause.
616616
*
617-
* PlaceHolderVars are included too, if includePlaceHolderVars is true.
618-
* If it isn't true, an error is thrown if any are found.
619-
* Note that Vars within a PHV's expression are *not* included.
617+
* PlaceHolderVars are handled according to 'behavior':
618+
* PVC_REJECT_PLACEHOLDERS throw error if PlaceHolderVar found
619+
* PVC_INCLUDE_PLACEHOLDERS include PlaceHolderVars in output list
620+
* PVC_RECURSE_PLACEHOLDERS recurse into PlaceHolderVar argument
621+
* Vars within a PHV's expression are included only in the last case.
620622
*
621-
* CurrentOfExpr nodes are *not* included.
623+
* CurrentOfExpr nodes are ignored in all cases.
622624
*
623625
* Upper-level vars (with varlevelsup > 0) are not included.
624626
* (These probably represent errors too, but we don't complain.)
@@ -630,12 +632,12 @@ find_minimum_var_level_walker(Node *node,
630632
* of sublinks to subplans!
631633
*/
632634
List *
633-
pull_var_clause(Node *node, bool includePlaceHolderVars)
635+
pull_var_clause(Node *node, PVCPlaceHolderBehavior behavior)
634636
{
635637
pull_var_clause_context context;
636638

637639
context.varlist = NIL;
638-
context.includePlaceHolderVars = includePlaceHolderVars;
640+
context.behavior = behavior;
639641

640642
pull_var_clause_walker(node, &context);
641643
return context.varlist;
@@ -654,12 +656,20 @@ pull_var_clause_walker(Node *node, pull_var_clause_context *context)
654656
}
655657
if (IsA(node, PlaceHolderVar))
656658
{
657-
if (!context->includePlaceHolderVars)
658-
elog(ERROR, "PlaceHolderVar found where not expected");
659-
if (((PlaceHolderVar *) node)->phlevelsup == 0)
660-
context->varlist = lappend(context->varlist, node);
661-
/* we do NOT descend into the contained expression */
662-
return false;
659+
switch (context->behavior)
660+
{
661+
case PVC_REJECT_PLACEHOLDERS:
662+
elog(ERROR, "PlaceHolderVar found where not expected");
663+
break;
664+
case PVC_INCLUDE_PLACEHOLDERS:
665+
if (((PlaceHolderVar *) node)->phlevelsup == 0)
666+
context->varlist = lappend(context->varlist, node);
667+
/* we do NOT descend into the contained expression */
668+
return false;
669+
case PVC_RECURSE_PLACEHOLDERS:
670+
/* ignore the placeholder, look at its argument instead */
671+
break;
672+
}
663673
}
664674
return expression_tree_walker(node, pull_var_clause_walker,
665675
(void *) context);

0 commit comments

Comments
 (0)