From eaccfded98a9c677d3a2e849c1747ec90e8596a6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 10 Aug 2012 11:35:33 -0400 Subject: 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.) --- src/include/parser/parse_node.h | 49 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) (limited to 'src/include/parser/parse_node.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 @@ -18,6 +18,54 @@ #include "utils/relcache.h" +/* + * 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 */ @@ -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 */ -- cgit v1.2.3