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, 230 insertions, 44 deletions

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 385f8e767e4..e9267c56fc5 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -37,6 +37,7 @@
 
 bool		Transform_null_equals = false;
 
+static Node *transformExprRecurse(ParseState *pstate, Node *expr);
 static Node *transformParamRef(ParseState *pstate, ParamRef *pref);
 static Node *transformAExprOp(ParseState *pstate, A_Expr *a);
 static Node *transformAExprAnd(ParseState *pstate, A_Expr *a);
@@ -100,9 +101,27 @@ static Expr *make_distinct_op(ParseState *pstate, List *opname,
  * input and output of transformExpr; see SubLink for example.
  */
 Node *
-transformExpr(ParseState *pstate, Node *expr)
+transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind)
 {
-	Node	   *result = NULL;
+	Node	   *result;
+	ParseExprKind sv_expr_kind;
+
+	/* Save and restore identity of expression type we're parsing */
+	Assert(exprKind != EXPR_KIND_NONE);
+	sv_expr_kind = pstate->p_expr_kind;
+	pstate->p_expr_kind = exprKind;
+
+	result = transformExprRecurse(pstate, expr);
+
+	pstate->p_expr_kind = sv_expr_kind;
+
+	return result;
+}
+
+static Node *
+transformExprRecurse(ParseState *pstate, Node *expr)
+{
+	Node	   *result;
 
 	if (expr == NULL)
 		return NULL;
@@ -133,7 +152,7 @@ transformExpr(ParseState *pstate, Node *expr)
 			{
 				A_Indirection *ind = (A_Indirection *) expr;
 
-				result = transformExpr(pstate, ind->arg);
+				result = transformExprRecurse(pstate, ind->arg);
 				result = transformIndirection(pstate, result,
 											  ind->indirection);
 				break;
@@ -230,6 +249,8 @@ transformExpr(ParseState *pstate, Node *expr)
 						break;
 					default:
 						elog(ERROR, "unrecognized A_Expr kind: %d", a->kind);
+						result = NULL;		/* keep compiler quiet */
+						break;
 				}
 				break;
 			}
@@ -242,7 +263,7 @@ transformExpr(ParseState *pstate, Node *expr)
 			{
 				NamedArgExpr *na = (NamedArgExpr *) expr;
 
-				na->arg = (Expr *) transformExpr(pstate, (Node *) na->arg);
+				na->arg = (Expr *) transformExprRecurse(pstate, (Node *) na->arg);
 				result = expr;
 				break;
 			}
@@ -279,7 +300,7 @@ transformExpr(ParseState *pstate, Node *expr)
 			{
 				NullTest   *n = (NullTest *) expr;
 
-				n->arg = (Expr *) transformExpr(pstate, (Node *) n->arg);
+				n->arg = (Expr *) transformExprRecurse(pstate, (Node *) n->arg);
 				/* the argument can be any type, so don't coerce it */
 				n->argisrow = type_is_rowtype(exprType((Node *) n->arg));
 				result = expr;
@@ -334,6 +355,7 @@ transformExpr(ParseState *pstate, Node *expr)
 		default:
 			/* should not reach here */
 			elog(ERROR, "unrecognized node type: %d", (int) nodeTag(expr));
+			result = NULL;		/* keep compiler quiet */
 			break;
 	}
 
@@ -843,7 +865,7 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 		else
 			n->arg = (Expr *) lexpr;
 
-		result = transformExpr(pstate, (Node *) n);
+		result = transformExprRecurse(pstate, (Node *) n);
 	}
 	else if (lexpr && IsA(lexpr, RowExpr) &&
 			 rexpr && IsA(rexpr, SubLink) &&
@@ -860,14 +882,14 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 		s->testexpr = lexpr;
 		s->operName = a->name;
 		s->location = a->location;
-		result = transformExpr(pstate, (Node *) s);
+		result = transformExprRecurse(pstate, (Node *) s);
 	}
 	else if (lexpr && IsA(lexpr, RowExpr) &&
 			 rexpr && IsA(rexpr, RowExpr))
 	{
 		/* "row op row" */
-		lexpr = transformExpr(pstate, lexpr);
-		rexpr = transformExpr(pstate, rexpr);
+		lexpr = transformExprRecurse(pstate, lexpr);
+		rexpr = transformExprRecurse(pstate, rexpr);
 		Assert(IsA(lexpr, RowExpr));
 		Assert(IsA(rexpr, RowExpr));
 
@@ -880,8 +902,8 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	else
 	{
 		/* Ordinary scalar operator */
-		lexpr = transformExpr(pstate, lexpr);
-		rexpr = transformExpr(pstate, rexpr);
+		lexpr = transformExprRecurse(pstate, lexpr);
+		rexpr = transformExprRecurse(pstate, rexpr);
 
 		result = (Node *) make_op(pstate,
 								  a->name,
@@ -896,8 +918,8 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 static Node *
 transformAExprAnd(ParseState *pstate, A_Expr *a)
 {
-	Node	   *lexpr = transformExpr(pstate, a->lexpr);
-	Node	   *rexpr = transformExpr(pstate, a->rexpr);
+	Node	   *lexpr = transformExprRecurse(pstate, a->lexpr);
+	Node	   *rexpr = transformExprRecurse(pstate, a->rexpr);
 
 	lexpr = coerce_to_boolean(pstate, lexpr, "AND");
 	rexpr = coerce_to_boolean(pstate, rexpr, "AND");
@@ -910,8 +932,8 @@ transformAExprAnd(ParseState *pstate, A_Expr *a)
 static Node *
 transformAExprOr(ParseState *pstate, A_Expr *a)
 {
-	Node	   *lexpr = transformExpr(pstate, a->lexpr);
-	Node	   *rexpr = transformExpr(pstate, a->rexpr);
+	Node	   *lexpr = transformExprRecurse(pstate, a->lexpr);
+	Node	   *rexpr = transformExprRecurse(pstate, a->rexpr);
 
 	lexpr = coerce_to_boolean(pstate, lexpr, "OR");
 	rexpr = coerce_to_boolean(pstate, rexpr, "OR");
@@ -924,7 +946,7 @@ transformAExprOr(ParseState *pstate, A_Expr *a)
 static Node *
 transformAExprNot(ParseState *pstate, A_Expr *a)
 {
-	Node	   *rexpr = transformExpr(pstate, a->rexpr);
+	Node	   *rexpr = transformExprRecurse(pstate, a->rexpr);
 
 	rexpr = coerce_to_boolean(pstate, rexpr, "NOT");
 
@@ -936,8 +958,8 @@ transformAExprNot(ParseState *pstate, A_Expr *a)
 static Node *
 transformAExprOpAny(ParseState *pstate, A_Expr *a)
 {
-	Node	   *lexpr = transformExpr(pstate, a->lexpr);
-	Node	   *rexpr = transformExpr(pstate, a->rexpr);
+	Node	   *lexpr = transformExprRecurse(pstate, a->lexpr);
+	Node	   *rexpr = transformExprRecurse(pstate, a->rexpr);
 
 	return (Node *) make_scalar_array_op(pstate,
 										 a->name,
@@ -950,8 +972,8 @@ transformAExprOpAny(ParseState *pstate, A_Expr *a)
 static Node *
 transformAExprOpAll(ParseState *pstate, A_Expr *a)
 {
-	Node	   *lexpr = transformExpr(pstate, a->lexpr);
-	Node	   *rexpr = transformExpr(pstate, a->rexpr);
+	Node	   *lexpr = transformExprRecurse(pstate, a->lexpr);
+	Node	   *rexpr = transformExprRecurse(pstate, a->rexpr);
 
 	return (Node *) make_scalar_array_op(pstate,
 										 a->name,
@@ -964,8 +986,8 @@ transformAExprOpAll(ParseState *pstate, A_Expr *a)
 static Node *
 transformAExprDistinct(ParseState *pstate, A_Expr *a)
 {
-	Node	   *lexpr = transformExpr(pstate, a->lexpr);
-	Node	   *rexpr = transformExpr(pstate, a->rexpr);
+	Node	   *lexpr = transformExprRecurse(pstate, a->lexpr);
+	Node	   *rexpr = transformExprRecurse(pstate, a->rexpr);
 
 	if (lexpr && IsA(lexpr, RowExpr) &&
 		rexpr && IsA(rexpr, RowExpr))
@@ -990,8 +1012,8 @@ transformAExprDistinct(ParseState *pstate, A_Expr *a)
 static Node *
 transformAExprNullIf(ParseState *pstate, A_Expr *a)
 {
-	Node	   *lexpr = transformExpr(pstate, a->lexpr);
-	Node	   *rexpr = transformExpr(pstate, a->rexpr);
+	Node	   *lexpr = transformExprRecurse(pstate, a->lexpr);
+	Node	   *rexpr = transformExprRecurse(pstate, a->rexpr);
 	OpExpr	   *result;
 
 	result = (OpExpr *) make_op(pstate,
@@ -1029,7 +1051,7 @@ transformAExprOf(ParseState *pstate, A_Expr *a)
 	 * Checking an expression for match to a list of type names. Will result
 	 * in a boolean constant node.
 	 */
-	Node	   *lexpr = transformExpr(pstate, a->lexpr);
+	Node	   *lexpr = transformExprRecurse(pstate, a->lexpr);
 	Const	   *result;
 	ListCell   *telem;
 	Oid			ltype,
@@ -1092,12 +1114,12 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
 	 * First step: transform all the inputs, and detect whether any are
 	 * RowExprs or contain Vars.
 	 */
-	lexpr = transformExpr(pstate, a->lexpr);
+	lexpr = transformExprRecurse(pstate, a->lexpr);
 	haveRowExpr = (lexpr && IsA(lexpr, RowExpr));
 	rexprs = rvars = rnonvars = NIL;
 	foreach(l, (List *) a->rexpr)
 	{
-		Node	   *rexpr = transformExpr(pstate, lfirst(l));
+		Node	   *rexpr = transformExprRecurse(pstate, lfirst(l));
 
 		haveRowExpr |= (rexpr && IsA(rexpr, RowExpr));
 		rexprs = lappend(rexprs, rexpr);
@@ -1222,8 +1244,8 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
 	targs = NIL;
 	foreach(args, fn->args)
 	{
-		targs = lappend(targs, transformExpr(pstate,
-											 (Node *) lfirst(args)));
+		targs = lappend(targs, transformExprRecurse(pstate,
+													(Node *) lfirst(args)));
 	}
 
 	/* ... and hand off to ParseFuncOrColumn */
@@ -1258,7 +1280,7 @@ transformCaseExpr(ParseState *pstate, CaseExpr *c)
 	newc = makeNode(CaseExpr);
 
 	/* transform the test expression, if any */
-	arg = transformExpr(pstate, (Node *) c->arg);
+	arg = transformExprRecurse(pstate, (Node *) c->arg);
 
 	/* generate placeholder for test expression */
 	if (arg)
@@ -1311,14 +1333,14 @@ transformCaseExpr(ParseState *pstate, CaseExpr *c)
 											 warg,
 											 w->location);
 		}
-		neww->expr = (Expr *) transformExpr(pstate, warg);
+		neww->expr = (Expr *) transformExprRecurse(pstate, warg);
 
 		neww->expr = (Expr *) coerce_to_boolean(pstate,
 												(Node *) neww->expr,
 												"CASE/WHEN");
 
 		warg = (Node *) w->result;
-		neww->result = (Expr *) transformExpr(pstate, warg);
+		neww->result = (Expr *) transformExprRecurse(pstate, warg);
 		neww->location = w->location;
 
 		newargs = lappend(newargs, neww);
@@ -1337,7 +1359,7 @@ transformCaseExpr(ParseState *pstate, CaseExpr *c)
 		n->location = -1;
 		defresult = (Node *) n;
 	}
-	newc->defresult = (Expr *) transformExpr(pstate, defresult);
+	newc->defresult = (Expr *) transformExprRecurse(pstate, defresult);
 
 	/*
 	 * Note: default result is considered the most significant type in
@@ -1380,12 +1402,92 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
 {
 	Node	   *result = (Node *) sublink;
 	Query	   *qtree;
+	const char *err;
 
 	/* If we already transformed this node, do nothing */
 	if (IsA(sublink->subselect, Query))
 		return result;
 
+	/*
+	 * Check to see if the sublink is in an invalid place within the query.
+	 * We allow sublinks everywhere in SELECT/INSERT/UPDATE/DELETE, but
+	 * generally not in utility statements.
+	 */
+	err = NULL;
+	switch (pstate->p_expr_kind)
+	{
+		case EXPR_KIND_NONE:
+			Assert(false);		/* can't happen */
+			break;
+		case EXPR_KIND_OTHER:
+			/* Accept sublink here; caller must throw error if wanted */
+			break;
+		case EXPR_KIND_JOIN_ON:
+		case EXPR_KIND_JOIN_USING:
+		case EXPR_KIND_FROM_SUBSELECT:
+		case EXPR_KIND_FROM_FUNCTION:
+		case EXPR_KIND_WHERE:
+		case EXPR_KIND_HAVING:
+		case EXPR_KIND_WINDOW_PARTITION:
+		case EXPR_KIND_WINDOW_ORDER:
+		case EXPR_KIND_WINDOW_FRAME_RANGE:
+		case EXPR_KIND_WINDOW_FRAME_ROWS:
+		case EXPR_KIND_SELECT_TARGET:
+		case EXPR_KIND_INSERT_TARGET:
+		case EXPR_KIND_UPDATE_SOURCE:
+		case EXPR_KIND_UPDATE_TARGET:
+		case EXPR_KIND_GROUP_BY:
+		case EXPR_KIND_ORDER_BY:
+		case EXPR_KIND_DISTINCT_ON:
+		case EXPR_KIND_LIMIT:
+		case EXPR_KIND_OFFSET:
+		case EXPR_KIND_RETURNING:
+		case EXPR_KIND_VALUES:
+			/* okay */
+			break;
+		case EXPR_KIND_CHECK_CONSTRAINT:
+		case EXPR_KIND_DOMAIN_CHECK:
+			err = _("cannot use subquery in CHECK constraint");
+			break;
+		case EXPR_KIND_COLUMN_DEFAULT:
+		case EXPR_KIND_FUNCTION_DEFAULT:
+			err = _("cannot use subquery in DEFAULT expression");
+			break;
+		case EXPR_KIND_INDEX_EXPRESSION:
+			err = _("cannot use subquery in index expression");
+			break;
+		case EXPR_KIND_INDEX_PREDICATE:
+			err = _("cannot use subquery in index predicate");
+			break;
+		case EXPR_KIND_ALTER_COL_TRANSFORM:
+			err = _("cannot use subquery in transform expression");
+			break;
+		case EXPR_KIND_EXECUTE_PARAMETER:
+			err = _("cannot use subquery in EXECUTE parameter");
+			break;
+		case EXPR_KIND_TRIGGER_WHEN:
+			err = _("cannot use subquery in trigger WHEN condition");
+			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_FEATURE_NOT_SUPPORTED),
+				 errmsg_internal("%s", err),
+				 parser_errposition(pstate, sublink->location)));
+
 	pstate->p_hasSubLinks = true;
+
+	/*
+	 * OK, let's transform the sub-SELECT.
+	 */
 	qtree = parse_sub_analyze(sublink->subselect, pstate, NULL, false);
 
 	/*
@@ -1450,7 +1552,7 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
 		/*
 		 * Transform lefthand expression, and convert to a list
 		 */
-		lefthand = transformExpr(pstate, sublink->testexpr);
+		lefthand = transformExprRecurse(pstate, sublink->testexpr);
 		if (lefthand && IsA(lefthand, RowExpr))
 			left_list = ((RowExpr *) lefthand)->args;
 		else
@@ -1557,7 +1659,7 @@ transformArrayExpr(ParseState *pstate, A_ArrayExpr *a,
 		}
 		else
 		{
-			newe = transformExpr(pstate, e);
+			newe = transformExprRecurse(pstate, e);
 
 			/*
 			 * Check for sub-array expressions, if we haven't already found
@@ -1679,7 +1781,7 @@ transformRowExpr(ParseState *pstate, RowExpr *r)
 	ListCell   *lc;
 
 	/* Transform the field expressions */
-	newr->args = transformExpressionList(pstate, r->args);
+	newr->args = transformExpressionList(pstate, r->args, pstate->p_expr_kind);
 
 	/* Barring later casting, we consider the type RECORD */
 	newr->row_typeid = RECORDOID;
@@ -1712,7 +1814,7 @@ transformCoalesceExpr(ParseState *pstate, CoalesceExpr *c)
 		Node	   *e = (Node *) lfirst(args);
 		Node	   *newe;
 
-		newe = transformExpr(pstate, e);
+		newe = transformExprRecurse(pstate, e);
 		newargs = lappend(newargs, newe);
 	}
 
@@ -1751,7 +1853,7 @@ transformMinMaxExpr(ParseState *pstate, MinMaxExpr *m)
 		Node	   *e = (Node *) lfirst(args);
 		Node	   *newe;
 
-		newe = transformExpr(pstate, e);
+		newe = transformExprRecurse(pstate, e);
 		newargs = lappend(newargs, newe);
 	}
 
@@ -1805,7 +1907,7 @@ transformXmlExpr(ParseState *pstate, XmlExpr *x)
 
 		Assert(IsA(r, ResTarget));
 
-		expr = transformExpr(pstate, r->val);
+		expr = transformExprRecurse(pstate, r->val);
 
 		if (r->name)
 			argname = map_sql_identifier_to_xml_name(r->name, false, false);
@@ -1851,7 +1953,7 @@ transformXmlExpr(ParseState *pstate, XmlExpr *x)
 		Node	   *e = (Node *) lfirst(lc);
 		Node	   *newe;
 
-		newe = transformExpr(pstate, e);
+		newe = transformExprRecurse(pstate, e);
 		switch (x->op)
 		{
 			case IS_XMLCONCAT:
@@ -1914,7 +2016,7 @@ transformXmlSerialize(ParseState *pstate, XmlSerialize *xs)
 	xexpr = makeNode(XmlExpr);
 	xexpr->op = IS_XMLSERIALIZE;
 	xexpr->args = list_make1(coerce_to_specific_type(pstate,
-											 transformExpr(pstate, xs->expr),
+										transformExprRecurse(pstate, xs->expr),
 													 XMLOID,
 													 "XMLSERIALIZE"));
 
@@ -1977,7 +2079,7 @@ transformBooleanTest(ParseState *pstate, BooleanTest *b)
 			clausename = NULL;	/* keep compiler quiet */
 	}
 
-	b->arg = (Expr *) transformExpr(pstate, (Node *) b->arg);
+	b->arg = (Expr *) transformExprRecurse(pstate, (Node *) b->arg);
 
 	b->arg = (Expr *) coerce_to_boolean(pstate,
 										(Node *) b->arg,
@@ -2082,7 +2184,7 @@ static Node *
 transformTypeCast(ParseState *pstate, TypeCast *tc)
 {
 	Node	   *result;
-	Node	   *expr = transformExpr(pstate, tc->arg);
+	Node	   *expr = transformExprRecurse(pstate, tc->arg);
 	Oid			inputType = exprType(expr);
 	Oid			targetType;
 	int32		targetTypmod;
@@ -2130,7 +2232,7 @@ transformCollateClause(ParseState *pstate, CollateClause *c)
 	Oid			argtype;
 
 	newc = makeNode(CollateExpr);
-	newc->arg = (Expr *) transformExpr(pstate, c->arg);
+	newc->arg = (Expr *) transformExprRecurse(pstate, c->arg);
 
 	argtype = exprType((Node *) newc->arg);
 
@@ -2433,3 +2535,87 @@ make_distinct_op(ParseState *pstate, List *opname, Node *ltree, Node *rtree,
 
 	return result;
 }
+
+/*
+ * Produce a string identifying an expression by kind.
+ *
+ * Note: when practical, use a simple SQL keyword for the result.  If that
+ * doesn't work well, check call sites to see whether custom error message
+ * strings are required.
+ */
+const char *
+ParseExprKindName(ParseExprKind exprKind)
+{
+	switch (exprKind)
+	{
+		case EXPR_KIND_NONE:
+			return "invalid expression context";
+		case EXPR_KIND_OTHER:
+			return "extension expression";
+		case EXPR_KIND_JOIN_ON:
+			return "JOIN/ON";
+		case EXPR_KIND_JOIN_USING:
+			return "JOIN/USING";
+		case EXPR_KIND_FROM_SUBSELECT:
+			return "sub-SELECT in FROM";
+		case EXPR_KIND_FROM_FUNCTION:
+			return "function in FROM";
+		case EXPR_KIND_WHERE:
+			return "WHERE";
+		case EXPR_KIND_HAVING:
+			return "HAVING";
+		case EXPR_KIND_WINDOW_PARTITION:
+			return "window PARTITION BY";
+		case EXPR_KIND_WINDOW_ORDER:
+			return "window ORDER BY";
+		case EXPR_KIND_WINDOW_FRAME_RANGE:
+			return "window RANGE";
+		case EXPR_KIND_WINDOW_FRAME_ROWS:
+			return "window ROWS";
+		case EXPR_KIND_SELECT_TARGET:
+			return "SELECT";
+		case EXPR_KIND_INSERT_TARGET:
+			return "INSERT";
+		case EXPR_KIND_UPDATE_SOURCE:
+		case EXPR_KIND_UPDATE_TARGET:
+			return "UPDATE";
+		case EXPR_KIND_GROUP_BY:
+			return "GROUP BY";
+		case EXPR_KIND_ORDER_BY:
+			return "ORDER BY";
+		case EXPR_KIND_DISTINCT_ON:
+			return "DISTINCT ON";
+		case EXPR_KIND_LIMIT:
+			return "LIMIT";
+		case EXPR_KIND_OFFSET:
+			return "OFFSET";
+		case EXPR_KIND_RETURNING:
+			return "RETURNING";
+		case EXPR_KIND_VALUES:
+			return "VALUES";
+		case EXPR_KIND_CHECK_CONSTRAINT:
+		case EXPR_KIND_DOMAIN_CHECK:
+			return "CHECK";
+		case EXPR_KIND_COLUMN_DEFAULT:
+		case EXPR_KIND_FUNCTION_DEFAULT:
+			return "DEFAULT";
+		case EXPR_KIND_INDEX_EXPRESSION:
+			return "index expression";
+		case EXPR_KIND_INDEX_PREDICATE:
+			return "index predicate";
+		case EXPR_KIND_ALTER_COL_TRANSFORM:
+			return "USING";
+		case EXPR_KIND_EXECUTE_PARAMETER:
+			return "EXECUTE";
+		case EXPR_KIND_TRIGGER_WHEN:
+			return "WHEN";
+
+			/*
+			 * 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, we'll fall through to the "unrecognized" return.
+			 */
+	}
+	return "unrecognized expression kind";
+}