Skip to content

Commit 37d20eb

Browse files
committed
Clean up some mistakes in handling of uplevel Vars in planner.
Most parts of the planner should ignore, or indeed never even see, uplevel Vars because they will be or have been replaced by Params. There were a couple of places that got it wrong though, probably my fault from recent changes...
1 parent 5adebf8 commit 37d20eb

File tree

6 files changed

+65
-34
lines changed

6 files changed

+65
-34
lines changed

src/backend/optimizer/plan/initsplan.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.38 1999/08/22 20:14:47 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.39 1999/08/26 05:07:41 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -50,7 +50,7 @@ static void check_hashjoinable(RestrictInfo *restrictinfo);
5050
void
5151
make_var_only_tlist(Query *root, List *tlist)
5252
{
53-
List *tlist_vars = pull_var_clause((Node *) tlist);
53+
List *tlist_vars = pull_var_clause((Node *) tlist, false);
5454

5555
add_vars_to_targetlist(root, tlist_vars);
5656
freeList(tlist_vars);

src/backend/optimizer/plan/planner.c

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.65 1999/08/22 23:56:45 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.66 1999/08/26 05:07:41 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -296,33 +296,37 @@ union_planner(Query *parse)
296296

297297
/*
298298
* If we have a HAVING clause, do the necessary things with it.
299+
* This code should parallel query_planner()'s initial processing
300+
* of the WHERE clause.
299301
*/
300302
if (parse->havingQual)
301303
{
302304
List *ql;
303305

304-
/* convert the havingQual to conjunctive normal form (cnf) */
305-
parse->havingQual = (Node *) cnfify((Expr *) parse->havingQual, true);
306+
/* Replace uplevel Vars with Params */
307+
if (PlannerQueryLevel > 1)
308+
parse->havingQual = SS_replace_correlation_vars(parse->havingQual);
306309

307310
if (parse->hasSubLinks)
308311
{
309-
/*
310-
* There may be a subselect in the havingQual, so we have to
311-
* process it using the same function as for a subselect in
312-
* 'where'
313-
*/
312+
/* Expand SubLinks to SubPlans */
314313
parse->havingQual = SS_process_sublinks(parse->havingQual);
315314

316315
/*
317316
* Check for ungrouped variables passed to subplans. (Probably
318-
* this should be done for the targetlist as well???)
317+
* this should be done for the targetlist as well??? But we
318+
* should NOT do it for the WHERE qual, since WHERE is
319+
* evaluated pre-GROUP.)
319320
*/
320321
if (check_subplans_for_ungrouped_vars(parse->havingQual,
321322
parse->groupClause,
322323
parse->targetList))
323324
elog(ERROR, "Sub-SELECT in HAVING clause must use only GROUPed attributes from outer SELECT");
324325
}
325326

327+
/* convert the havingQual to conjunctive normal form (cnf) */
328+
parse->havingQual = (Node *) cnfify((Expr *) parse->havingQual, true);
329+
326330
/*
327331
* Require an aggregate function to appear in each clause of the
328332
* havingQual (else it could have been done as a WHERE constraint).
@@ -428,10 +432,11 @@ make_subplanTargetList(Query *parse,
428432

429433
/*
430434
* Otherwise, start with a "flattened" tlist (having just the vars
431-
* mentioned in the targetlist and HAVING qual).
435+
* mentioned in the targetlist and HAVING qual --- but not upper-
436+
* level Vars; they will be replaced by Params later on).
432437
*/
433438
sub_tlist = flatten_tlist(tlist);
434-
extravars = pull_var_clause(parse->havingQual);
439+
extravars = pull_var_clause(parse->havingQual, false);
435440
sub_tlist = add_to_flat_tlist(sub_tlist, extravars);
436441
freeList(extravars);
437442

src/backend/optimizer/util/clauses.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.49 1999/08/25 23:21:41 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.50 1999/08/26 05:09:05 tgl Exp $
1111
*
1212
* HISTORY
1313
* AUTHOR DATE MAJOR EVENT
@@ -339,7 +339,7 @@ make_ands_implicit(Expr *clause)
339339
/*
340340
* pull_constant_clauses
341341
* Scans through a list of qualifications and find those that
342-
* contain no variables.
342+
* contain no variables (of the current query level).
343343
*
344344
* Returns a list of the constant clauses in constantQual and the remaining
345345
* quals as the return value.
@@ -480,11 +480,14 @@ check_subplans_for_ungrouped_vars_walker(Node *node,
480480
* Var nodes are considered distinct if they have different varno
481481
* or varattno values. If there are several occurrences of the same
482482
* varno/varattno, you get a randomly chosen one...
483+
*
484+
* Note that upper-level vars are ignored, since they normally will
485+
* become Params with respect to this query level.
483486
*/
484487
void
485488
clause_get_relids_vars(Node *clause, Relids *relids, List **vars)
486489
{
487-
List *clvars = pull_var_clause(clause);
490+
List *clvars = pull_var_clause(clause, false);
488491
List *varno_list = NIL;
489492
List *var_list = NIL;
490493
List *i;
@@ -494,7 +497,6 @@ clause_get_relids_vars(Node *clause, Relids *relids, List **vars)
494497
Var *var = (Var *) lfirst(i);
495498
List *vi;
496499

497-
Assert(var->varlevelsup == 0);
498500
if (!intMember(var->varno, varno_list))
499501
varno_list = lconsi(var->varno, varno_list);
500502
foreach(vi, var_list)

src/backend/optimizer/util/tlist.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/tlist.c,v 1.40 1999/08/22 20:14:54 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/tlist.c,v 1.41 1999/08/26 05:09:06 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -147,6 +147,10 @@ new_unsorted_tlist(List *targetlist)
147147
* flatten_tlist
148148
* Create a target list that only contains unique variables.
149149
*
150+
* Note that Vars with varlevelsup > 0 are not included in the output
151+
* tlist. We expect that those will eventually be replaced with Params,
152+
* but that probably has not happened at the time this routine is called.
153+
*
150154
* 'tlist' is the current target list
151155
*
152156
* Returns the "flattened" new target list.
@@ -157,7 +161,7 @@ new_unsorted_tlist(List *targetlist)
157161
List *
158162
flatten_tlist(List *tlist)
159163
{
160-
List *vlist = pull_var_clause((Node *) tlist);
164+
List *vlist = pull_var_clause((Node *) tlist, false);
161165
List *new_tlist;
162166

163167
new_tlist = add_to_flat_tlist(NIL, vlist);

src/backend/optimizer/util/var.c

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/var.c,v 1.23 1999/08/22 20:14:54 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/var.c,v 1.24 1999/08/26 05:09:06 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -19,16 +19,23 @@
1919
#include "optimizer/var.h"
2020

2121

22+
typedef struct {
23+
List *varlist;
24+
bool includeUpperVars;
25+
} pull_var_clause_context;
26+
2227
static bool pull_varnos_walker(Node *node, List **listptr);
2328
static bool contain_var_clause_walker(Node *node, void *context);
24-
static bool pull_var_clause_walker(Node *node, List **listptr);
29+
static bool pull_var_clause_walker(Node *node,
30+
pull_var_clause_context *context);
2531

2632

2733
/*
2834
* pull_varnos
2935
*
3036
* Create a list of all the distinct varnos present in a parsetree
31-
* (tlist or qual).
37+
* (tlist or qual). Note that only varnos attached to level-zero
38+
* Vars are considered --- upper Vars refer to some other rtable!
3239
*/
3340
List *
3441
pull_varnos(Node *node)
@@ -47,7 +54,7 @@ pull_varnos_walker(Node *node, List **listptr)
4754
if (IsA(node, Var))
4855
{
4956
Var *var = (Var *) node;
50-
if (!intMember(var->varno, *listptr))
57+
if (var->varlevelsup == 0 && !intMember(var->varno, *listptr))
5158
*listptr = lconsi(var->varno, *listptr);
5259
return false;
5360
}
@@ -56,7 +63,8 @@ pull_varnos_walker(Node *node, List **listptr)
5663

5764
/*
5865
* contain_var_clause
59-
* Recursively scan a clause to discover whether it contains any Var nodes.
66+
* Recursively scan a clause to discover whether it contains any Var nodes
67+
* (of the current query level).
6068
*
6169
* Returns true if any varnode found.
6270
*/
@@ -72,36 +80,48 @@ contain_var_clause_walker(Node *node, void *context)
7280
if (node == NULL)
7381
return false;
7482
if (IsA(node, Var))
75-
return true; /* abort the tree traversal and return true */
83+
{
84+
if (((Var *) node)->varlevelsup == 0)
85+
return true; /* abort the tree traversal and return true */
86+
return false;
87+
}
7688
return expression_tree_walker(node, contain_var_clause_walker, context);
7789
}
7890

7991
/*
8092
* pull_var_clause
8193
* Recursively pulls all var nodes from an expression clause.
8294
*
95+
* Upper-level vars (with varlevelsup > 0) are included only
96+
* if includeUpperVars is true. Most callers probably want
97+
* to ignore upper-level vars.
98+
*
8399
* Returns list of varnodes found. Note the varnodes themselves are not
84100
* copied, only referenced.
85101
*/
86102
List *
87-
pull_var_clause(Node *clause)
103+
pull_var_clause(Node *clause, bool includeUpperVars)
88104
{
89-
List *result = NIL;
105+
pull_var_clause_context context;
90106

91-
pull_var_clause_walker(clause, &result);
92-
return result;
107+
context.varlist = NIL;
108+
context.includeUpperVars = includeUpperVars;
109+
110+
pull_var_clause_walker(clause, &context);
111+
return context.varlist;
93112
}
94113

95114
static bool
96-
pull_var_clause_walker(Node *node, List **listptr)
115+
pull_var_clause_walker(Node *node, pull_var_clause_context *context)
97116
{
98117
if (node == NULL)
99118
return false;
100119
if (IsA(node, Var))
101120
{
102-
*listptr = lappend(*listptr, node);
121+
if (((Var *) node)->varlevelsup == 0 || context->includeUpperVars)
122+
context->varlist = lappend(context->varlist, node);
103123
return false;
104124
}
105125
return expression_tree_walker(node, pull_var_clause_walker,
106-
(void *) listptr);
126+
(void *) context);
107127
}

src/include/optimizer/var.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*
77
* Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Id: var.h,v 1.9 1999/08/22 20:14:57 tgl Exp $
9+
* $Id: var.h,v 1.10 1999/08/26 05:06:17 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -17,6 +17,6 @@
1717

1818
extern List *pull_varnos(Node *me);
1919
extern bool contain_var_clause(Node *clause);
20-
extern List *pull_var_clause(Node *clause);
20+
extern List *pull_var_clause(Node *clause, bool includeUpperVars);
2121

2222
#endif /* VAR_H */

0 commit comments

Comments
 (0)