From 37d20eb8558607e18816849a0a3f3d941317dc13 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 26 Aug 1999 05:09:06 +0000 Subject: [PATCH] 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... --- src/backend/optimizer/plan/initsplan.c | 4 +-- src/backend/optimizer/plan/planner.c | 27 +++++++++------ src/backend/optimizer/util/clauses.c | 10 +++--- src/backend/optimizer/util/tlist.c | 8 +++-- src/backend/optimizer/util/var.c | 46 ++++++++++++++++++-------- src/include/optimizer/var.h | 4 +-- 6 files changed, 65 insertions(+), 34 deletions(-) diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index a89c40b943..20a01902ef 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.38 1999/08/22 20:14:47 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.39 1999/08/26 05:07:41 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -50,7 +50,7 @@ static void check_hashjoinable(RestrictInfo *restrictinfo); void make_var_only_tlist(Query *root, List *tlist) { - List *tlist_vars = pull_var_clause((Node *) tlist); + List *tlist_vars = pull_var_clause((Node *) tlist, false); add_vars_to_targetlist(root, tlist_vars); freeList(tlist_vars); diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 3c7a665563..b5c8d7d3de 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.65 1999/08/22 23:56:45 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.66 1999/08/26 05:07:41 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -296,26 +296,27 @@ union_planner(Query *parse) /* * If we have a HAVING clause, do the necessary things with it. + * This code should parallel query_planner()'s initial processing + * of the WHERE clause. */ if (parse->havingQual) { List *ql; - /* convert the havingQual to conjunctive normal form (cnf) */ - parse->havingQual = (Node *) cnfify((Expr *) parse->havingQual, true); + /* Replace uplevel Vars with Params */ + if (PlannerQueryLevel > 1) + parse->havingQual = SS_replace_correlation_vars(parse->havingQual); if (parse->hasSubLinks) { - /* - * There may be a subselect in the havingQual, so we have to - * process it using the same function as for a subselect in - * 'where' - */ + /* Expand SubLinks to SubPlans */ parse->havingQual = SS_process_sublinks(parse->havingQual); /* * Check for ungrouped variables passed to subplans. (Probably - * this should be done for the targetlist as well???) + * this should be done for the targetlist as well??? But we + * should NOT do it for the WHERE qual, since WHERE is + * evaluated pre-GROUP.) */ if (check_subplans_for_ungrouped_vars(parse->havingQual, parse->groupClause, @@ -323,6 +324,9 @@ union_planner(Query *parse) elog(ERROR, "Sub-SELECT in HAVING clause must use only GROUPed attributes from outer SELECT"); } + /* convert the havingQual to conjunctive normal form (cnf) */ + parse->havingQual = (Node *) cnfify((Expr *) parse->havingQual, true); + /* * Require an aggregate function to appear in each clause of the * havingQual (else it could have been done as a WHERE constraint). @@ -428,10 +432,11 @@ make_subplanTargetList(Query *parse, /* * Otherwise, start with a "flattened" tlist (having just the vars - * mentioned in the targetlist and HAVING qual). + * mentioned in the targetlist and HAVING qual --- but not upper- + * level Vars; they will be replaced by Params later on). */ sub_tlist = flatten_tlist(tlist); - extravars = pull_var_clause(parse->havingQual); + extravars = pull_var_clause(parse->havingQual, false); sub_tlist = add_to_flat_tlist(sub_tlist, extravars); freeList(extravars); diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 2d960b5cf0..1ab09217c5 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.49 1999/08/25 23:21:41 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.50 1999/08/26 05:09:05 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -339,7 +339,7 @@ make_ands_implicit(Expr *clause) /* * pull_constant_clauses * Scans through a list of qualifications and find those that - * contain no variables. + * contain no variables (of the current query level). * * Returns a list of the constant clauses in constantQual and the remaining * quals as the return value. @@ -480,11 +480,14 @@ check_subplans_for_ungrouped_vars_walker(Node *node, * Var nodes are considered distinct if they have different varno * or varattno values. If there are several occurrences of the same * varno/varattno, you get a randomly chosen one... + * + * Note that upper-level vars are ignored, since they normally will + * become Params with respect to this query level. */ void clause_get_relids_vars(Node *clause, Relids *relids, List **vars) { - List *clvars = pull_var_clause(clause); + List *clvars = pull_var_clause(clause, false); List *varno_list = NIL; List *var_list = NIL; List *i; @@ -494,7 +497,6 @@ clause_get_relids_vars(Node *clause, Relids *relids, List **vars) Var *var = (Var *) lfirst(i); List *vi; - Assert(var->varlevelsup == 0); if (!intMember(var->varno, varno_list)) varno_list = lconsi(var->varno, varno_list); foreach(vi, var_list) diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c index dfe2963581..43007f33ff 100644 --- a/src/backend/optimizer/util/tlist.c +++ b/src/backend/optimizer/util/tlist.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/util/tlist.c,v 1.40 1999/08/22 20:14:54 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/tlist.c,v 1.41 1999/08/26 05:09:06 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -147,6 +147,10 @@ new_unsorted_tlist(List *targetlist) * flatten_tlist * Create a target list that only contains unique variables. * + * Note that Vars with varlevelsup > 0 are not included in the output + * tlist. We expect that those will eventually be replaced with Params, + * but that probably has not happened at the time this routine is called. + * * 'tlist' is the current target list * * Returns the "flattened" new target list. @@ -157,7 +161,7 @@ new_unsorted_tlist(List *targetlist) List * flatten_tlist(List *tlist) { - List *vlist = pull_var_clause((Node *) tlist); + List *vlist = pull_var_clause((Node *) tlist, false); 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 a544041122..af58b2556d 100644 --- a/src/backend/optimizer/util/var.c +++ b/src/backend/optimizer/util/var.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/util/var.c,v 1.23 1999/08/22 20:14:54 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/var.c,v 1.24 1999/08/26 05:09:06 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -19,16 +19,23 @@ #include "optimizer/var.h" +typedef struct { + List *varlist; + bool includeUpperVars; +} pull_var_clause_context; + static bool pull_varnos_walker(Node *node, List **listptr); static bool contain_var_clause_walker(Node *node, void *context); -static bool pull_var_clause_walker(Node *node, List **listptr); +static bool pull_var_clause_walker(Node *node, + pull_var_clause_context *context); /* * pull_varnos * * Create a list of all the distinct varnos present in a parsetree - * (tlist or qual). + * (tlist or qual). Note that only varnos attached to level-zero + * Vars are considered --- upper Vars refer to some other rtable! */ List * pull_varnos(Node *node) @@ -47,7 +54,7 @@ pull_varnos_walker(Node *node, List **listptr) if (IsA(node, Var)) { Var *var = (Var *) node; - if (!intMember(var->varno, *listptr)) + if (var->varlevelsup == 0 && !intMember(var->varno, *listptr)) *listptr = lconsi(var->varno, *listptr); return false; } @@ -56,7 +63,8 @@ pull_varnos_walker(Node *node, List **listptr) /* * contain_var_clause - * Recursively scan a clause to discover whether it contains any Var nodes. + * Recursively scan a clause to discover whether it contains any Var nodes + * (of the current query level). * * Returns true if any varnode found. */ @@ -72,7 +80,11 @@ contain_var_clause_walker(Node *node, void *context) if (node == NULL) return false; if (IsA(node, Var)) - return true; /* abort the tree traversal and return true */ + { + if (((Var *) node)->varlevelsup == 0) + return true; /* abort the tree traversal and return true */ + return false; + } return expression_tree_walker(node, contain_var_clause_walker, context); } @@ -80,28 +92,36 @@ contain_var_clause_walker(Node *node, void *context) * pull_var_clause * Recursively pulls all var nodes from an expression clause. * + * Upper-level vars (with varlevelsup > 0) are included only + * if includeUpperVars is true. Most callers probably want + * to ignore upper-level vars. + * * Returns list of varnodes found. Note the varnodes themselves are not * copied, only referenced. */ List * -pull_var_clause(Node *clause) +pull_var_clause(Node *clause, bool includeUpperVars) { - List *result = NIL; + pull_var_clause_context context; - pull_var_clause_walker(clause, &result); - return result; + context.varlist = NIL; + context.includeUpperVars = includeUpperVars; + + pull_var_clause_walker(clause, &context); + return context.varlist; } static bool -pull_var_clause_walker(Node *node, List **listptr) +pull_var_clause_walker(Node *node, pull_var_clause_context *context) { if (node == NULL) return false; if (IsA(node, Var)) { - *listptr = lappend(*listptr, node); + if (((Var *) node)->varlevelsup == 0 || context->includeUpperVars) + context->varlist = lappend(context->varlist, node); return false; } return expression_tree_walker(node, pull_var_clause_walker, - (void *) listptr); + (void *) context); } diff --git a/src/include/optimizer/var.h b/src/include/optimizer/var.h index 440b62f49a..e2d5390876 100644 --- a/src/include/optimizer/var.h +++ b/src/include/optimizer/var.h @@ -6,7 +6,7 @@ * * Copyright (c) 1994, Regents of the University of California * - * $Id: var.h,v 1.9 1999/08/22 20:14:57 tgl Exp $ + * $Id: var.h,v 1.10 1999/08/26 05:06:17 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -17,6 +17,6 @@ extern List *pull_varnos(Node *me); extern bool contain_var_clause(Node *clause); -extern List *pull_var_clause(Node *clause); +extern List *pull_var_clause(Node *clause, bool includeUpperVars); #endif /* VAR_H */