Skip to content

Commit b19adc1

Browse files
tglsfdctglsfdc
committed
Fix parse_agg.c to detect ungrouped Vars in sub-SELECTs; remove code
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.
1 parent a4d82dd commit b19adc1

File tree

6 files changed

+107
-229
lines changed

6 files changed

+107
-229
lines changed

‎src/backend/optimizer/plan/planner.c

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.139 2003/01/15 19:35:40 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.140 2003/01/17 03:25:03 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -208,17 +208,6 @@ subquery_planner(Query *parse, double tuple_fraction)
208208
/* These are not targetlist items, but close enough... */
209209
}
210210

211-
/*
212-
* Check for ungrouped variables passed to subplans in targetlist and
213-
* HAVING clause (but not in WHERE or JOIN/ON clauses, since those are
214-
* evaluated before grouping). We can't do this any earlier because
215-
* we must use the preprocessed targetlist for comparisons of grouped
216-
* expressions.
217-
*/
218-
if (parse->hasSubLinks &&
219-
(parse->groupClause != NIL || parse->hasAggs))
220-
check_subplans_for_ungrouped_vars(parse);
221-
222211
/*
223212
* A HAVING clause without aggregates is equivalent to a WHERE clause
224213
* (except it can only refer to grouped fields). Transfer any

‎src/backend/optimizer/util/clauses.c

Lines changed: 1 addition & 163 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.123 2003/01/17 02:01:16 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.124 2003/01/17 03:25:03 tgl Exp $
1212
*
1313
* HISTORY
1414
* AUTHOR DATE MAJOR EVENT
@@ -20,18 +20,15 @@
2020
#include "postgres.h"
2121

2222
#include "catalog/pg_language.h"
23-
#include "catalog/pg_operator.h"
2423
#include "catalog/pg_proc.h"
2524
#include "catalog/pg_type.h"
2625
#include "executor/executor.h"
2726
#include "miscadmin.h"
2827
#include "nodes/makefuncs.h"
2928
#include "nodes/nodeFuncs.h"
3029
#include "optimizer/clauses.h"
31-
#include "optimizer/tlist.h"
3230
#include "optimizer/var.h"
3331
#include "parser/analyze.h"
34-
#include "parser/parsetree.h"
3532
#include "tcop/tcopprot.h"
3633
#include "utils/acl.h"
3734
#include "utils/builtins.h"
@@ -45,12 +42,6 @@
4542
#define MAKEBOOLCONST(val,isnull) \
4643
((Node *) makeConst(BOOLOID, 1, (Datum) (val), (isnull), true))
4744

48-
typedef struct
49-
{
50-
Query *query;
51-
List *groupClauses;
52-
} check_subplans_for_ungrouped_vars_context;
53-
5445
typedef struct
5546
{
5647
int nargs;
@@ -64,8 +55,6 @@ static bool pull_agg_clause_walker(Node *node, List **listptr);
6455
static bool expression_returns_set_walker(Node *node, void *context);
6556
static bool contain_subplans_walker(Node *node, void *context);
6657
static bool pull_subplans_walker(Node *node, List **listptr);
67-
static bool check_subplans_for_ungrouped_vars_walker(Node *node,
68-
check_subplans_for_ungrouped_vars_context *context);
6958
static bool contain_mutable_functions_walker(Node *node, void *context);
7059
static bool contain_volatile_functions_walker(Node *node, void *context);
7160
static bool contain_nonstrict_functions_walker(Node *node, void *context);
@@ -552,157 +541,6 @@ pull_subplans_walker(Node *node, List **listptr)
552541
(void *) listptr);
553542
}
554543

555-
/*
556-
* check_subplans_for_ungrouped_vars
557-
* Check for subplans that are being passed ungrouped variables as
558-
* parameters; generate an error message if any are found.
559-
*
560-
* In most contexts, ungrouped variables will be detected by the parser (see
561-
* parse_agg.c, check_ungrouped_columns()). But that routine currently does
562-
* not check subplans, because the necessary info is not computed until the
563-
* planner runs. So we do it here, after we have processed sublinks into
564-
* subplans. This ought to be cleaned up someday.
565-
*
566-
* A deficiency in this scheme is that any outer reference var must be
567-
* grouped by itself; we don't recognize groupable expressions within
568-
* subselects. For example, consider
569-
* SELECT
570-
* (SELECT x FROM bar where y = (foo.a + foo.b))
571-
* FROM foo
572-
* GROUP BY a + b;
573-
* This query will be rejected although it could be allowed.
574-
*/
575-
void
576-
check_subplans_for_ungrouped_vars(Query *query)
577-
{
578-
check_subplans_for_ungrouped_vars_context context;
579-
List *gl;
580-
581-
context.query = query;
582-
583-
/*
584-
* Build a list of the acceptable GROUP BY expressions for use in the
585-
* walker (to avoid repeated scans of the targetlist within the
586-
* recursive routine).
587-
*/
588-
context.groupClauses = NIL;
589-
foreach(gl, query->groupClause)
590-
{
591-
GroupClause *grpcl = lfirst(gl);
592-
Node *expr;
593-
594-
expr = get_sortgroupclause_expr(grpcl, query->targetList);
595-
context.groupClauses = lcons(expr, context.groupClauses);
596-
}
597-
598-
/*
599-
* Recursively scan the targetlist and the HAVING clause. WHERE and
600-
* JOIN/ON conditions are not examined, since they are evaluated
601-
* before grouping.
602-
*/
603-
check_subplans_for_ungrouped_vars_walker((Node *) query->targetList,
604-
&context);
605-
check_subplans_for_ungrouped_vars_walker(query->havingQual,
606-
&context);
607-
608-
freeList(context.groupClauses);
609-
}
610-
611-
static bool
612-
check_subplans_for_ungrouped_vars_walker(Node *node,
613-
check_subplans_for_ungrouped_vars_context *context)
614-
{
615-
List *gl;
616-
617-
if (node == NULL)
618-
return false;
619-
if (IsA(node, Const) ||
620-
IsA(node, Param))
621-
return false; /* constants are always acceptable */
622-
623-
/*
624-
* If we find an aggregate function, do not recurse into its
625-
* arguments. Subplans invoked within aggregate calls are allowed to
626-
* receive ungrouped variables. (This test and the next one should
627-
* match the logic in parse_agg.c's check_ungrouped_columns().)
628-
*/
629-
if (IsA(node, Aggref))
630-
return false;
631-
632-
/*
633-
* Check to see if subexpression as a whole matches any GROUP BY item.
634-
* We need to do this at every recursion level so that we recognize
635-
* GROUPed-BY expressions before reaching variables within them.
636-
*/
637-
foreach(gl, context->groupClauses)
638-
{
639-
if (equal(node, lfirst(gl)))
640-
return false; /* acceptable, do not descend more */
641-
}
642-
643-
/*
644-
* We can ignore Vars other than in subplan args lists, since the
645-
* parser already checked 'em.
646-
*/
647-
if (is_subplan(node))
648-
{
649-
/*
650-
* The args list of the subplan node represents attributes from
651-
* outside passed into the sublink.
652-
*/
653-
List *t;
654-
655-
foreach(t, ((SubPlan *) node)->args)
656-
{
657-
Node *thisarg = lfirst(t);
658-
Var *var;
659-
bool contained_in_group_clause;
660-
661-
/*
662-
* We do not care about args that are not local variables;
663-
* params or outer-level vars are not our responsibility to
664-
* check. (The outer-level query passing them to us needs to
665-
* worry, instead.)
666-
*/
667-
if (!IsA(thisarg, Var))
668-
continue;
669-
var = (Var *) thisarg;
670-
if (var->varlevelsup > 0)
671-
continue;
672-
673-
/*
674-
* Else, see if it is a grouping column.
675-
*/
676-
contained_in_group_clause = false;
677-
foreach(gl, context->groupClauses)
678-
{
679-
if (equal(thisarg, lfirst(gl)))
680-
{
681-
contained_in_group_clause = true;
682-
break;
683-
}
684-
}
685-
686-
if (!contained_in_group_clause)
687-
{
688-
/* Found an ungrouped argument. Complain. */
689-
RangeTblEntry *rte;
690-
char *attname;
691-
692-
Assert(var->varno > 0 &&
693-
(int) var->varno <= length(context->query->rtable));
694-
rte = rt_fetch(var->varno, context->query->rtable);
695-
attname = get_rte_attribute_name(rte, var->varattno);
696-
elog(ERROR, "Sub-SELECT uses un-GROUPed attribute %s.%s from outer query",
697-
rte->eref->aliasname, attname);
698-
}
699-
}
700-
}
701-
return expression_tree_walker(node,
702-
check_subplans_for_ungrouped_vars_walker,
703-
(void *) context);
704-
}
705-
706544

707545
/*****************************************************************************
708546
* Check clauses for mutable functions

‎src/backend/parser/analyze.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.259 2003/01/02 19:29:22 tgl Exp $
9+
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.260 2003/01/17 03:25:04 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -354,7 +354,7 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
354354
qry->hasSubLinks = pstate->p_hasSubLinks;
355355
qry->hasAggs = pstate->p_hasAggs;
356356
if (pstate->p_hasAggs)
357-
parseCheckAggregates(pstate, qry, qual);
357+
parseCheckAggregates(pstate, qry);
358358

359359
return qry;
360360
}
@@ -575,7 +575,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt,
575575
qry->hasSubLinks = pstate->p_hasSubLinks;
576576
qry->hasAggs = pstate->p_hasAggs;
577577
if (pstate->p_hasAggs)
578-
parseCheckAggregates(pstate, qry, NULL);
578+
parseCheckAggregates(pstate, qry);
579579

580580
return qry;
581581
}
@@ -1671,13 +1671,13 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
16711671
qry->limitOffset = stmt->limitOffset;
16721672
qry->limitCount = stmt->limitCount;
16731673

1674+
qry->rtable = pstate->p_rtable;
1675+
qry->jointree = makeFromExpr(pstate->p_joinlist, qual);
1676+
16741677
qry->hasSubLinks = pstate->p_hasSubLinks;
16751678
qry->hasAggs = pstate->p_hasAggs;
16761679
if (pstate->p_hasAggs || qry->groupClause || qry->havingQual)
1677-
parseCheckAggregates(pstate, qry, qual);
1678-
1679-
qry->rtable = pstate->p_rtable;
1680-
qry->jointree = makeFromExpr(pstate->p_joinlist, qual);
1680+
parseCheckAggregates(pstate, qry);
16811681

16821682
if (stmt->forUpdate != NIL)
16831683
transformForUpdate(qry, stmt->forUpdate);
@@ -1900,13 +1900,13 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
19001900
qry->limitOffset = limitOffset;
19011901
qry->limitCount = limitCount;
19021902

1903+
qry->rtable = pstate->p_rtable;
1904+
qry->jointree = makeFromExpr(pstate->p_joinlist, NULL);
1905+
19031906
qry->hasSubLinks = pstate->p_hasSubLinks;
19041907
qry->hasAggs = pstate->p_hasAggs;
19051908
if (pstate->p_hasAggs || qry->groupClause || qry->havingQual)
1906-
parseCheckAggregates(pstate, qry, NULL);
1907-
1908-
qry->rtable = pstate->p_rtable;
1909-
qry->jointree = makeFromExpr(pstate->p_joinlist, NULL);
1909+
parseCheckAggregates(pstate, qry);
19101910

19111911
if (forUpdate != NIL)
19121912
transformForUpdate(qry, forUpdate);
@@ -2146,7 +2146,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
21462146
qry->hasSubLinks = pstate->p_hasSubLinks;
21472147
qry->hasAggs = pstate->p_hasAggs;
21482148
if (pstate->p_hasAggs)
2149-
parseCheckAggregates(pstate, qry, qual);
2149+
parseCheckAggregates(pstate, qry);
21502150

21512151
/*
21522152
* Now we are done with SELECT-like processing, and can get on with

0 commit comments

Comments
 (0)