From 31a0f1d33bccd69c76980d9cc602ad75ae696161 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 28 Jan 2004 07:46:44 +0000 Subject: [PATCH] Fix oversight in check_ungrouped_columns optimization that avoids unnecessary checks for complex grouping expressions: we cannot check whether the expressions are simple Vars until after we apply flatten_join_alias_vars, because in the case of FULL JOIN that routine can introduce non-Var expressions. Per example from Joel Knight. --- src/backend/parser/parse_agg.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index ab365ac84d..6f85595610 100644 --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_agg.c,v 1.60 2003/11/29 19:51:51 pgsql Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_agg.c,v 1.61 2004/01/28 07:46:44 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -98,7 +98,7 @@ void parseCheckAggregates(ParseState *pstate, Query *qry) { List *groupClauses = NIL; - bool have_non_var_grouping = false; + bool have_non_var_grouping; List *lst; bool hasJoinRTEs; Node *clause; @@ -127,9 +127,7 @@ parseCheckAggregates(ParseState *pstate, Query *qry) * No aggregates allowed in GROUP BY clauses, either. * * 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...). - * And detect whether any of the expressions aren't simple Vars. + * expressions for use by check_ungrouped_columns(). */ foreach(lst, qry->groupClause) { @@ -144,8 +142,6 @@ parseCheckAggregates(ParseState *pstate, Query *qry) (errcode(ERRCODE_GROUPING_ERROR), errmsg("aggregates not allowed in GROUP BY clause"))); groupClauses = lcons(expr, groupClauses); - if (!IsA(expr, Var)) - have_non_var_grouping = true; } /* @@ -170,6 +166,21 @@ parseCheckAggregates(ParseState *pstate, Query *qry) groupClauses = (List *) flatten_join_alias_vars(qry, (Node *) groupClauses); + /* + * Detect whether any of the grouping expressions aren't simple Vars; + * if they're all Vars then we don't have to work so hard in the + * recursive scans. (Note we have to flatten aliases before this.) + */ + have_non_var_grouping = false; + foreach(lst, groupClauses) + { + if (!IsA((Node *) lfirst(lst), Var)) + { + have_non_var_grouping = true; + break; + } + } + /* * Check the targetlist and HAVING clause for ungrouped variables. */ -- 2.40.0