8000 Centralize the logic for detecting misplaced aggregates, window funcs… · postgres/postgres@eaccfde · GitHub
[go: up one dir, main page]
Skip to content

Commit eaccfde

Browse files
tglsfdctglsfdc
committed
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 parent b3055ab commit eaccfde

File tree

29 files changed

+952
-781
lines changed
  • window.out
  • with.out
  • sql

    • 29 files changed

    +952
    -781
    lines changed

    ‎src/backend/catalog/heap.c

    Lines changed: 9 additions & 36 deletions
    Original file line numberDiff line numberDiff line change
    @@ -2415,40 +2415,28 @@ cookDefault(ParseState *pstate,
    24152415
    /*
    24162416
    * Transform raw parsetree to executable expression.
    24172417
    */
    2418-
    expr = transformExpr(pstate, raw_default);
    2418+
    expr = transformExpr(pstate, raw_default, EXPR_KIND_COLUMN_DEFAULT);
    24192419

    24202420
    /*
    2421-
    * Make sure default expr does not refer to any vars.
    2421+
    * Make sure default expr does not refer to any vars (we need this check
    2422+
    * since the pstate includes the target table).
    24222423
    */
    24232424
    if (contain_var_clause(expr))
    24242425
    ereport(ERROR,
    24252426
    (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
    24262427
    errmsg("cannot use column references in default expression")));
    24272428

    24282429
    /*
    2430+
    * transformExpr() should have already rejected subqueries, aggregates,
    2431+
    * and window functions, based on the EXPR_KIND_ for a default expression.
    2432+
    *
    24292433
    * It can't return a set either.
    24302434
    */
    24312435
    if (expression_returns_set(expr))
    24322436
    ereport(ERROR,
    24332437
    (errcode(ERRCODE_DATATYPE_MISMATCH),
    24342438
    errmsg("default expression must not return a set")));
    24352439

    2436-
    /*
    2437-
    * No subplans or aggregates, either...
    2438-
    */
    2439-
    if (pstate->p_hasSubLinks)
    2440-
    ereport(ERROR,
    2441-
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    2442-
    errmsg("cannot use subquery in default expression")));
    2443-
    if (pstate->p_hasAggs)
    2444-
    ereport(ERROR,
    2445-
    (errcode(ERRCODE_GROUPING_ERROR),
    2446-
    errmsg("cannot use aggregate function in default expression")));
    2447-
    if (pstate->p_hasWindowFuncs)
    2448-
    ereport(ERROR,
    2449-
    (errcode(ERRCODE_WINDOWING_ERROR),
    2450-
    errmsg("cannot use window function in default expression")));
    2451-
    24522440
    /*
    24532441
    * Coerce the expression to the correct type and typmod, if given. This
    24542442
    * should match the parser's processing of non-defaulted expressions ---
    @@ -2499,7 +2487,7 @@ cookConstraint(ParseState *pstate,
    24992487
    /*
    25002488
    * Transform raw parsetree to executable expression.
    25012489
    */
    2502-
    expr = transformExpr(pstate, raw_constraint);
    2490+
    expr = transformExpr(pstate, raw_constraint, EXPR_KIND_CHECK_CONSTRAINT);
    25032491

    25042492
    /*
    25052493
    * Make sure it yields a boolean result.
    @@ -2512,30 +2500,15 @@ cookConstraint(ParseState *pstate,
    25122500
    assign_expr_collations(pstate, expr);
    25132501

    25142502
    /*
    2515-
    * Make sure no outside relations are referred to.
    2503+
    * Make sure no outside relations are referred to (this is probably dead
    2504+
    * code now that add_missing_from is history).
    25162505
    */
    25172506
    if (list_length(pstate->p_rtable) != 1)
    25182507
    ereport(ERROR,
    25192508
    (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
    25202509
    errmsg("only table \"%s\" can be referenced in check constraint",
    25212510
    relname)));
    25222511

    2523-
    /*
    2524-
    * No subplans or aggregates, either...
    2525-
    */
    2526-
    if (pstate->p_hasSubLinks)
    2527-
    ereport(ERROR,
    2528-
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    2529-
    errmsg("cannot use subquery in check constraint")));
    2530-
    if (pstate->p_hasAggs)
    2531-
    ereport(ERROR,
    2532-
    (errcode(ERRCODE_GROUPING_ERROR),
    2533-
    errmsg("cannot use aggregate function in check constraint")));
    2534-
    if (pstate->p_hasWindowFuncs)
    2535-
    ereport(ERROR,
    2536-
    (errcode(ERRCODE_WINDOWING_ERROR),
    2537-
    errmsg("cannot use window function in check constraint")));
    2538-
    25392512
    return expr;
    25402513
    }
    25412514

    ‎src/backend/commands/functioncmds.c

    Lines changed: 8 additions & 16 deletions
    Original file line numberDiff line numberDiff line change
    @@ -344,12 +344,14 @@ examine_parameter_list(List *parameters, Oid languageOid,
    344344
    (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
    345345
    errmsg("only input parameters can have default values")));
    346346

    347-
    def = transformExpr(pstate, fp->defexpr);
    347+
    def = transformExpr(pstate, fp->defexpr,
    348+
    EXPR_KIND_FUNCTION_DEFAULT);
    348349
    def = coerce_to_specific_type(pstate, def, toid, "DEFAULT");
    349350
    assign_expr_collations(pstate, def);
    350351

    351352
    /*
    352-
    * Make sure no variables are referred to.
    353+
    * Make sure no variables are referred to (this is probably dead
    354+
    * code now that add_missing_from is history).
    353355
    */
    354356
    if (list_length(pstate->p_rtable) != 0 ||
    355357
    contain_var_clause(def))
    @@ -358,28 +360,18 @@ examine_parameter_list(List *parameters, Oid languageOid,
    358360
    errmsg("cannot use table references in parameter default value")));
    359361

    360362
    /*
    363+
    * transformExpr() should have already rejected subqueries,
    364+
    * aggregates, and window functions, based on the EXPR_KIND_ for a
    365+
    * default expression.
    366+
    *
    361367
    * It can't return a set either --- but coerce_to_specific_type
    362368
    * already checked that for us.
    363369
    *
    364-
    * No subplans or aggregates, either...
    365-
    *
    366370
    * Note: the point of these restrictions is to ensure that an
    367371
    * expression that, on its face, hasn't got subplans, aggregates,
    368372
    * etc cannot suddenly have them after function default arguments
    369373
    * are inserted.
    370374
    */
    371-
    if (pstate->p_hasSubLinks)
    372-
    ereport(ERROR,
    373-
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    374-
    errmsg("cannot use subquery in parameter default value")));
    375-
    if (pstate->p_hasAggs)
    376-
    ereport(ERROR,
    377-
    (errcode(ERRCODE_GROUPING_ERROR),
    378-
    errmsg("cannot use aggregate function in parameter default value")));
    379-
    if (pstate->p_hasWindowFuncs)
    380-
    ereport(ERROR,
    381-
    (errcode(ERRCODE_WINDOWING_ERROR),
    382-
    errmsg("cannot use window function in parameter default value")));
    383375

    384376
    *parameterDefaults = lappend(*parameterDefaults, def);
    385377
    have_defaults = true;

    ‎src/backend/commands/indexcmds.c

    Lines changed: 5 additions & 21 deletions
    Original file line numberDiff line numberDiff line change
    @@ -941,17 +941,9 @@ static void
    941941
    CheckPredicate(Expr *predicate)
    942942
    {
    943943
    /*
    944-
    * We don't currently support generation of an actual query plan for a
    945-
    * predicate, only simple scalar expressions; hence these restrictions.
    944+
    * transformExpr() should have already rejected subqueries, aggregates,
    945+
    * and window functions, based on the EXPR_KIND_ for a predicate.
    946946
    */
    947-
    if (contain_subplans((Node *) predicate))
    948-
    ereport(ERROR,
    949-
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    950-
    errmsg("cannot use subquery in index predicate")));
    951-
    if (contain_agg_clause((Node *) predicate))
    952-
    ereport(ERROR,
    953-
    (errcode(ERRCODE_GROUPING_ERROR),
    954-
    errmsg("cannot use aggregate in index predicate")));
    955947

    956948
    /*
    957949
    * A predicate using mutable functions is probably wrong, for the same
    @@ -1072,18 +1064,10 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
    10721064
    expr);
    10731065

    10741066
    /*
    1075-
    * We don't currently support generation of an actual query
    1076-
    * plan for an index expression, only simple scalar
    1077-
    * expressions; hence these restrictions.
    1067+
    * transformExpr() should have already rejected subqueries,
    1068+
    * aggregates, and window functions, based on the EXPR_KIND_
    1069+
    * for an index expression.
    10781070
    */
    1079-
    if (contain_subplans(expr))
    1080-
    ereport(ERROR,
    1081-
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    1082-
    errmsg("cannot use subquery in index expression")));
    1083-
    if (contain_agg_clause(expr))
    1084-
    ereport(ERROR,
    1085-
    (errcode(ERRCODE_GROUPING_ERROR),
    1086-
    errmsg("cannot use aggregate function in index expression")));
    10871071

    10881072
    /*
    10891073
    * A expression using mutable functions is probably wrong,

    ‎src/backend/commands/prepare.c

    Lines changed: 1 addition & 15 deletions
    Original file line numberDiff line numberDiff line change
    @@ -354,21 +354,7 @@ EvaluateParams(PreparedStatement *pstmt, List *params,
    354354
    Oid expected_type_id = param_types[i];
    355355
    Oid given_type_id;
    356356

    357-
    expr = transformExpr(pstate, expr);
    358-
    359-
    /* Cannot contain subselects or aggregates */
    360-
    if (pstate->p_hasSubLinks)
    361-
    ereport(ERROR,
    362-
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    363-
    errmsg("cannot use subquery in EXECUTE parameter")));
    364-
    if (pstate->p_hasAggs)
    365-
    ereport(ERROR,
    366-
    (errcode(ERRCODE_GROUPING_ERROR),
    367-
    errmsg("cannot use aggregate function in EXECUTE parameter")));
    368-
    if (pstate->p_hasWindowFuncs)
    369-
    ereport(ERROR,
    370-
    (errcode(ERRCODE_WINDOWING_ERROR),
    371-
    errmsg("cannot use window function in EXECUTE parameter")));
    357+
    expr = transformExpr(pstate, expr, EXPR_KIND_EXECUTE_PARAMETER);
    372358

    373359
    given_type_id = exprType(expr);
    374360

    ‎src/backend/commands/tablecmds.c

    Lines changed: 2 additions & 15 deletions
    Original file line numberDiff line numberDiff line change
    @@ -7178,27 +7178,14 @@ ATPrepAlterColumnType(List **wqueue,
    71787178
    true);
    71797179
    addRTEtoQuery(pstate, rte, false, true, true);
    71807180

    7181-
    transform = transformExpr(pstate, transform);
    7181+
    transform = transformExpr(pstate, transform,
    7182+
    EXPR_KIND_ALTER_COL_TRANSFORM);
    71827183

    71837184
    /* It can't return a set */
    71847185
    if (expression_returns_set(transform))
    71857186
    ereport(ERROR,
    71867187
    (errcode(ERRCODE_DATATYPE_MISMATCH),
    71877188
    errmsg("transform expression must not return a set")));
    7188-
    7189-
    /* No subplans or aggregates, either... */
    7190-
    if (pstate->p_hasSubLinks)
    7191-
    ereport(ERROR,
    7192-
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    7193-
    errmsg("cannot use subquery in transform expression")));
    7194-
    if (pstate->p_hasAggs)
    7195-
    ereport(ERROR,
    7196-
    (errcode(ERRCODE_GROUPING_ERROR),
    7197-
    errmsg("cannot use aggregate function in transform expression")));
    7198-
    if (pstate->p_hasWindowFuncs)
    7199-
    ereport(ERROR,
    7200-
    (errcode(ERRCODE_WINDOWING_ERROR),
    7201-
    errmsg("cannot use window function in transform expression")));
    72027189
    }
    72037190
    else
    72047191
    {

    ‎src/backend/commands/trigger.c

    Lines changed: 1 addition & 16 deletions
    Original file line numberDiff line numberDiff line change
    @@ -286,26 +286,11 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
    286286
    /* Transform expression. Copy to be sure we don't modify original */
    287287
    whenClause = transformWhereClause(pstate,
    288288
    copyObject(stmt->whenClause),
    289+
    EXPR_KIND_TRIGGER_WHEN,
    289290
    "WHEN");
    290291
    /* we have to fix its collations too */
    291292
    assign_expr_collations(pstate, whenClause);
    292293

    293-
    /*
    294-
    * No subplans or aggregates, please
    295-
    */
    296-
    if (pstate->p_hasSubLinks)
    297-
    ereport(ERROR,
    298-
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    299-
    errmsg("cannot use subquery in trigger WHEN condition")));
    300-
    if (pstate->p_hasAggs)
    301-
    ereport(ERROR,
    302-
    (errcode(ERRCODE_GROUPING_ERROR),
    303-
    errmsg("cannot use aggregate function in trigger WHEN condition")));
    304-
    if (pstate->p_hasWindowFuncs)
    305-
    ereport(ERROR,
    306-
    (errcode(ERRCODE_WINDOWING_ERROR),
    307-
    errmsg("cannot use window function in trigger WHEN condition")));
    308-
    309294
    /*
    310295
    * Check for disallowed references to OLD/NEW.
    311296
    *

    ‎src/backend/commands/typecmds.c

    Lines changed: 5 additions & 28 deletions
    Original file line numberDiff line numberDiff line change
    @@ -2871,7 +2871,7 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
    28712871

    28722872
    pstate->p_value_substitute = (Node *) domVal;
    28732873

    2874-
    expr = transformExpr(pstate, constr->raw_expr);
    2874+
    expr = transformExpr(pstate, constr->raw_expr, EXPR_KIND_DOMAIN_CHECK);
    28752875

    28762876
    /*
    28772877
    * Make sure it yields a boolean result.
    @@ -2884,38 +2884,15 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
    28842884
    assign_expr_collations(pstate, expr);
    28852885

    28862886
    /*
    2887-
    * Make sure no outside relations are referred to.
    2887+
    * Domains don't allow variables (this is probably dead code now that
    2888+
    * add_missing_from is history, but let's be sure).
    28882889
    */
    2889-
    if (list_length(pstate->p_rtable) != 0)
    2890+
    if (list_length(pstate->p_rtable) != 0 ||
    2891+
    contain_var_clause(expr))
    28902892
    ereport(ERROR,
    28912893
    (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
    28922894
    errmsg("cannot use table references in domain check constraint")));
    28932895

    2894-
    /*
    2895-
    * Domains don't allow var clauses (this should be redundant with the
    2896-
    * above check, but make it anyway)
    2897-
    */
    2898-
    if (contain_var_clause(expr))
    2899-
    ereport(ERROR,
    2900-
    (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
    2901-
    errmsg("cannot use table references in domain check constraint")));
    2902-
    2903-
    /*
    2904-
    * No subplans or aggregates, either...
    2905-
    */
    2906-
    if (pstate->p_hasSubLinks)
    2907-
    ereport(ERROR,
    2908-
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    2909-
    errmsg("cannot use subquery in check constraint")));
    2910-
    if (pstate->p_hasAggs)
    2911-
    ereport(ERROR,
    2912-
    (errcode(ERRCODE_GROUPING_ERROR),
    2913-
    errmsg("cannot use aggregate function in check constraint")));
    2914-
    if (pstate->p_hasWindowFuncs)
    2915-
    ereport(ERROR,
    2916-
    (errcode(ERRCODE_WINDOWING_ERROR),
    2917-
    errmsg("cannot use window function in check constraint")));
    2918-
    29192896
    /*
    29202897
    * Convert to string form for storage.
    29212898
    */

    ‎src/backend/optimizer/util/clauses.c

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -596,7 +596,7 @@ count_agg_clauses_walker(Node *node, count_agg_clauses_context *context)
    596596
    bool
    597597
    contain_window_function(Node *clause)
    598598
    {
    599-
    return checkExprHasWindowFuncs(clause);
    599+
    return contain_windowfuncs(clause);
    600600
    }
    601601

    602602
    /*

    0 commit comments

    Comments
     (0)
    0