Centralize the logic for detecting misplaced aggregates, window funcs, etc.

Formerly we relied on checking after-the-fact to see if an expression
contained aggregates, window functions, or sub-selects when it shouldn't.
This is grotty, easily forgotten (indeed, we had forgotten to teach
DefineIndex about rejecting window functions), and none too efficient
since it requires extra traversals of the parse tree.  To improve matters,
define an enum type that classifies all SQL sub-expressions, store it in
ParseState to show what kind of expression we are currently parsing, and
make transformAggregateCall, transformWindowFuncCall, and transformSubLink
check the expression type and throw error if the type indicates the
construct is disallowed.  This allows removal of a large number of ad-hoc
checks scattered around the code base.  The enum type is sufficiently
fine-grained that we can still produce error messages of at least the
same specificity as before.

Bringing these error checks together revealed that we'd been none too
consistent about phrasing of the error messages, so standardize the wording
a bit.

Also, rewrite checking of aggregate arguments so that it requires only one
traversal of the arguments, rather than up to three as before.

In passing, clean up some more comments left over from add_missing_from
support, and annotate some tests that I think are dead code now that that's
gone.  (I didn't risk actually removing said dead code, though.)

1 files changed, 384 insertions, 156 deletions

diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 5854f81005d..d1d835b8007 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -20,6 +20,7 @@
 #include "optimizer/tlist.h"
 #include "parser/parse_agg.h"
 #include "parser/parse_clause.h"
+#include "parser/parse_expr.h"
 #include "parser/parsetree.h"
 #include "rewrite/rewriteManip.h"
 #include "utils/builtins.h"
@@ -28,6 +29,14 @@
 typedef struct
 {
 	ParseState *pstate;
+	int			min_varlevel;
+	int			min_agglevel;
+	int			sublevels_up;
+} check_agg_arguments_context;
+
+typedef struct
+{
+	ParseState *pstate;
 	Query	   *qry;
 	List	   *groupClauses;
 	bool		have_non_var_grouping;
@@ -35,6 +44,9 @@ typedef struct
 	int			sublevels_up;
 } check_ungrouped_columns_context;
 
+static int	check_agg_arguments(ParseState *pstate, List *args);
+static bool check_agg_arguments_walker(Node *node,
+						   check_agg_arguments_context *context);
 static void check_ungrouped_columns(Node *node, ParseState *pstate, Query *qry,
 						List *groupClauses, bool have_non_var_grouping,
 						List **func_grouped_rels);
@@ -72,6 +84,8 @@ transformAggregateCall(ParseState *pstate, Aggref *agg,
 	int			save_next_resno;
 	int			min_varlevel;
 	ListCell   *lc;
+	const char *err;
+	bool		errkind;
 
 	/*
 	 * Transform the plain list of Exprs into a targetlist.  We don't bother
@@ -102,6 +116,7 @@ transformAggregateCall(ParseState *pstate, Aggref *agg,
 	torder = transformSortClause(pstate,
 								 aggorder,
 								 &tlist,
+								 EXPR_KIND_ORDER_BY,
 								 true /* fix unknowns */ ,
 								 true /* force SQL99 rules */ );
 
@@ -142,55 +157,261 @@ transformAggregateCall(ParseState *pstate, Aggref *agg,
 	pstate->p_next_resno = save_next_resno;
 
 	/*
-	 * The aggregate's level is the same as the level of the lowest-level
-	 * variable or aggregate in its arguments; or if it contains no variables
-	 * at all, we presume it to be local.
+	 * Check the arguments to compute the aggregate's level and detect
+	 * improper nesting.
 	 */
-	min_varlevel = find_minimum_var_level((Node *) agg->args);
+	min_varlevel = check_agg_arguments(pstate, agg->args);
+	agg->agglevelsup = min_varlevel;
+
+	/* Mark the correct pstate level as having aggregates */
+	while (min_varlevel-- > 0)
+		pstate = pstate->parentParseState;
+	pstate->p_hasAggs = true;
 
 	/*
-	 * An aggregate can't directly contain another aggregate call of the same
-	 * level (though outer aggs are okay).	We can skip this check if we
-	 * didn't find any local vars or aggs.
+	 * Check to see if the aggregate function is in an invalid place within
+	 * its aggregation query.
+	 *
+	 * For brevity we support two schemes for reporting an error here: set
+	 * "err" to a custom message, or set "errkind" true if the error context
+	 * is sufficiently identified by what ParseExprKindName will return, *and*
+	 * what it will return is just a SQL keyword.  (Otherwise, use a custom
+	 * message to avoid creating translation problems.)
 	 */
-	if (min_varlevel == 0)
+	err = NULL;
+	errkind = false;
+	switch (pstate->p_expr_kind)
 	{
-		if (pstate->p_hasAggs &&
-			checkExprHasAggs((Node *) agg->args))
-			ereport(ERROR,
-					(errcode(ERRCODE_GROUPING_ERROR),
-					 errmsg("aggregate function calls cannot be nested"),
-					 parser_errposition(pstate,
-							   locate_agg_of_level((Node *) agg->args, 0))));
-	}
+		case EXPR_KIND_NONE:
+			Assert(false);		/* can't happen */
+			break;
+		case EXPR_KIND_OTHER:
+			/* Accept aggregate here; caller must throw error if wanted */
+			break;
+		case EXPR_KIND_JOIN_ON:
+		case EXPR_KIND_JOIN_USING:
+			err = _("aggregate functions are not allowed in JOIN conditions");
+			break;
+		case EXPR_KIND_FROM_SUBSELECT:
+			/* Should only be possible in a LATERAL subquery */
+			Assert(pstate->p_lateral_active);
+			/* Aggregate scope rules make it worth being explicit here */
+			err = _("aggregate functions are not allowed in FROM clause of their own query level");
+			break;
+		case EXPR_KIND_FROM_FUNCTION:
+			err = _("aggregate functions are not allowed in functions in FROM");
+			break;
+		case EXPR_KIND_WHERE:
+			errkind = true;
+			break;
+		case EXPR_KIND_HAVING:
+			/* okay */
+			break;
+		case EXPR_KIND_WINDOW_PARTITION:
+			/* okay */
+			break;
+		case EXPR_KIND_WINDOW_ORDER:
+			/* okay */
+			break;
+		case EXPR_KIND_WINDOW_FRAME_RANGE:
+			err = _("aggregate functions are not allowed in window RANGE");
+			break;
+		case EXPR_KIND_WINDOW_FRAME_ROWS:
+			err = _("aggregate functions are not allowed in window ROWS");
+			break;
+		case EXPR_KIND_SELECT_TARGET:
+			/* okay */
+			break;
+		case EXPR_KIND_INSERT_TARGET:
+		case EXPR_KIND_UPDATE_SOURCE:
+		case EXPR_KIND_UPDATE_TARGET:
+			errkind = true;
+			break;
+		case EXPR_KIND_GROUP_BY:
+			errkind = true;
+			break;
+		case EXPR_KIND_ORDER_BY:
+			/* okay */
+			break;
+		case EXPR_KIND_DISTINCT_ON:
+			/* okay */
+			break;
+		case EXPR_KIND_LIMIT:
+		case EXPR_KIND_OFFSET:
+			errkind = true;
+			break;
+		case EXPR_KIND_RETURNING:
+			errkind = true;
+			break;
+		case EXPR_KIND_VALUES:
+			errkind = true;
+			break;
+		case EXPR_KIND_CHECK_CONSTRAINT:
+		case EXPR_KIND_DOMAIN_CHECK:
+			err = _("aggregate functions are not allowed in CHECK constraints");
+			break;
+		case EXPR_KIND_COLUMN_DEFAULT:
+		case EXPR_KIND_FUNCTION_DEFAULT:
+			err = _("aggregate functions are not allowed in DEFAULT expressions");
+			break;
+		case EXPR_KIND_INDEX_EXPRESSION:
+			err = _("aggregate functions are not allowed in index expressions");
+			break;
+		case EXPR_KIND_INDEX_PREDICATE:
+			err = _("aggregate functions are not allowed in index predicates");
+			break;
+		case EXPR_KIND_ALTER_COL_TRANSFORM:
+			err = _("aggregate functions are not allowed in transform expressions");
+			break;
+		case EXPR_KIND_EXECUTE_PARAMETER:
+			err = _("aggregate functions are not allowed in EXECUTE parameters");
+			break;
+		case EXPR_KIND_TRIGGER_WHEN:
+			err = _("aggregate functions are not allowed in trigger WHEN conditions");
+			break;
 
-	/* It can't contain window functions either */
-	if (pstate->p_hasWindowFuncs &&
-		checkExprHasWindowFuncs((Node *) agg->args))
+			/*
+			 * There is intentionally no default: case here, so that the
+			 * compiler will warn if we add a new ParseExprKind without
+			 * extending this switch.  If we do see an unrecognized value at
+			 * runtime, the behavior will be the same as for EXPR_KIND_OTHER,
+			 * which is sane anyway.
+			 */
+	}
+	if (err)
 		ereport(ERROR,
 				(errcode(ERRCODE_GROUPING_ERROR),
-				 errmsg("aggregate function calls cannot contain window function calls"),
-				 parser_errposition(pstate,
-									locate_windowfunc((Node *) agg->args))));
+				 errmsg_internal("%s", err),
+				 parser_errposition(pstate, agg->location)));
+	if (errkind)
+		ereport(ERROR,
+				(errcode(ERRCODE_GROUPING_ERROR),
+				 /* translator: %s is name of a SQL construct, eg GROUP BY */
+				 errmsg("aggregate functions are not allowed in %s",
+						ParseExprKindName(pstate->p_expr_kind)),
+				 parser_errposition(pstate, agg->location)));
+}
 
-	if (min_varlevel < 0)
-		min_varlevel = 0;
-	agg->agglevelsup = min_varlevel;
+/*
+ * check_agg_arguments
+ *	  Scan the arguments of an aggregate function to determine the
+ *	  aggregate's semantic level (zero is the current select's level,
+ *	  one is its parent, etc).
+ *
+ * The aggregate's level is the same as the level of the lowest-level variable
+ * or aggregate in its arguments; or if it contains no variables at all, we
+ * presume it to be local.
+ *
+ * We also take this opportunity to detect any aggregates or window functions
+ * nested within the arguments.  We can throw error immediately if we find
+ * a window function.  Aggregates are a bit trickier because it's only an
+ * error if the inner aggregate is of the same semantic level as the outer,
+ * which we can't know until we finish scanning the arguments.
+ */
+static int
+check_agg_arguments(ParseState *pstate, List *args)
+{
+	int			agglevel;
+	check_agg_arguments_context context;
 
-	/* Mark the correct pstate as having aggregates */
-	while (min_varlevel-- > 0)
-		pstate = pstate->parentParseState;
-	pstate->p_hasAggs = true;
+	context.pstate = pstate;
+	context.min_varlevel = -1;	/* signifies nothing found yet */
+	context.min_agglevel = -1;
+	context.sublevels_up = 0;
+
+	(void) expression_tree_walker((Node *) args,
+								  check_agg_arguments_walker,
+								  (void *) &context);
+
+	/*
+	 * If we found no vars nor aggs at all, it's a level-zero aggregate;
+	 * otherwise, its level is the minimum of vars or aggs.
+	 */
+	if (context.min_varlevel < 0)
+	{
+		if (context.min_agglevel < 0)
+			return 0;
+		agglevel = context.min_agglevel;
+	}
+	else if (context.min_agglevel < 0)
+		agglevel = context.min_varlevel;
+	else
+		agglevel = Min(context.min_varlevel, context.min_agglevel);
 
 	/*
-	 * Complain if we are inside a LATERAL subquery of the aggregation query.
-	 * We must be in its FROM clause, so the aggregate is misplaced.
+	 * If there's a nested aggregate of the same semantic level, complain.
 	 */
-	if (pstate->p_lateral_active)
+	if (agglevel == context.min_agglevel)
 		ereport(ERROR,
 				(errcode(ERRCODE_GROUPING_ERROR),
-				 errmsg("aggregates not allowed in FROM clause"),
-				 parser_errposition(pstate, agg->location)));
+				 errmsg("aggregate function calls cannot be nested"),
+				 parser_errposition(pstate,
+									locate_agg_of_level((Node *) args,
+														agglevel))));
+
+	return agglevel;
+}
+
+static bool
+check_agg_arguments_walker(Node *node,
+						   check_agg_arguments_context *context)
+{
+	if (node == NULL)
+		return false;
+	if (IsA(node, Var))
+	{
+		int			varlevelsup = ((Var *) node)->varlevelsup;
+
+		/* convert levelsup to frame of reference of original query */
+		varlevelsup -= context->sublevels_up;
+		/* ignore local vars of subqueries */
+		if (varlevelsup >= 0)
+		{
+			if (context->min_varlevel < 0 ||
+				context->min_varlevel > varlevelsup)
+				context->min_varlevel = varlevelsup;
+		}
+		return false;
+	}
+	if (IsA(node, Aggref))
+	{
+		int			agglevelsup = ((Aggref *) node)->agglevelsup;
+
+		/* convert levelsup to frame of reference of original query */
+		agglevelsup -= context->sublevels_up;
+		/* ignore local aggs of subqueries */
+		if (agglevelsup >= 0)
+		{
+			if (context->min_agglevel < 0 ||
+				context->min_agglevel > agglevelsup)
+				context->min_agglevel = agglevelsup;
+		}
+		/* no need to examine args of the inner aggregate */
+		return false;
+	}
+	/* We can throw error on sight for a window function */
+	if (IsA(node, WindowFunc))
+		ereport(ERROR,
+				(errcode(ERRCODE_GROUPING_ERROR),
+				 errmsg("aggregate function calls cannot contain window function calls"),
+				 parser_errposition(context->pstate,
+									((WindowFunc *) node)->location)));
+	if (IsA(node, Query))
+	{
+		/* Recurse into subselects */
+		bool		result;
+
+		context->sublevels_up++;
+		result = query_tree_walker((Query *) node,
+								   check_agg_arguments_walker,
+								   (void *) context,
+								   0);
+		context->sublevels_up--;
+		return result;
+	}
+	return expression_tree_walker(node,
+								  check_agg_arguments_walker,
+								  (void *) context);
 }
 
 /*
@@ -208,12 +429,15 @@ void
 transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
 						WindowDef *windef)
 {
+	const char *err;
+	bool		errkind;
+
 	/*
 	 * A window function call can't contain another one (but aggs are OK). XXX
 	 * is this required by spec, or just an unimplemented feature?
 	 */
 	if (pstate->p_hasWindowFuncs &&
-		checkExprHasWindowFuncs((Node *) wfunc->args))
+		contain_windowfuncs((Node *) wfunc->args))
 		ereport(ERROR,
 				(errcode(ERRCODE_WINDOWING_ERROR),
 				 errmsg("window function calls cannot be nested"),
@@ -221,6 +445,121 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
 								  locate_windowfunc((Node *) wfunc->args))));
 
 	/*
+	 * Check to see if the window function is in an invalid place within the
+	 * query.
+	 *
+	 * For brevity we support two schemes for reporting an error here: set
+	 * "err" to a custom message, or set "errkind" true if the error context
+	 * is sufficiently identified by what ParseExprKindName will return, *and*
+	 * what it will return is just a SQL keyword.  (Otherwise, use a custom
+	 * message to avoid creating translation problems.)
+	 */
+	err = NULL;
+	errkind = false;
+	switch (pstate->p_expr_kind)
+	{
+		case EXPR_KIND_NONE:
+			Assert(false);		/* can't happen */
+			break;
+		case EXPR_KIND_OTHER:
+			/* Accept window func here; caller must throw error if wanted */
+			break;
+		case EXPR_KIND_JOIN_ON:
+		case EXPR_KIND_JOIN_USING:
+			err = _("window functions are not allowed in JOIN conditions");
+			break;
+		case EXPR_KIND_FROM_SUBSELECT:
+			/* can't get here, but just in case, throw an error */
+			errkind = true;
+			break;
+		case EXPR_KIND_FROM_FUNCTION:
+			err = _("window functions are not allowed in functions in FROM");
+			break;
+		case EXPR_KIND_WHERE:
+			errkind = true;
+			break;
+		case EXPR_KIND_HAVING:
+			errkind = true;
+			break;
+		case EXPR_KIND_WINDOW_PARTITION:
+		case EXPR_KIND_WINDOW_ORDER:
+		case EXPR_KIND_WINDOW_FRAME_RANGE:
+		case EXPR_KIND_WINDOW_FRAME_ROWS:
+			err = _("window functions are not allowed in window definitions");
+			break;
+		case EXPR_KIND_SELECT_TARGET:
+			/* okay */
+			break;
+		case EXPR_KIND_INSERT_TARGET:
+		case EXPR_KIND_UPDATE_SOURCE:
+		case EXPR_KIND_UPDATE_TARGET:
+			errkind = true;
+			break;
+		case EXPR_KIND_GROUP_BY:
+			errkind = true;
+			break;
+		case EXPR_KIND_ORDER_BY:
+			/* okay */
+			break;
+		case EXPR_KIND_DISTINCT_ON:
+			/* okay */
+			break;
+		case EXPR_KIND_LIMIT:
+		case EXPR_KIND_OFFSET:
+			errkind = true;
+			break;
+		case EXPR_KIND_RETURNING:
+			errkind = true;
+			break;
+		case EXPR_KIND_VALUES:
+			errkind = true;
+			break;
+		case EXPR_KIND_CHECK_CONSTRAINT:
+		case EXPR_KIND_DOMAIN_CHECK:
+			err = _("window functions are not allowed in CHECK constraints");
+			break;
+		case EXPR_KIND_COLUMN_DEFAULT:
+		case EXPR_KIND_FUNCTION_DEFAULT:
+			err = _("window functions are not allowed in DEFAULT expressions");
+			break;
+		case EXPR_KIND_INDEX_EXPRESSION:
+			err = _("window functions are not allowed in index expressions");
+			break;
+		case EXPR_KIND_INDEX_PREDICATE:
+			err = _("window functions are not allowed in index predicates");
+			break;
+		case EXPR_KIND_ALTER_COL_TRANSFORM:
+			err = _("window functions are not allowed in transform expressions");
+			break;
+		case EXPR_KIND_EXECUTE_PARAMETER:
+			err = _("window functions are not allowed in EXECUTE parameters");
+			break;
+		case EXPR_KIND_TRIGGER_WHEN:
+			err = _("window functions are not allowed in trigger WHEN conditions");
+			break;
+
+			/*
+			 * There is intentionally no default: case here, so that the
+			 * compiler will warn if we add a new ParseExprKind without
+			 * extending this switch.  If we do see an unrecognized value at
+			 * runtime, the behavior will be the same as for EXPR_KIND_OTHER,
+			 * which is sane anyway.
+			 */
+	}
+	if (err)
+		ereport(ERROR,
+				(errcode(ERRCODE_WINDOWING_ERROR),
+				 errmsg_internal("%s", err),
+				 parser_errposition(pstate, wfunc->location)));
+	if (errkind)
+		ereport(ERROR,
+				(errcode(ERRCODE_WINDOWING_ERROR),
+				 /* translator: %s is name of a SQL construct, eg GROUP BY */
+				 errmsg("window functions are not allowed in %s",
+						ParseExprKindName(pstate->p_expr_kind)),
+				 parser_errposition(pstate, wfunc->location)));
+
+	/*
 	 * If the OVER clause just specifies a window name, find that WINDOW
 	 * clause (which had better be present).  Otherwise, try to match all the
 	 * properties of the OVER clause, and make a new entry in the p_windowdefs
@@ -294,11 +633,14 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
 /*
  * parseCheckAggregates
  *	Check for aggregates where they shouldn't be and improper grouping.
+ *	This function should be called after the target list and qualifications
+ *	are finalized.
  *
- *	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.
+ *	Misplaced aggregates are now mostly detected in transformAggregateCall,
+ *	but it seems more robust to check for aggregates in recursive queries
+ *	only after everything is finalized.  In any case it's hard to detect
+ *	improper grouping on-the-fly, so we have to make another pass over the
+ *	query for that.
  */
 void
 parseCheckAggregates(ParseState *pstate, Query *qry)
@@ -331,31 +673,8 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
 	}
 
 	/*
-	 * Aggregates must never appear in WHERE or JOIN/ON clauses.
-	 *
-	 * (Note this check should appear first to deliver an appropriate error
-	 * message; otherwise we are likely to complain about some innocent
-	 * variable in the target list, which is outright misleading if the
-	 * problem is in WHERE.)
-	 */
-	if (checkExprHasAggs(qry->jointree->quals))
-		ereport(ERROR,
-				(errcode(ERRCODE_GROUPING_ERROR),
-				 errmsg("aggregates not allowed in WHERE clause"),
-				 parser_errposition(pstate,
-							 locate_agg_of_level(qry->jointree->quals, 0))));
-	if (checkExprHasAggs((Node *) qry->jointree->fromlist))
-		ereport(ERROR,
-				(errcode(ERRCODE_GROUPING_ERROR),
-				 errmsg("aggregates not allowed in JOIN conditions"),
-				 parser_errposition(pstate,
-				 locate_agg_of_level((Node *) qry->jointree->fromlist, 0))));
-
-	/*
-	 * 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().
+	 * Build a list of the acceptable GROUP BY expressions for use by
+	 * check_ungrouped_columns().
 	 */
 	foreach(l, qry->groupClause)
 	{
@@ -365,12 +684,6 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
 		expr = get_sortgroupclause_expr(grpcl, qry->targetList);
 		if (expr == NULL)
 			continue;			/* probably cannot happen */
-		if (checkExprHasAggs(expr))
-			ereport(ERROR,
-					(errcode(ERRCODE_GROUPING_ERROR),
-					 errmsg("aggregates not allowed in GROUP BY clause"),
-					 parser_errposition(pstate,
-										locate_agg_of_level(expr, 0))));
 		groupClauses = lcons(expr, groupClauses);
 	}
 
@@ -438,97 +751,12 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
 	if (pstate->p_hasAggs && hasSelfRefRTEs)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_RECURSION),
-				 errmsg("aggregate functions not allowed in a recursive query's recursive term"),
+				 errmsg("aggregate functions are not allowed in a recursive query's recursive term"),
 				 parser_errposition(pstate,
 									locate_agg_of_level((Node *) qry, 0))));
 }
 
 /*
- * parseCheckWindowFuncs
- *	Check for window functions where they shouldn't be.
- *
- *	We have to forbid window functions in WHERE, JOIN/ON, HAVING, GROUP BY,
- *	and window specifications.	(Other clauses, such as RETURNING and LIMIT,
- *	have already been checked.)  Transformation of all these clauses must
- *	be completed already.
- */
-void
-parseCheckWindowFuncs(ParseState *pstate, Query *qry)
-{
-	ListCell   *l;
-
-	/* This should only be called if we found window functions */
-	Assert(pstate->p_hasWindowFuncs);
-
-	if (checkExprHasWindowFuncs(qry->jointree->quals))
-		ereport(ERROR,
-				(errcode(ERRCODE_WINDOWING_ERROR),
-				 errmsg("window functions not allowed in WHERE clause"),
-				 parser_errposition(pstate,
-								  locate_windowfunc(qry->jointree->quals))));
-	if (checkExprHasWindowFuncs((Node *) qry->jointree->fromlist))
-		ereport(ERROR,
-				(errcode(ERRCODE_WINDOWING_ERROR),
-				 errmsg("window functions not allowed in JOIN conditions"),
-				 parser_errposition(pstate,
-					  locate_windowfunc((Node *) qry->jointree->fromlist))));
-	if (checkExprHasWindowFuncs(qry->havingQual))
-		ereport(ERROR,
-				(errcode(ERRCODE_WINDOWING_ERROR),
-				 errmsg("window functions not allowed in HAVING clause"),
-				 parser_errposition(pstate,
-									locate_windowfunc(qry->havingQual))));
-
-	foreach(l, qry->groupClause)
-	{
-		SortGroupClause *grpcl = (SortGroupClause *) lfirst(l);
-		Node	   *expr;
-
-		expr = get_sortgroupclause_expr(grpcl, qry->targetList);
-		if (checkExprHasWindowFuncs(expr))
-			ereport(ERROR,
-					(errcode(ERRCODE_WINDOWING_ERROR),
-				   errmsg("window functions not allowed in GROUP BY clause"),
-					 parser_errposition(pstate,
-										locate_windowfunc(expr))));
-	}
-
-	foreach(l, qry->windowClause)
-	{
-		WindowClause *wc = (WindowClause *) lfirst(l);
-		ListCell   *l2;
-
-		foreach(l2, wc->partitionClause)
-		{
-			SortGroupClause *grpcl = (SortGroupClause *) lfirst(l2);
-			Node	   *expr;
-
-			expr = get_sortgroupclause_expr(grpcl, qry->targetList);
-			if (checkExprHasWindowFuncs(expr))
-				ereport(ERROR,
-						(errcode(ERRCODE_WINDOWING_ERROR),
-				 errmsg("window functions not allowed in window definition"),
-						 parser_errposition(pstate,
-											locate_windowfunc(expr))));
-		}
-		foreach(l2, wc->orderClause)
-		{
-			SortGroupClause *grpcl = (SortGroupClause *) lfirst(l2);
-			Node	   *expr;
-
-			expr = get_sortgroupclause_expr(grpcl, qry->targetList);
-			if (checkExprHasWindowFuncs(expr))
-				ereport(ERROR,
-						(errcode(ERRCODE_WINDOWING_ERROR),
-				 errmsg("window functions not allowed in window definition"),
-						 parser_errposition(pstate,
-											locate_windowfunc(expr))));
-		}
-		/* startOffset and limitOffset were checked in transformFrameOffset */
-	}
-}
-
-/*
  * check_ungrouped_columns -
  *	  Scan the given expression tree for ungrouped variables (variables
  *	  that are not listed in the groupClauses list and are not within