open-dsi
/
postgres
mirror of https://github.com/postgres/postgres
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix parse_agg.c to detect ungrouped Vars in sub-SELECTs; remove code

Browse Source 
that used to do it in planner.  That was an ancient kluge that was
never satisfactory; errors should be detected at parse time when possible.
But at the time we didn't have the support mechanism (expression_tree_walker
et al) to make it convenient to do in the parser.
WIN32_DEV
Tom Lane 23 years ago
parent a4d82dd4b4
commit b19adc1aae
6 changed files with 107 additions and 229 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 13
      src/backend/optimizer/plan/planner.c
  2. 164
      src/backend/optimizer/util/clauses.c
  3. 24
      src/backend/parser/analyze.c
  4. 125
      src/backend/parser/parse_agg.c
  5. 3
      src/include/optimizer/clauses.h
  6. 7
      src/include/parser/parse_agg.h

13
src/backend/optimizer/plan/planner.c
Unescape View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.139 2003/01/15 19:35:40 tgl Exp $
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.140 2003/01/17 03:25:03 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -208,17 +208,6 @@ subquery_planner(Query *parse, double tuple_fraction)
/* These are not targetlist items, but close enough... */
}
/*
* Check for ungrouped variables passed to subplans in targetlist and
* HAVING clause (but not in WHERE or JOIN/ON clauses, since those are
* evaluated before grouping). We can't do this any earlier because
* we must use the preprocessed targetlist for comparisons of grouped
* expressions.
*/
if (parse->hasSubLinks &&
(parse->groupClause != NIL || parse->hasAggs))
check_subplans_for_ungrouped_vars(parse);
/*
* A HAVING clause without aggregates is equivalent to a WHERE clause
* (except it can only refer to grouped fields). Transfer any

164
src/backend/optimizer/util/clauses.c
Unescape View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.123 2003/01/17 02:01:16 tgl Exp $
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.124 2003/01/17 03:25:03 tgl Exp $
*
* HISTORY
* AUTHOR DATE MAJOR EVENT
@ -20,7 +20,6 @@
#include "postgres.h"
#include "catalog/pg_language.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
#include "executor/executor.h"
@ -28,10 +27,8 @@
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "optimizer/clauses.h"
#include "optimizer/tlist.h"
#include "optimizer/var.h"
#include "parser/analyze.h"
#include "parser/parsetree.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
#include "utils/builtins.h"
@ -45,12 +42,6 @@
#define MAKEBOOLCONST(val,isnull) \
((Node *) makeConst(BOOLOID, 1, (Datum) (val), (isnull), true))
typedef struct
{
Query *query;
List *groupClauses;
} check_subplans_for_ungrouped_vars_context;
typedef struct
{
int nargs;
@ -64,8 +55,6 @@ static bool pull_agg_clause_walker(Node *node, List **listptr);
static bool expression_returns_set_walker(Node *node, void *context);
static bool contain_subplans_walker(Node *node, void *context);
static bool pull_subplans_walker(Node *node, List **listptr);
static bool check_subplans_for_ungrouped_vars_walker(Node *node,
check_subplans_for_ungrouped_vars_context *context);
static bool contain_mutable_functions_walker(Node *node, void *context);
static bool contain_volatile_functions_walker(Node *node, void *context);
static bool contain_nonstrict_functions_walker(Node *node, void *context);
@ -552,157 +541,6 @@ pull_subplans_walker(Node *node, List **listptr)
(void *) listptr);
}
/*
* check_subplans_for_ungrouped_vars
* Check for subplans that are being passed ungrouped variables as
* parameters; generate an error message if any are found.
*
* In most contexts, ungrouped variables will be detected by the parser (see
* parse_agg.c, check_ungrouped_columns()). But that routine currently does
* not check subplans, because the necessary info is not computed until the
* planner runs. So we do it here, after we have processed sublinks into
* subplans. This ought to be cleaned up someday.
*
* A deficiency in this scheme is that any outer reference var must be
* grouped by itself; we don't recognize groupable expressions within
* subselects. For example, consider
* SELECT
* (SELECT x FROM bar where y = (foo.a + foo.b))
* FROM foo
* GROUP BY a + b;
* This query will be rejected although it could be allowed.
*/
void
check_subplans_for_ungrouped_vars(Query *query)
{
check_subplans_for_ungrouped_vars_context context;
List *gl;
context.query = query;
/*
* Build a list of the acceptable GROUP BY expressions for use in the
* walker (to avoid repeated scans of the targetlist within the
* recursive routine).
*/
context.groupClauses = NIL;
foreach(gl, query->groupClause)
{
GroupClause *grpcl = lfirst(gl);
Node *expr;
expr = get_sortgroupclause_expr(grpcl, query->targetList);
context.groupClauses = lcons(expr, context.groupClauses);
}
/*
* Recursively scan the targetlist and the HAVING clause. WHERE and
* JOIN/ON conditions are not examined, since they are evaluated
* before grouping.
*/
check_subplans_for_ungrouped_vars_walker((Node *) query->targetList,
&context);
check_subplans_for_ungrouped_vars_walker(query->havingQual,
&context);
freeList(context.groupClauses);
}
static bool
check_subplans_for_ungrouped_vars_walker(Node *node,
check_subplans_for_ungrouped_vars_context *context)
{
List *gl;
if (node == NULL)
return false;
if (IsA(node, Const) ||
IsA(node, Param))
return false; /* constants are always acceptable */
/*
* If we find an aggregate function, do not recurse into its
* arguments. Subplans invoked within aggregate calls are allowed to
* receive ungrouped variables. (This test and the next one should
* match the logic in parse_agg.c's check_ungrouped_columns().)
*/
if (IsA(node, Aggref))
return false;
/*
* Check to see if subexpression as a whole matches any GROUP BY item.
* We need to do this at every recursion level so that we recognize
* GROUPed-BY expressions before reaching variables within them.
*/
foreach(gl, context->groupClauses)
{
if (equal(node, lfirst(gl)))
return false; /* acceptable, do not descend more */
}
/*
* We can ignore Vars other than in subplan args lists, since the
* parser already checked 'em.
*/
if (is_subplan(node))
{
/*
* The args list of the subplan node represents attributes from
* outside passed into the sublink.
*/
List *t;
foreach(t, ((SubPlan *) node)->args)
{
Node *thisarg = lfirst(t);
Var *var;
bool contained_in_group_clause;
/*
* We do not care about args that are not local variables;
* params or outer-level vars are not our responsibility to
* check. (The outer-level query passing them to us needs to
* worry, instead.)
*/
if (!IsA(thisarg, Var))
continue;
var = (Var *) thisarg;
if (var->varlevelsup > 0)
continue;
/*
* Else, see if it is a grouping column.
*/
contained_in_group_clause = false;
foreach(gl, context->groupClauses)
{
if (equal(thisarg, lfirst(gl)))
{
contained_in_group_clause = true;
break;
}
}
if (!contained_in_group_clause)
{
/* Found an ungrouped argument. Complain. */
RangeTblEntry *rte;
char *attname;
Assert(var->varno > 0 &&
(int) var->varno <= length(context->query->rtable));
rte = rt_fetch(var->varno, context->query->rtable);
attname = get_rte_attribute_name(rte, var->varattno);
elog(ERROR, "Sub-SELECT uses un-GROUPed attribute %s.%s from outer query",
rte->eref->aliasname, attname);
}
}
}
return expression_tree_walker(node,
check_subplans_for_ungrouped_vars_walker,
(void *) context);
}
/*****************************************************************************
* Check clauses for mutable functions

24
src/backend/parser/analyze.c
Unescape View File

@ -6,7 +6,7 @@
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.259 2003/01/02 19:29:22 tgl Exp $
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.260 2003/01/17 03:25:04 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -354,7 +354,7 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
qry->hasSubLinks = pstate->p_hasSubLinks;
qry->hasAggs = pstate->p_hasAggs;
if (pstate->p_hasAggs)
parseCheckAggregates(pstate, qry, qual);
parseCheckAggregates(pstate, qry);
return qry;
}
@ -575,7 +575,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt,
qry->hasSubLinks = pstate->p_hasSubLinks;
qry->hasAggs = pstate->p_hasAggs;
if (pstate->p_hasAggs)
parseCheckAggregates(pstate, qry, NULL);
parseCheckAggregates(pstate, qry);
return qry;
}
@ -1671,13 +1671,13 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
qry->limitOffset = stmt->limitOffset;
qry->limitCount = stmt->limitCount;
qry->rtable = pstate->p_rtable;
qry->jointree = makeFromExpr(pstate->p_joinlist, qual);
qry->hasSubLinks = pstate->p_hasSubLinks;
qry->hasAggs = pstate->p_hasAggs;
if (pstate->p_hasAggs || qry->groupClause || qry->havingQual)
parseCheckAggregates(pstate, qry, qual);
qry->rtable = pstate->p_rtable;
qry->jointree = makeFromExpr(pstate->p_joinlist, qual);
parseCheckAggregates(pstate, qry);
if (stmt->forUpdate != NIL)
transformForUpdate(qry, stmt->forUpdate);
@ -1900,13 +1900,13 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
qry->limitOffset = limitOffset;
qry->limitCount = limitCount;
qry->rtable = pstate->p_rtable;
qry->jointree = makeFromExpr(pstate->p_joinlist, NULL);
qry->hasSubLinks = pstate->p_hasSubLinks;
qry->hasAggs = pstate->p_hasAggs;
if (pstate->p_hasAggs || qry->groupClause || qry->havingQual)
parseCheckAggregates(pstate, qry, NULL);
qry->rtable = pstate->p_rtable;
qry->jointree = makeFromExpr(pstate->p_joinlist, NULL);
parseCheckAggregates(pstate, qry);
if (forUpdate != NIL)
transformForUpdate(qry, forUpdate);
@ -2146,7 +2146,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
qry->hasSubLinks = pstate->p_hasSubLinks;
qry->hasAggs = pstate->p_hasAggs;
if (pstate->p_hasAggs)
parseCheckAggregates(pstate, qry, qual);
parseCheckAggregates(pstate, qry);
/*
* Now we are done with SELECT-like processing, and can get on with

125
src/backend/parser/parse_agg.c
Unescape View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/parser/parse_agg.c,v 1.50 2002/06/20 20:29:32 momjian Exp $
* $Header: /cvsroot/pgsql/src/backend/parser/parse_agg.c,v 1.51 2003/01/17 03:25:04 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -24,10 +24,12 @@ typedef struct
{
ParseState *pstate;
List *groupClauses;
bool have_non_var_grouping;
int sublevels_up;
} check_ungrouped_columns_context;
static void check_ungrouped_columns(Node *node, ParseState *pstate,
List *groupClauses);
List *groupClauses, bool have_non_var_grouping);
static bool check_ungrouped_columns_walker(Node *node,
check_ungrouped_columns_context *context);
@ -39,24 +41,29 @@ static bool check_ungrouped_columns_walker(Node *node,
* if any are 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.
* parser output. This means we can use expression_tree_walker.
*
* 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.
* NOTE: we recognize grouping expressions in the main query, but only
* grouping Vars in subqueries. For example, this will be rejected,
* although it could be allowed:
* SELECT
* (SELECT x FROM bar where y = (foo.a + foo.b))
* FROM foo
* GROUP BY a + b;
* The difficulty is the need to account for different sublevels_up.
* This appears to require a whole custom version of equal(), which is
* way more pain than the feature seems worth.
*/
static void
check_ungrouped_columns(Node *node, ParseState *pstate,
List *groupClauses)
List *groupClauses, bool have_non_var_grouping)
{
check_ungrouped_columns_context context;
context.pstate = pstate;
context.groupClauses = groupClauses;
context.have_non_var_grouping = have_non_var_grouping;
context.sublevels_up = 0;
check_ungrouped_columns_walker(node, &context);
}
@ -68,32 +75,38 @@ check_ungrouped_columns_walker(Node *node,
if (node == NULL)
return false;
if (IsA(node, Const) ||IsA(node, Param))
if (IsA(node, Const) ||
IsA(node, Param))
return false; /* constants are always acceptable */
/*
* If we find an aggregate function, do not recurse into its
* arguments.
* arguments; ungrouped vars in the arguments are not an error.
*/
if (IsA(node, Aggref))
return false;
/*
* Check to see if subexpression as a whole matches any GROUP BY item.
* If we have any GROUP BY items that are not simple Vars,
* check to see if subexpression as a whole matches any GROUP BY item.
* We need to do this at every recursion level so that we recognize
* GROUPed-BY expressions before reaching variables within them.
* But this only works at the outer query level, as noted above.
*/
foreach(gl, context->groupClauses)
if (context->have_non_var_grouping && context->sublevels_up == 0)
{
if (equal(node, lfirst(gl)))
return false; /* acceptable, do not descend more */
foreach(gl, context->groupClauses)
{
if (equal(node, lfirst(gl)))
return false; /* acceptable, do not descend more */
}
}
/*
* If we have an ungrouped Var, we have a failure --- unless it is an
* outer-level Var. In that case it's a constant as far as this query
* level is concerned, and we can accept it. (If it's ungrouped as
* far as the upper query is concerned, that's someone else's
* If we have an ungrouped Var of the original query level, we have a
* failure. Vars below the original query level are not a problem,
* and neither are Vars from above it. (If such Vars are ungrouped as
* far as their own query level is concerned, that's someone else's
* problem...)
*/
if (IsA(node, Var))
@ -102,17 +115,52 @@ check_ungrouped_columns_walker(Node *node,
RangeTblEntry *rte;
char *attname;
if (var->varlevelsup > 0)
return false; /* outer-level Var is acceptable */
if (var->varlevelsup != context->sublevels_up)
return false; /* it's not local to my query, ignore */
/*
* Check for a match, if we didn't do it above.
*/
if (!context->have_non_var_grouping || context->sublevels_up != 0)
{
foreach(gl, context->groupClauses)
{
Var *gvar = (Var *) lfirst(gl);
if (IsA(gvar, Var) &&
gvar->varno == var->varno &&
gvar->varattno == var->varattno &&
gvar->varlevelsup == 0)
return false; /* acceptable, we're okay */
}
}
/* Found an ungrouped local variable; generate error message */
Assert(var->varno > 0 &&
(int) var->varno <= length(context->pstate->p_rtable));
rte = rt_fetch(var->varno, context->pstate->p_rtable);
attname = get_rte_attribute_name(rte, var->varattno);
elog(ERROR, "Attribute %s.%s must be GROUPed or used in an aggregate function",
rte->eref->aliasname, attname);
if (context->sublevels_up == 0)
elog(ERROR, "Attribute %s.%s must be GROUPed or used in an aggregate function",
rte->eref->aliasname, attname);
else
elog(ERROR, "Sub-SELECT uses un-GROUPed attribute %s.%s from outer query",
rte->eref->aliasname, attname);
}
if (IsA(node, Query))
{
/* Recurse into subselects */
bool result;
context->sublevels_up++;
result = query_tree_walker((Query *) node,
check_ungrouped_columns_walker,
(void *) context,
0);
context->sublevels_up--;
return result;
}
/* Otherwise, recurse. */
return expression_tree_walker(node, check_ungrouped_columns_walker,
(void *) context);
}
@ -124,15 +172,13 @@ check_ungrouped_columns_walker(Node *node,
* Ideally this should be done earlier, but it's difficult to distinguish
* aggregates from plain functions at the grammar level. So instead we
* check here. This function should be called after the target list and
* qualifications are finalized. BUT: in some cases we want to call this
* routine before we've assembled the joinlist and qual into a FromExpr.
* So, rather than looking at qry->jointree, look at pstate->p_joinlist
* and the explicitly-passed qual.
* qualifications are finalized.
*/
void
parseCheckAggregates(ParseState *pstate, Query *qry, Node *qual)
parseCheckAggregates(ParseState *pstate, Query *qry)
{
List *groupClauses = NIL;
bool have_non_var_grouping = false;
List *tl;
/* This should only be called if we found aggregates, GROUP, or HAVING */
@ -146,9 +192,9 @@ parseCheckAggregates(ParseState *pstate, Query *qry, Node *qual)
* variable in the target list, which is outright misleading if the
* problem is in WHERE.)
*/
if (contain_agg_clause(qual))
if (contain_agg_clause(qry->jointree->quals))
elog(ERROR, "Aggregates not allowed in WHERE clause");
if (contain_agg_clause((Node *) pstate->p_joinlist))
if (contain_agg_clause((Node *) qry->jointree->fromlist))
elog(ERROR, "Aggregates not allowed in JOIN conditions");
/*
@ -156,7 +202,8 @@ parseCheckAggregates(ParseState *pstate, Query *qry, Node *qual)
*
* While we are at it, build a list of the acceptable GROUP BY
* expressions for use by check_ungrouped_columns() (this avoids
* repeated scans of the targetlist within the recursive routine...)
* repeated scans of the targetlist within the recursive routine...).
* And detect whether any of the expressions aren't simple Vars.
*/
foreach(tl, qry->groupClause)
{
@ -164,16 +211,22 @@ parseCheckAggregates(ParseState *pstate, Query *qry, Node *qual)
Node *expr;
expr = get_sortgroupclause_expr(grpcl, qry->targetList);
if (expr == NULL)
continue; /* probably cannot happen */
if (contain_agg_clause(expr))
elog(ERROR, "Aggregates not allowed in GROUP BY clause");
groupClauses = lcons(expr, groupClauses);
if (!IsA(expr, Var))
have_non_var_grouping = true;
}
/*
* Check the targetlist and HAVING clause for ungrouped variables.
*/
check_ungrouped_columns((Node *) qry->targetList, pstate, groupClauses);
check_ungrouped_columns((Node *) qry->havingQual, pstate, groupClauses);
check_ungrouped_columns((Node *) qry->targetList, pstate,
groupClauses, have_non_var_grouping);
check_ungrouped_columns((Node *) qry->havingQual, pstate,
groupClauses, have_non_var_grouping);
/* Release the list storage (but not the pointed-to expressions!) */
freeList(groupClauses);

3
src/include/optimizer/clauses.h
Unescape View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $Id: clauses.h,v 1.60 2003/01/17 02:01:21 tgl Exp $
* $Id: clauses.h,v 1.61 2003/01/17 03:25:04 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -52,7 +52,6 @@ extern bool expression_returns_set(Node *clause);
extern bool contain_subplans(Node *clause);
extern List *pull_subplans(Node *clause);
extern void check_subplans_for_ungrouped_vars(Query *query);
extern bool contain_mutable_functions(Node *clause);
extern bool contain_volatile_functions(Node *clause);

7
src/include/parser/parse_agg.h
Unescape View File

@ -1,13 +1,12 @@
/*-------------------------------------------------------------------------
*
* parse_agg.h
*
*
* handle aggregates in parser
*
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $Id: parse_agg.h,v 1.24 2002/06/20 20:29:51 momjian Exp $
* $Id: parse_agg.h,v 1.25 2003/01/17 03:25:04 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -16,6 +15,6 @@
#include "parser/parse_node.h"
extern void parseCheckAggregates(ParseState *pstate, Query *qry, Node *qual);
extern void parseCheckAggregates(ParseState *pstate, Query *qry);
#endif /* PARSE_AGG_H */

Write Preview
Loading…
Cancel
Save