From 974bdd94f99a715dacc0c1c00b4aff89342e79a7 Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Mon, 21 Jun 1999 01:18:02 +0000 Subject: [PATCH] On second thought, expression_tree_walker should handle bare SubLink nodes after all ... --- src/backend/optimizer/util/clauses.c | 20 +++++++++++----- src/backend/parser/parse_agg.c | 36 ++++++++-------------------- 2 files changed, 24 insertions(+), 32 deletions(-) diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 3427302c8de..ad320dcd047 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.36 1999/06/19 03:41:45 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.37 1999/06/21 01:18:02 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -810,7 +810,8 @@ CommuteClause(Node *clause) * The walker routine should return "false" to continue the tree walk, or * "true" to abort the walk and immediately return "true" to the top-level * caller. This can be used to short-circuit the traversal if the walker - * has found what it came for. + * has found what it came for. "false" is returned to the top-level caller + * iff no invocation of the walker returned "true". * * The node types handled by expression_tree_walker include all those * normally found in target lists and qualifier clauses during the planning @@ -827,10 +828,11 @@ CommuteClause(Node *clause) * appropriate behavior by recognizing subplan nodes and doing the right * thing. * - * Bare SubLink nodes (without a SUBPLAN_EXPR) will trigger an error unless - * detected and processed by the walker. We expect that walkers used before - * sublink processing is done will handle them properly. (XXX Maybe ignoring - * them would be better default behavior?) + * Bare SubLink nodes (without a SUBPLAN_EXPR) are handled by recursing into + * the "lefthand" argument list only. (A bare SubLink should be seen only if + * the tree has not yet been processed by subselect.c.) Again, this can be + * overridden by the walker, but it seems to be the most useful default + * behavior. *-------------------- */ @@ -923,6 +925,12 @@ expression_tree_walker(Node *node, bool (*walker) (), void *context) return true; } break; + case T_SubLink: + /* A "bare" SubLink (note we will not come here if we found + * a SUBPLAN_EXPR node above). Examine the lefthand side, + * but not the oper list nor the subquery. + */ + return walker(((SubLink *) node)->lefthand, context); case T_List: foreach(temp, (List *) node) { diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index f7374171b20..80662278f70 100644 --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/parser/parse_agg.c,v 1.22 1999/06/19 03:48:31 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/parser/parse_agg.c,v 1.23 1999/06/21 01:18:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -43,9 +43,7 @@ static bool exprIsAggOrGroupCol_walker(Node *node, List *groupClauses); * Returns true if any aggregate found. * * NOTE: we assume that the given clause has been transformed suitably for - * parser output. This means we can use the planner's expression_tree_walker, - * except that we have to process SubLink nodes specially, since they haven't - * been turned into SubPlan nodes yet. + * parser output. This means we can use the planner's expression_tree_walker. */ static bool contain_agg_clause(Node *clause) @@ -60,12 +58,6 @@ contain_agg_clause_walker(Node *node, void *context) return false; if (IsA(node, Aggref)) return true; /* abort the tree traversal and return true */ - if (IsA(node, SubLink)) - { - /* Examine the lefthand side, but not the oper list nor the subquery */ - SubLink *sublink = (SubLink *) node; - return contain_agg_clause_walker((Node *) sublink->lefthand, context); - } return expression_tree_walker(node, contain_agg_clause_walker, context); } @@ -75,16 +67,15 @@ contain_agg_clause_walker(Node *node, void *context) * other than within the arguments of aggregate functions. * * NOTE: we assume that the given clause has been transformed suitably for - * parser output. This means we can use the planner's expression_tree_walker, - * except that we have to process SubLink nodes specially, since they haven't - * been turned into SubPlan nodes yet. + * parser output. This means we can use the planner's expression_tree_walker. * - * NOTE: in the case of a SubLink, we do not descend into the subquery. This - * means we will fail to detect ungrouped columns that appear as outer-level - * variables within a subquery. That seems unreasonably hard to handle here. - * Instead, we expect the planner to check for ungrouped columns after it's - * found all the outer-level references inside the subquery and converted - * them into a list of parameters for the subquery. + * NOTE: in the case of a SubLink, expression_tree_walker does not descend + * into the subquery. This means we will fail to detect ungrouped columns + * that appear as outer-level variables within a subquery. That case seems + * unreasonably hard to handle here. Instead, we expect the planner to check + * for ungrouped columns after it's found all the outer-level references + * inside the subquery and converted them into a list of parameters for the + * subquery. */ static bool exprIsAggOrGroupCol(Node *expr, List *groupClauses) @@ -128,13 +119,6 @@ exprIsAggOrGroupCol_walker(Node *node, List *groupClauses) return false; /* outer-level Var is acceptable */ } /* Otherwise, recurse. */ - if (IsA(node, SubLink)) - { - /* Examine the lefthand side, but not the oper list nor the subquery */ - SubLink *sublink = (SubLink *) node; - return exprIsAggOrGroupCol_walker((Node *) sublink->lefthand, - groupClauses); - } return expression_tree_walker(node, exprIsAggOrGroupCol_walker, (void *) groupClauses); } -- GitLab