From 4438b70b945f71ac35b5031d3f07e7e973449247 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 19 Apr 1999 01:43:12 +0000 Subject: [PATCH] Repair some problems in planner's handling of HAVING clauses. 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 | 7 +- src/backend/optimizer/plan/planner.c | 166 ++++------- src/backend/optimizer/plan/setrefs.c | 416 ++++++++++----------------- src/include/optimizer/planmain.h | 8 +- 4 files changed, 218 insertions(+), 379 deletions(-) diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 426a953891..90b84d591a 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -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; } diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 27d5892ee0..543b5d6d7c 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.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"); } /* diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index e3bfc87586..53ebd8c34d 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -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) +/* + * 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, - *l1; - List *agg_list = NIL; - - int contained_in_group_clause = 0; - + List *t; if (IsA(clause, Var)) { - TargetEntry *subplanVar; - - /* - * Ha! A Var node! + /* Ignore vars elsewhere in the having clause, since the + * parser already checked 'em. */ - 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) - */ - ((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); + bool contained_in_group_clause = false; + List *gl; - 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)) + 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 */ diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h index 42e22e150f..44dad8cfd6 100644 --- a/src/include/optimizer/planmain.h +++ b/src/include/optimizer/planmain.h @@ -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 */