Repair some problems in planner's handling of HAVING clauses.
authorTom Lane
Mon, 19 Apr 1999 01:43:12 +0000 (01:43 +0000)
committerTom Lane
Mon, 19 Apr 1999 01:43:12 +0000 (01:43 +0000)
This fixes a few of the problems Hiroshi Inoue complained of, but
I have not touched the rewrite-related issues.

src/backend/nodes/copyfuncs.c
src/backend/optimizer/plan/planner.c
src/backend/optimizer/plan/setrefs.c
src/include/optimizer/planmain.h

index 426a953891f2af42d667dd878230333bf5aaa656..90b84d591a96579a875e6e0352f2f3dfb5ab2e36 100644 (file)
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.76 1999/03/03 00:02:42 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.77 1999/04/19 01:43:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -470,7 +470,10 @@ _copyAgg(Agg *from)
 
    CopyPlanFields((Plan *) from, (Plan *) newnode);
 
-   newnode->aggs = get_agg_tlist_references(newnode);
+   /* Cannot copy agg list; it must be rebuilt to point to subnodes of
+    * new node.
+    */ 
+   set_agg_tlist_references(newnode);
 
    return newnode;
 }
index 27d5892ee0cca35ab2f920a8712015844688e850..543b5d6d7c6aaa8e971191e4704340b68f6f7e59 100644 (file)
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.46 1999/03/19 18:56:37 momjian Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.47 1999/04/19 01:43:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -138,32 +138,14 @@ union_planner(Query *parse)
    else
    {
      List  **vpm = NULL;
-     
-     /***S*H***/
-     /* This is only necessary if aggregates are in use in queries like:
-      * SELECT sid 
-      * FROM part
-      * GROUP BY sid
-      * HAVING MIN(pid) > 1;  (pid is used but never selected for!!!)
-      * because the function 'query_planner' creates the plan for the lefttree
-      * of the 'GROUP' node and returns only those attributes contained in 'tlist'.
-      * The original 'tlist' contains only 'sid' here and that's why we have to
-      * to extend it to attributes which are not selected but are used in the 
-      * havingQual. */
-         
-     /* 'check_having_qual_for_vars' takes the havingQual and the actual 'tlist'
-      * as arguments and recursively scans the havingQual for attributes 
-      * (VAR nodes) that are not contained in 'tlist' yet. If so, it creates
-      * a new entry and attaches it to the list 'new_tlist' (consisting of the 
-      * VAR node and the RESDOM node as usual with tlists :-)  ) */
-     if (parse->hasAggs)
-     {
-         if (parse->havingQual != NULL)
-         {
-           new_tlist = check_having_qual_for_vars(parse->havingQual,new_tlist);
-         }
-     }
-     
+
+     /*
+      * If there is a HAVING clause, make sure all vars referenced in it
+      * are included in the target list for query_planner().
+      */
+     if (parse->havingQual)
+         new_tlist = check_having_qual_for_vars(parse->havingQual, new_tlist);
+
      new_tlist = preprocess_targetlist(new_tlist,
                        parse->commandType,
                        parse->resultRelation,
@@ -208,10 +190,10 @@ union_planner(Query *parse)
                    parse->rtable);
      
      if (parse->rtable != NULL)
-       {
+     {
          vpm = (List **) palloc(length(parse->rtable) * sizeof(List *));
          memset(vpm, 0, length(parse->rtable) * sizeof(List *));
-       }
+     }
      PlannerVarParam = lcons(vpm, PlannerVarParam);
      result_plan = query_planner(parse,
                      parse->commandType,
@@ -246,89 +228,67 @@ union_planner(Query *parse)
                                   result_plan);
    }
 
+   /*
+    * If we have a HAVING clause, do the necessary things with it.
+    */
+   if (parse->havingQual)
+   {
+       List  **vpm = NULL;
+
+       if (parse->rtable != NULL)
+       {
+           vpm = (List **) palloc(length(parse->rtable) * sizeof(List *));
+           memset(vpm, 0, length(parse->rtable) * sizeof(List *));
+       }
+       PlannerVarParam = lcons(vpm, PlannerVarParam);
+
+       /* convert the havingQual to conjunctive normal form (cnf) */
+       parse->havingQual = (Node *) cnfify((Expr *) parse->havingQual, true);
+
+       if (parse->hasSubLinks)
+       {
+           /* There is a subselect in the havingQual, so we have to process it
+            * using the same function as for a subselect in 'where'
+            */
+           parse->havingQual =
+               (Node *) SS_process_sublinks(parse->havingQual);
+           /* Check for ungrouped variables passed to subplans.
+            * (Probably this should be done by the parser, but right now
+            * the parser is not smart enough to tell which level the vars
+            * belong to?)
+            */
+           check_having_for_ungrouped_vars(parse->havingQual,
+                                           parse->groupClause);
+       }
+
+       /* Calculate the opfids from the opnos */
+       parse->havingQual = (Node *) fix_opids((List *) parse->havingQual);
+
+       PlannerVarParam = lnext(PlannerVarParam);
+       if (vpm != NULL)
+           pfree(vpm);     
+   }
+
    /*
     * If aggregate is present, insert the agg node
     */
    if (parse->hasAggs)
    {
-           int old_length=0, new_length=0;
-       
-       /* Create the Agg node but use 'tlist' not 'new_tlist' as target list because we
-        * don't want the additional attributes (only used for the havingQual, see above)
-        * to show up in the result */
+       /* Use 'tlist' not 'new_tlist' as target list because we
+        * don't want the additional attributes used for the havingQual
+        * (see above) to show up in the result
+        */
        result_plan = (Plan *) make_agg(tlist, result_plan);
 
+       /* HAVING clause, if any, becomes qual of the Agg node */
+       result_plan->qual = (List *) parse->havingQual;
+
        /*
-        * get the varno/attno entries to the appropriate references to
-        * the result tuple of the subplans.
+        * Update vars to refer to subplan result tuples,
+        * find Aggrefs, make sure there is an Aggref in every HAVING clause.
         */
-       ((Agg *) result_plan)->aggs = get_agg_tlist_references((Agg *) result_plan); 
-
-       /***S*H***/
-       if(parse->havingQual!=NULL) 
-         {
-           List       *clause;
-           List      **vpm = NULL;
-           
-           
-           /* stuff copied from above to handle the use of attributes from outside
-            * in subselects */
-
-           if (parse->rtable != NULL)
-             {
-           vpm = (List **) palloc(length(parse->rtable) * sizeof(List *));
-           memset(vpm, 0, length(parse->rtable) * sizeof(List *));
-             }
-           PlannerVarParam = lcons(vpm, PlannerVarParam);
-           
-
-           /* convert the havingQual to conjunctive normal form (cnf) */
-           parse->havingQual = (Node *) cnfify((Expr *)(Node *) parse->havingQual,true);
-
-           /* There is a subselect in the havingQual, so we have to process it
-                     * using the same function as for a subselect in 'where' */
-           if (parse->hasSubLinks)
-             {
-           parse->havingQual = 
-             (Node *) SS_process_sublinks((Node *) parse->havingQual);
-             }
-                       
-           
-           /* Calculate the opfids from the opnos (=select the correct functions for
-            * the used VAR datatypes) */
-           parse->havingQual = (Node *) fix_opids((List *) parse->havingQual);
-           
-           ((Agg *) result_plan)->plan.qual=(List *) parse->havingQual;
-
-           /* Check every clause of the havingQual for aggregates used and append
-            * them to result_plan->aggs
-            */
-           foreach(clause, ((Agg *) result_plan)->plan.qual)
-             {
-           /* Make sure there are aggregates in the havingQual 
-            * if so, the list must be longer after check_having_qual_for_aggs
-            */
-           old_length=length(((Agg *) result_plan)->aggs);         
-           
-           ((Agg *) result_plan)->aggs = nconc(((Agg *) result_plan)->aggs,
-               check_having_qual_for_aggs((Node *) lfirst(clause),
-                      ((Agg *) result_plan)->plan.lefttree->targetlist,
-                      ((List *) parse->groupClause)));
-
-           /* Have a look at the length of the returned list. If there is no
-            * difference, no aggregates have been found and that means, that
-            * the Qual belongs to the where clause */
-           if (((new_length=length(((Agg *) result_plan)->aggs)) == old_length) ||
-               (new_length == 0))
-             {
-               elog(ERROR,"This could have been done in a where clause!!");
-               return (Plan *)NIL;
-             }
-             }
-           PlannerVarParam = lnext(PlannerVarParam);
-           if (vpm != NULL)
-             pfree(vpm);       
-         }
+       if (! set_agg_tlist_references((Agg *) result_plan))
+           elog(ERROR, "SELECT/HAVING requires aggregates to be valid");
    }         
 
    /*
index e3bfc87586a42dd6cbbc1c4d117c53a5d949582e..53ebd8c34ddb8dc3b4c652617fee4298d52437e8 100644 (file)
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.40 1999/02/15 01:06:58 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.41 1999/04/19 01:43:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -695,32 +695,48 @@ OperandIsInner(Node *opnd, int inner_relid)
 
 /*---------------------------------------------------------
  *
- * get_agg_tlist_references -
- *   generates the target list of an Agg node so that it points to
+ * set_agg_tlist_references -
+ *   This routine has several responsibilities:
+ * * Update the target list of an Agg node so that it points to
  *   the tuples returned by its left tree subplan.
+ * * If there is a qual list (from a HAVING clause), similarly update
+ *   vars in it to point to the subplan target list.
+ * * Generate the aggNode->aggs list of Aggref nodes contained in the Agg.
  *
- * We now also generate a linked list of Aggref pointers for Agg.
- *
+ * The return value is TRUE if all qual clauses include Aggrefs, or FALSE
+ * if any do not (caller may choose to raise an error condition).
  */
-List *
-get_agg_tlist_references(Agg *aggNode)
+bool
+set_agg_tlist_references(Agg *aggNode)
 {
-   List       *aggTargetList;
    List       *subplanTargetList;
    List       *tl;
-   List       *aggreg_list = NIL;
+   List       *ql;
+   bool        all_quals_ok;
 
-   aggTargetList = aggNode->plan.targetlist;
    subplanTargetList = aggNode->plan.lefttree->targetlist;
+   aggNode->aggs = NIL;
 
-   foreach(tl, aggTargetList)
+   foreach(tl, aggNode->plan.targetlist)
    {
        TargetEntry *tle = lfirst(tl);
 
-       aggreg_list = nconc(
-         replace_agg_clause(tle->expr, subplanTargetList), aggreg_list);
+       aggNode->aggs = nconc(replace_agg_clause(tle->expr, subplanTargetList),
+                             aggNode->aggs);
    }
-   return aggreg_list;
+
+   all_quals_ok = true;
+   foreach(ql, aggNode->plan.qual)
+   {
+       Node *qual = lfirst(ql);
+       List *qualaggs = replace_agg_clause(qual, subplanTargetList);
+       if (qualaggs == NIL)
+           all_quals_ok = false; /* this qual clause has no agg functions! */
+       else
+           aggNode->aggs = nconc(qualaggs, aggNode->aggs);
+   }
+
+   return all_quals_ok;
 }
 
 static List *
@@ -740,30 +756,49 @@ replace_agg_clause(Node *clause, List *subplanTargetList)
 
        /*
         * Change the varno & varattno fields of the var node.
+        * Note we assume match_varid() will succeed ...
         *
         */
        ((Var *) clause)->varattno = subplanVar->resdom->resno;
 
        return NIL;
    }
-   else if (is_funcclause(clause))
+   else if (is_subplan(clause))
    {
+       SubLink *sublink = ((SubPlan *) ((Expr *) clause)->oper)->sublink;
 
        /*
-        * This is a function. Recursively call this routine for its
-        * arguments...
+        * Only the lefthand side of the sublink should be checked for
+        * aggregates to be attached to the aggs list
+        */
+       foreach(t, sublink->lefthand)
+           agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
+                            agg_list);
+       /* The first argument of ...->oper has also to be checked */
+       foreach(t, sublink->oper)
+           agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
+                            agg_list);
+       return agg_list;
+   }
+   else if (IsA(clause, Expr))
+   {
+       /*
+        * Recursively scan the arguments of an expression.
+        * NOTE: this must come after is_subplan() case since
+        * subplan is a kind of Expr node.
         */
        foreach(t, ((Expr *) clause)->args)
        {
-           agg_list = nconc(agg_list,
-                      replace_agg_clause(lfirst(t), subplanTargetList));
+           agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
+                            agg_list);
        }
        return agg_list;
    }
    else if (IsA(clause, Aggref))
    {
        return lcons(clause,
-                    replace_agg_clause(((Aggref *) clause)->target, subplanTargetList));
+                    replace_agg_clause(((Aggref *) clause)->target,
+                                       subplanTargetList));
    }
    else if (IsA(clause, ArrayRef))
    {
@@ -774,53 +809,30 @@ replace_agg_clause(Node *clause, List *subplanTargetList)
         * expression and its index expression...
         */
        foreach(t, aref->refupperindexpr)
-       {
-           agg_list = nconc(agg_list,
-                      replace_agg_clause(lfirst(t), subplanTargetList));
-       }
+           agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
+                            agg_list);
        foreach(t, aref->reflowerindexpr)
-       {
-           agg_list = nconc(agg_list,
-                      replace_agg_clause(lfirst(t), subplanTargetList));
-       }
-       agg_list = nconc(agg_list,
-                  replace_agg_clause(aref->refexpr, subplanTargetList));
-       agg_list = nconc(agg_list,
-             replace_agg_clause(aref->refassgnexpr, subplanTargetList));
-
+           agg_list = nconc(replace_agg_clause(lfirst(t), subplanTargetList),
+                            agg_list);
+       agg_list = nconc(replace_agg_clause(aref->refexpr, subplanTargetList),
+                        agg_list);
+       agg_list = nconc(replace_agg_clause(aref->refassgnexpr,
+                                           subplanTargetList),
+                        agg_list);
        return agg_list;
    }
-   else if (is_opclause(clause))
-   {
-
-       /*
-        * This is an operator. Recursively call this routine for both its
-        * left and right operands
-        */
-       Node       *left = (Node *) get_leftop((Expr *) clause);
-       Node       *right = (Node *) get_rightop((Expr *) clause);
-
-       if (left != (Node *) NULL)
-           agg_list = nconc(agg_list,
-                            replace_agg_clause(left, subplanTargetList));
-       if (right != (Node *) NULL)
-           agg_list = nconc(agg_list,
-                          replace_agg_clause(right, subplanTargetList));
-
-       return agg_list;
-   }
-   else if (IsA(clause, Param) ||IsA(clause, Const))
+   else if (IsA(clause, Param) || IsA(clause, Const))
    {
        /* do nothing! */
        return NIL;
    }
    else
    {
-
        /*
         * Ooops! we can not handle that!
         */
-       elog(ERROR, "replace_agg_clause: Can not handle this tlist!\n");
+       elog(ERROR, "replace_agg_clause: Cannot handle node type %d",
+            nodeTag(clause));
        return NIL;
    }
 }
@@ -911,48 +923,42 @@ del_agg_clause(Node *clause)
    return NULL;
 }
 
-/***S*H***/
-/* check_having_qual_for_vars takes the the havingQual and the actual targetlist as arguments
- * and recursively scans the havingQual for attributes that are not included in the targetlist
- * yet. Attributes contained in the havingQual but not in the targetlist show up with queries
- * like:
- * SELECT sid
- * FROM part
- * GROUP BY sid
- * HAVING MIN(pid) > 1;  (pid is used but never selected for!!!).
- * To be able to handle queries like that correctly we have to extend the actual targetlist
- * (which will be the one used for the GROUP node later on) by these attributes. */
+/*
+ * check_having_qual_for_vars takes the havingQual and the actual targetlist
+ * as arguments and recursively scans the havingQual for attributes that are
+ * not included in the targetlist yet.  This will occur with queries like:
+ *
+ * SELECT sid FROM part GROUP BY sid HAVING MIN(pid) > 1;
+ *
+ * To be able to handle queries like that correctly we have to extend the
+ * actual targetlist (which will be the one used for the GROUP node later on)
+ * by these attributes.  The return value is the extended targetlist.
+ */
 List *
 check_having_qual_for_vars(Node *clause, List *targetlist_so_far)
 {
    List       *t;
 
-
    if (IsA(clause, Var))
    {
        RelOptInfo  tmp_rel;
 
-
-       tmp_rel.targetlist = targetlist_so_far;
-
        /*
         * Ha! A Var node!
         */
 
+       tmp_rel.targetlist = targetlist_so_far;
+
        /* Check if the VAR is already contained in the targetlist */
        if (tlist_member((Var *) clause, (List *) targetlist_so_far) == NULL)
            add_var_to_tlist(&tmp_rel, (Var *) clause);
 
        return tmp_rel.targetlist;
    }
-
-   else if (is_funcclause(clause) || not_clause(clause) ||
-            or_clause(clause) || and_clause(clause))
+   else if (IsA(clause, Expr) && ! is_subplan(clause))
    {
-
        /*
-        * This is a function. Recursively call this routine for its
-        * arguments...
+        * Recursively scan the arguments of an expression.
         */
        foreach(t, ((Expr *) clause)->args)
            targetlist_so_far = check_having_qual_for_vars(lfirst(t), targetlist_so_far);
@@ -980,125 +986,105 @@ check_having_qual_for_vars(Node *clause, List *targetlist_so_far)
 
        return targetlist_so_far;
    }
-   else if (is_opclause(clause))
-   {
-
-       /*
-        * This is an operator. Recursively call this routine for both its
-        * left and right operands
-        */
-       Node       *left = (Node *) get_leftop((Expr *) clause);
-       Node       *right = (Node *) get_rightop((Expr *) clause);
-
-       if (left != (Node *) NULL)
-           targetlist_so_far = check_having_qual_for_vars(left, targetlist_so_far);
-       if (right != (Node *) NULL)
-           targetlist_so_far = check_having_qual_for_vars(right, targetlist_so_far);
-
-       return targetlist_so_far;
-   }
-   else if (IsA(clause, Param) ||IsA(clause, Const))
+   else if (IsA(clause, Param) || IsA(clause, Const))
    {
        /* do nothing! */
        return targetlist_so_far;
    }
-
    /*
     * If we get to a sublink, then we only have to check the lefthand
-    * side of the expression to see if there are any additional VARs
+    * side of the expression to see if there are any additional VARs.
+    * QUESTION: can this code actually be hit?
     */
    else if (IsA(clause, SubLink))
    {
-       foreach(t, ((List *) ((SubLink *) clause)->lefthand))
+       foreach(t, ((SubLink *) clause)->lefthand)
            targetlist_so_far = check_having_qual_for_vars(lfirst(t), targetlist_so_far);
        return targetlist_so_far;
    }
    else
    {
-
        /*
         * Ooops! we can not handle that!
         */
-       elog(ERROR, "check_having_qual_for_vars: Can not handle this having_qual! %d\n",
+       elog(ERROR, "check_having_qual_for_vars: Cannot handle node type %d",
             nodeTag(clause));
        return NIL;
    }
 }
 
-/* check_having_qual_for_aggs takes the havingQual, the targetlist and the groupClause
- * as arguments and scans the havingQual recursively for aggregates. If an aggregate is
- * found it is attached to a list and returned by the function. (All the returned lists
- * are concenated to result_plan->aggs in planner.c:union_planner() */
-List *
-check_having_qual_for_aggs(Node *clause, List *subplanTargetList, List *groupClause)
-{
-   List       *t,
-              *l1;
-   List       *agg_list = NIL;
-
-   int         contained_in_group_clause = 0;
+/*
+ * check_having_for_ungrouped_vars takes the havingQual and the list of
+ * GROUP BY clauses and checks for subplans in the havingQual that are being
+ * passed ungrouped variables as parameters.  In other contexts, ungrouped
+ * vars in the havingQual will be detected by the parser (see parse_agg.c,
+ * exprIsAggOrGroupCol()).  But that routine currently does not check subplans,
+ * because the necessary info is not computed until the planner runs.
+ * This ought to be cleaned up someday.
+ *
+ * NOTE: the havingClause has been cnf-ified, so AND subclauses have been
+ * turned into a plain List.  Thus, this routine has to cope with List nodes
+ * where the routine above does not...
+ */
 
+void
+check_having_for_ungrouped_vars(Node *clause, List *groupClause)
+{
+   List       *t;
 
    if (IsA(clause, Var))
    {
-       TargetEntry *subplanVar;
-
-       /*
-        * Ha! A Var node!
-        */
-       subplanVar = match_varid((Var *) clause, subplanTargetList);
-
-       /*
-        * Change the varno & varattno fields of the var node to point to
-        * the resdom->resno fields of the subplan (lefttree)
+       /* Ignore vars elsewhere in the having clause, since the
+        * parser already checked 'em.
         */
-       ((Var *) clause)->varattno = subplanVar->resdom->resno;
-
-       return NIL;
-
    }
-   /***S*H***/
-   else if (is_funcclause(clause) || not_clause(clause) ||
-            or_clause(clause) || and_clause(clause))
+   else if (is_subplan(clause))
    {
-       int         new_length = 0,
-                   old_length = 0;
-
        /*
-        * This is a function. Recursively call this routine for its
-        * arguments... (i.e. for AND, OR, ... clauses!)
+        * The args list of the subplan node represents attributes from outside
+        * passed into the sublink.
         */
        foreach(t, ((Expr *) clause)->args)
        {
-           old_length = length((List *) agg_list);
-
-           agg_list = nconc(agg_list,
-                check_having_qual_for_aggs(lfirst(t), subplanTargetList,
-                                           groupClause));
-
-           /*
-            * The arguments of OR or AND clauses are comparisons or
-            * relations and because we are in the havingQual there must
-            * be at least one operand using an aggregate function. If so,
-            * we will find it and the length of the agg_list will be
-            * increased after the above call to
-            * check_having_qual_for_aggs. If there are no aggregates
-            * used, the query could have been formulated using the
-            * 'where' clause
-            */
-           if (((new_length = length((List *) agg_list)) == old_length) || (new_length == 0))
+           bool contained_in_group_clause = false;
+           List       *gl;
+
+           foreach(gl, groupClause)
            {
-               elog(ERROR, "This could have been done in a where clause!!");
-               return NIL;
+               if (var_equal(lfirst(t),
+                             get_expr(((GroupClause *) lfirst(gl))->entry)))
+               {
+                   contained_in_group_clause = true;
+                   break;
+               }
            }
+
+           if (!contained_in_group_clause)
+               elog(ERROR, "Sub-SELECT in HAVING clause must use only GROUPed attributes from outer SELECT");
        }
-       return agg_list;
+   }
+   else if (IsA(clause, Expr))
+   {
+       /*
+        * Recursively scan the arguments of an expression.
+        * NOTE: this must come after is_subplan() case since
+        * subplan is a kind of Expr node.
+        */
+       foreach(t, ((Expr *) clause)->args)
+           check_having_for_ungrouped_vars(lfirst(t), groupClause);
+   }
+   else if (IsA(clause, List))
+   {
+       /*
+        * Recursively scan AND subclauses (see NOTE above).
+        */
+       foreach(t, ((List *) clause))
+           check_having_for_ungrouped_vars(lfirst(t), groupClause);
    }
    else if (IsA(clause, Aggref))
    {
-       return lcons(clause,
-                    check_having_qual_for_aggs(((Aggref *) clause)->target, subplanTargetList,
-                                               groupClause));
+           check_having_for_ungrouped_vars(((Aggref *) clause)->target,
+                                           groupClause);
    }
    else if (IsA(clause, ArrayRef))
    {
@@ -1109,130 +1095,22 @@ check_having_qual_for_aggs(Node *clause, List *subplanTargetList, List *groupCla
         * expression and its index expression...
         */
        foreach(t, aref->refupperindexpr)
-       {
-           agg_list = nconc(agg_list,
-                check_having_qual_for_aggs(lfirst(t), subplanTargetList,
-                                           groupClause));
-       }
+           check_having_for_ungrouped_vars(lfirst(t), groupClause);
        foreach(t, aref->reflowerindexpr)
-       {
-           agg_list = nconc(agg_list,
-                check_having_qual_for_aggs(lfirst(t), subplanTargetList,
-                                           groupClause));
-       }
-       agg_list = nconc(agg_list,
-            check_having_qual_for_aggs(aref->refexpr, subplanTargetList,
-                                       groupClause));
-       agg_list = nconc(agg_list,
-       check_having_qual_for_aggs(aref->refassgnexpr, subplanTargetList,
-                                  groupClause));
-
-       return agg_list;
+           check_having_for_ungrouped_vars(lfirst(t), groupClause);
+       check_having_for_ungrouped_vars(aref->refexpr, groupClause);
+       check_having_for_ungrouped_vars(aref->refassgnexpr, groupClause);
    }
-   else if (is_opclause(clause))
-   {
-
-       /*
-        * This is an operator. Recursively call this routine for both its
-        * left and right operands
-        */
-       Node       *left = (Node *) get_leftop((Expr *) clause);
-       Node       *right = (Node *) get_rightop((Expr *) clause);
-
-       if (left != (Node *) NULL)
-           agg_list = nconc(agg_list,
-                     check_having_qual_for_aggs(left, subplanTargetList,
-                                                groupClause));
-       if (right != (Node *) NULL)
-           agg_list = nconc(agg_list,
-                    check_having_qual_for_aggs(right, subplanTargetList,
-                                               groupClause));
-
-       return agg_list;
-   }
-   else if (IsA(clause, Param) ||IsA(clause, Const))
+   else if (IsA(clause, Param) || IsA(clause, Const))
    {
        /* do nothing! */
-       return NIL;
-   }
-
-   /*
-    * This is for Sublinks which show up as EXPR nodes. All the other
-    * EXPR nodes (funcclauses, and_clauses, or_clauses) were caught above
-    */
-   else if (IsA(clause, Expr))
-   {
-
-       /*
-        * Only the lefthand side of the sublink has to be checked for
-        * aggregates to be attached to result_plan->aggs (see
-        * planner.c:union_planner() )
-        */
-       foreach(t, ((List *) ((SubLink *) ((SubPlan *)
-                          ((Expr *) clause)->oper)->sublink)->lefthand))
-       {
-           agg_list = nconc(agg_list,
-                     check_having_qual_for_aggs(lfirst(t),
-                                       subplanTargetList, groupClause));
-       }
-
-       /* The first argument of ...->oper has also to be checked */
-       {
-           List       *tmp_ptr;
-
-           foreach(tmp_ptr, ((SubLink *) ((SubPlan *)
-                               ((Expr *) clause)->oper)->sublink)->oper)
-           {
-               agg_list = nconc(agg_list,
-                        check_having_qual_for_aggs((Node *) lfirst(((Expr *)
-                                                lfirst(tmp_ptr))->args),
-                                       subplanTargetList, groupClause));
-           }
-       }
-
-       /*
-        * All arguments to the Sublink node are attributes from outside
-        * used within the sublink. Here we have to check that only
-        * attributes that is grouped for are used!
-        */
-       foreach(t, ((Expr *) clause)->args)
-       {
-           contained_in_group_clause = 0;
-
-           foreach(l1, groupClause)
-           {
-               if (tlist_member(lfirst(t), lcons(((GroupClause *) lfirst(l1))->entry, NIL)) !=
-                   NULL)
-                   contained_in_group_clause = 1;
-           }
-
-           /*
-            * If the use of the attribute is allowed (i.e. it is in the
-            * groupClause) we have to adjust the varnos and varattnos
-            */
-           if (contained_in_group_clause)
-           {
-               agg_list = nconc(agg_list,
-                                check_having_qual_for_aggs(lfirst(t),
-                                       subplanTargetList, groupClause));
-           }
-           else
-           {
-               elog(ERROR, "You must group by the attribute used from outside!");
-               return NIL;
-           }
-       }
-       return agg_list;
    }
    else
    {
        /*
         * Ooops! we can not handle that!
         */
-       elog(ERROR, "check_having_qual_for_aggs: Can not handle this having_qual! %d\n",
+       elog(ERROR, "check_having_for_ungrouped_vars: Cannot handle node type %d",
             nodeTag(clause));
-       return NIL;
    }
 }
-
- /***S*H***//* End */
index 42e22e150f30cb6dcb6b6289f7e62e8f183c6a24..44dad8cfd66daaa28167cc9026edc01a1542ab76 100644 (file)
@@ -6,7 +6,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: planmain.h,v 1.22 1999/02/14 04:56:58 momjian Exp $
+ * $Id: planmain.h,v 1.23 1999/04/19 01:43:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -54,12 +54,10 @@ extern List *join_references(List *clauses, List *outer_tlist,
                             List *inner_tlist);
 extern List *index_outerjoin_references(List *inner_indxqual,
                           List *outer_tlist, Index inner_relid);
-extern List *get_agg_tlist_references(Agg *aggNode);
-extern void set_agg_agglist_references(Agg *aggNode);
+extern bool set_agg_tlist_references(Agg *aggNode);
 extern void del_agg_tlist_references(List *tlist);
-extern List *check_having_qual_for_aggs(Node *clause,
-                          List *subplanTargetList, List *groupClause);
 extern List *check_having_qual_for_vars(Node *clause, List *targetlist_so_far);
+extern void check_having_for_ungrouped_vars(Node *clause, List *groupClause);
 extern void transformKeySetQuery(Query *origNode);
 
 #endif  /* PLANMAIN_H */