On second thought, expression_tree_walker should handle bare

SubLink nodes after all ...

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 3427302c8de56794afb3a743bcb299d33ff3f16c..ad320dcd0476b847517976f3e0e9229cbcb20b1b 100644 (file)
--- 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 f7374171b201419f0d8c14fec277e6be45d11c8a..80662278f7047c6fdeac751f8c90228579ac6173 100644 (file)
--- 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);
 }