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.)

7 files changed, 64 insertions, 12 deletions

diff --git a/src/include/optimizer/var.h b/src/include/optimizer/var.h
index ec21df3a7e0..e3ba3144f36 100644
--- a/src/include/optimizer/var.h
+++ b/src/include/optimizer/var.h
@@ -37,7 +37,6 @@ extern List *pull_vars_of_level(Node *node, int levelsup);
 extern bool contain_var_clause(Node *node);
 extern bool contain_vars_of_level(Node *node, int levelsup);
 extern int	locate_var_of_level(Node *node, int levelsup);
-extern int	find_minimum_var_level(Node *node);
 extern List *pull_var_clause(Node *node, PVCAggregateBehavior aggbehavior,
 				PVCPlaceHolderBehavior phbehavior);
 extern Node *flatten_join_alias_vars(PlannerInfo *root, Node *node);
diff --git a/src/include/parser/parse_agg.h b/src/include/parser/parse_agg.h
index b32ee6c2725..c51fdd81411 100644
--- a/src/include/parser/parse_agg.h
+++ b/src/include/parser/parse_agg.h
@@ -22,7 +22,6 @@ extern void transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
 						WindowDef *windef);
 
 extern void parseCheckAggregates(ParseState *pstate, Query *qry);
-extern void parseCheckWindowFuncs(ParseState *pstate, Query *qry);
 
 extern void build_aggregate_fnexprs(Oid *agg_input_types,
 						int agg_num_inputs,
diff --git a/src/include/parser/parse_clause.h b/src/include/parser/parse_clause.h
index fd3fc8f5709..5d59ee973c7 100644
--- a/src/include/parser/parse_clause.h
+++ b/src/include/parser/parse_clause.h
@@ -23,14 +23,15 @@ extern bool interpretInhOption(InhOption inhOpt);
 extern bool interpretOidsOption(List *defList);
 
 extern Node *transformWhereClause(ParseState *pstate, Node *clause,
-					 const char *constructName);
+					 ParseExprKind exprKind, const char *constructName);
 extern Node *transformLimitClause(ParseState *pstate, Node *clause,
-					 const char *constructName);
+					 ParseExprKind exprKind, const char *constructName);
 extern List *transformGroupClause(ParseState *pstate, List *grouplist,
 					 List **targetlist, List *sortClause,
-					 bool useSQL99);
+					 ParseExprKind exprKind, bool useSQL99);
 extern List *transformSortClause(ParseState *pstate, List *orderlist,
-					List **targetlist, bool resolveUnknown, bool useSQL99);
+					List **targetlist, ParseExprKind exprKind,
+					bool resolveUnknown, bool useSQL99);
 
 extern List *transformWindowDefinitions(ParseState *pstate,
 						   List *windowdefs,
diff --git a/src/include/parser/parse_expr.h b/src/include/parser/parse_expr.h
index cbf281e315c..b9b9c7ee4dd 100644
--- a/src/include/parser/parse_expr.h
+++ b/src/include/parser/parse_expr.h
@@ -18,6 +18,8 @@
 /* GUC parameters */
 extern bool Transform_null_equals;
 
-extern Node *transformExpr(ParseState *pstate, Node *expr);
+extern Node *transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind);
+
+extern const char *ParseExprKindName(ParseExprKind exprKind);
 
 #endif   /* PARSE_EXPR_H */
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 200b9744e51..e3bb35f1308 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -19,6 +19,54 @@
 
 
 /*
+ * Expression kinds distinguished by transformExpr().  Many of these are not
+ * semantically distinct so far as expression transformation goes; rather,
+ * we distinguish them so that context-specific error messages can be printed.
+ *
+ * Note: EXPR_KIND_OTHER is not used in the core code, but is left for use
+ * by extension code that might need to call transformExpr().  The core code
+ * will not enforce any context-driven restrictions on EXPR_KIND_OTHER
+ * expressions, so the caller would have to check for sub-selects, aggregates,
+ * and window functions if those need to be disallowed.
+ */
+typedef enum ParseExprKind
+{
+	EXPR_KIND_NONE = 0,				/* "not in an expression" */
+	EXPR_KIND_OTHER,				/* reserved for extensions */
+	EXPR_KIND_JOIN_ON,				/* JOIN ON */
+	EXPR_KIND_JOIN_USING,			/* JOIN USING */
+	EXPR_KIND_FROM_SUBSELECT,		/* sub-SELECT in FROM clause */
+	EXPR_KIND_FROM_FUNCTION,		/* function in FROM clause */
+	EXPR_KIND_WHERE,				/* WHERE */
+	EXPR_KIND_HAVING,				/* HAVING */
+	EXPR_KIND_WINDOW_PARTITION,		/* window definition PARTITION BY */
+	EXPR_KIND_WINDOW_ORDER,			/* window definition ORDER BY */
+	EXPR_KIND_WINDOW_FRAME_RANGE,	/* window frame clause with RANGE */
+	EXPR_KIND_WINDOW_FRAME_ROWS,	/* window frame clause with ROWS */
+	EXPR_KIND_SELECT_TARGET,		/* SELECT target list item */
+	EXPR_KIND_INSERT_TARGET,		/* INSERT target list item */
+	EXPR_KIND_UPDATE_SOURCE,		/* UPDATE assignment source item */
+	EXPR_KIND_UPDATE_TARGET,		/* UPDATE assignment target item */
+	EXPR_KIND_GROUP_BY,				/* GROUP BY */
+	EXPR_KIND_ORDER_BY,				/* ORDER BY */
+	EXPR_KIND_DISTINCT_ON,			/* DISTINCT ON */
+	EXPR_KIND_LIMIT,				/* LIMIT */
+	EXPR_KIND_OFFSET,				/* OFFSET */
+	EXPR_KIND_RETURNING,			/* RETURNING */
+	EXPR_KIND_VALUES,				/* VALUES */
+	EXPR_KIND_CHECK_CONSTRAINT,		/* CHECK constraint for a table */
+	EXPR_KIND_DOMAIN_CHECK,			/* CHECK constraint for a domain */
+	EXPR_KIND_COLUMN_DEFAULT,		/* default value for a table column */
+	EXPR_KIND_FUNCTION_DEFAULT,		/* default parameter value for function */
+	EXPR_KIND_INDEX_EXPRESSION,		/* index expression */
+	EXPR_KIND_INDEX_PREDICATE,		/* index predicate */
+	EXPR_KIND_ALTER_COL_TRANSFORM,	/* transform expr in ALTER COLUMN TYPE */
+	EXPR_KIND_EXECUTE_PARAMETER,	/* parameter value in EXECUTE */
+	EXPR_KIND_TRIGGER_WHEN			/* WHEN condition in CREATE TRIGGER */
+} ParseExprKind;
+
+
+/*
  * Function signatures for parser hooks
  */
 typedef struct ParseState ParseState;
@@ -93,6 +141,7 @@ struct ParseState
 	List	   *p_future_ctes;	/* common table exprs not yet in namespace */
 	CommonTableExpr *p_parent_cte;		/* this query's containing CTE */
 	List	   *p_windowdefs;	/* raw representations of window clauses */
+	ParseExprKind p_expr_kind;	/* what kind of expression we're parsing */
 	int			p_next_resno;	/* next targetlist resno to assign */
 	List	   *p_locking_clause;		/* raw FOR UPDATE/FOR SHARE info */
 	Node	   *p_value_substitute;		/* what to replace VALUE with, if any */
diff --git a/src/include/parser/parse_target.h b/src/include/parser/parse_target.h
index d274a66b13e..e5bbaf4e12e 100644
--- a/src/include/parser/parse_target.h
+++ b/src/include/parser/parse_target.h
@@ -17,13 +17,16 @@
 #include "parser/parse_node.h"
 
 
-extern List *transformTargetList(ParseState *pstate, List *targetlist);
-extern List *transformExpressionList(ParseState *pstate, List *exprlist);
+extern List *transformTargetList(ParseState *pstate, List *targetlist,
+					ParseExprKind exprKind);
+extern List *transformExpressionList(ParseState *pstate, List *exprlist,
+						ParseExprKind exprKind);
 extern void markTargetListOrigins(ParseState *pstate, List *targetlist);
 extern TargetEntry *transformTargetEntry(ParseState *pstate,
-					 Node *node, Node *expr,
+					 Node *node, Node *expr, ParseExprKind exprKind,
 					 char *colname, bool resjunk);
 extern Expr *transformAssignedExpr(ParseState *pstate, Expr *expr,
+					  ParseExprKind exprKind,
 					  char *colname,
 					  int attrno,
 					  List *indirection,
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index 6f57b37b814..e13331dcb5e 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -52,9 +52,8 @@ extern void AddInvertedQual(Query *parsetree, Node *qual);
 
 extern bool contain_aggs_of_level(Node *node, int levelsup);
 extern int	locate_agg_of_level(Node *node, int levelsup);
+extern bool contain_windowfuncs(Node *node);
 extern int	locate_windowfunc(Node *node);
-extern bool checkExprHasAggs(Node *node);
-extern bool checkExprHasWindowFuncs(Node *node);
 extern bool checkExprHasSubLink(Node *node);
 
 extern Node *replace_rte_variables(Node *node,