From bb45c640411af61279bea044f8d108f9da96b735 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 6 Nov 2013 13:26:30 -0500 Subject: [PATCH] Support default arguments and named-argument notation for window functions. These things didn't work because the planner omitted to do the necessary preprocessing of a WindowFunc's argument list. Add the few dozen lines of code needed to handle that. Although this sounds like a feature addition, it's really a bug fix because the default-argument case was likely to crash previously, due to lack of checking of the number of supplied arguments in the built-in window functions. It's not a security issue because there's no way for a non-superuser to create a window function definition with defaults that refers to a built-in C function, but nonetheless people might be annoyed that it crashes rather than producing a useful error message. So back-patch as far as the patch applies easily, which turns out to be 9.2. I'll put a band-aid in earlier versions as a separate patch. (Note that these features still don't work for aggregates, and fixing that case will be harder since we represent aggregate arg lists as target lists not bare expression lists. There's no crash risk though because CREATE AGGREGATE doesn't accept defaults, and we reject named-argument notation when parsing an aggregate call.) --- doc/src/sgml/syntax.sgml | 5 +-- src/backend/optimizer/util/clauses.c | 50 ++++++++++++++++++++++++++++ src/backend/parser/parse_func.c | 11 ------ src/backend/utils/adt/ruleutils.c | 7 ++-- src/test/regress/expected/window.out | 35 +++++++++++++++++++ src/test/regress/sql/window.sql | 10 ++++++ 6 files changed, 103 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml index 4fe872290a..435709a8d4 100644 --- a/doc/src/sgml/syntax.sgml +++ b/doc/src/sgml/syntax.sgml @@ -2533,8 +2533,9 @@ SELECT concat_lower_or_upper('Hello', 'World', uppercase := true); - Named and mixed call notations can currently be used only with regular - functions, not with aggregate functions or window functions. + Named and mixed call notations currently cannot be used when calling an + aggregate function (but they do work when an aggregate function is used + as a window function). diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 76c032c569..4fa73a9d22 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -2251,6 +2251,56 @@ eval_const_expressions_mutator(Node *node, */ return (Node *) copyObject(param); } + case T_WindowFunc: + { + WindowFunc *expr = (WindowFunc *) node; + Oid funcid = expr->winfnoid; + List *args; + Expr *aggfilter; + HeapTuple func_tuple; + WindowFunc *newexpr; + + /* + * We can't really simplify a WindowFunc node, but we mustn't + * just fall through to the default processing, because we + * have to apply expand_function_arguments to its argument + * list. That takes care of inserting default arguments and + * expanding named-argument notation. + */ + func_tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid)); + if (!HeapTupleIsValid(func_tuple)) + elog(ERROR, "cache lookup failed for function %u", funcid); + + args = expand_function_arguments(expr->args, expr->wintype, + func_tuple); + + ReleaseSysCache(func_tuple); + + /* Now, recursively simplify the args (which are a List) */ + args = (List *) + expression_tree_mutator((Node *) args, + eval_const_expressions_mutator, + (void *) context); + /* ... and the filter expression, which isn't */ + aggfilter = (Expr *) + eval_const_expressions_mutator((Node *) expr->aggfilter, + context); + + /* And build the replacement WindowFunc node */ + newexpr = makeNode(WindowFunc); + newexpr->winfnoid = expr->winfnoid; + newexpr->wintype = expr->wintype; + newexpr->wincollid = expr->wincollid; + newexpr->inputcollid = expr->inputcollid; + newexpr->args = args; + newexpr->aggfilter = aggfilter; + newexpr->winref = expr->winref; + newexpr->winstar = expr->winstar; + newexpr->winagg = expr->winagg; + newexpr->location = expr->location; + + return (Node *) newexpr; + } case T_FuncExpr: { FuncExpr *expr = (FuncExpr *) node; diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index 2bd24c89c8..ede36d159a 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -537,17 +537,6 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, errmsg("window functions cannot return sets"), parser_errposition(pstate, location))); - /* - * We might want to support this later, but for now reject it because - * the planner and executor wouldn't cope with NamedArgExprs in a - * WindowFunc node. - */ - if (argnames != NIL) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("window functions cannot use named arguments"), - parser_errposition(pstate, location))); - /* parse_agg.c does additional window-func-specific processing */ transformWindowFuncCall(pstate, wfunc, over); diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 04b1c4f895..915fb7a668 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -7461,6 +7461,7 @@ get_windowfunc_expr(WindowFunc *wfunc, deparse_context *context) StringInfo buf = context->buf; Oid argtypes[FUNC_MAX_ARGS]; int nargs; + List *argnames; ListCell *l; if (list_length(wfunc->args) > FUNC_MAX_ARGS) @@ -7468,18 +7469,20 @@ get_windowfunc_expr(WindowFunc *wfunc, deparse_context *context) (errcode(ERRCODE_TOO_MANY_ARGUMENTS), errmsg("too many arguments"))); nargs = 0; + argnames = NIL; foreach(l, wfunc->args) { Node *arg = (Node *) lfirst(l); - Assert(!IsA(arg, NamedArgExpr)); + if (IsA(arg, NamedArgExpr)) + argnames = lappend(argnames, ((NamedArgExpr *) arg)->name); argtypes[nargs] = exprType(arg); nargs++; } appendStringInfo(buf, "%s(", generate_function_name(wfunc->winfnoid, nargs, - NIL, argtypes, + argnames, argtypes, false, NULL)); /* winstar can be set only in zero-argument aggregates */ if (wfunc->winstar) diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out index 7b31d131b1..1e6365b4f9 100644 --- a/src/test/regress/expected/window.out +++ b/src/test/regress/expected/window.out @@ -1035,3 +1035,38 @@ FROM empsalary GROUP BY depname; -- cleanup DROP TABLE empsalary; +-- test user-defined window function with named args and default args +CREATE FUNCTION nth_value_def(val anyelement, n integer = 1) RETURNS anyelement + LANGUAGE internal WINDOW IMMUTABLE STRICT AS 'window_nth_value'; +SELECT nth_value_def(n := 2, val := ten) OVER (PARTITION BY four), ten, four + FROM (SELECT * FROM tenk1 WHERE unique2 < 10 ORDER BY four, ten) s; + nth_value_def | ten | four +---------------+-----+------ + 0 | 0 | 0 + 0 | 0 | 0 + 0 | 4 | 0 + 1 | 1 | 1 + 1 | 1 | 1 + 1 | 7 | 1 + 1 | 9 | 1 + | 0 | 2 + 3 | 1 | 3 + 3 | 3 | 3 +(10 rows) + +SELECT nth_value_def(ten) OVER (PARTITION BY four), ten, four + FROM (SELECT * FROM tenk1 WHERE unique2 < 10 ORDER BY four, ten) s; + nth_value_def | ten | four +---------------+-----+------ + 0 | 0 | 0 + 0 | 0 | 0 + 0 | 4 | 0 + 1 | 1 | 1 + 1 | 1 | 1 + 1 | 7 | 1 + 1 | 9 | 1 + 0 | 0 | 2 + 1 | 1 | 3 + 1 | 3 | 3 +(10 rows) + diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql index 6ee3696da1..7297e62618 100644 --- a/src/test/regress/sql/window.sql +++ b/src/test/regress/sql/window.sql @@ -274,3 +274,13 @@ FROM empsalary GROUP BY depname; -- cleanup DROP TABLE empsalary; + +-- test user-defined window function with named args and default args +CREATE FUNCTION nth_value_def(val anyelement, n integer = 1) RETURNS anyelement + LANGUAGE internal WINDOW IMMUTABLE STRICT AS 'window_nth_value'; + +SELECT nth_value_def(n := 2, val := ten) OVER (PARTITION BY four), ten, four + FROM (SELECT * FROM tenk1 WHERE unique2 < 10 ORDER BY four, ten) s; + +SELECT nth_value_def(ten) OVER (PARTITION BY four), ten, four + FROM (SELECT * FROM tenk1 WHERE unique2 < 10 ORDER BY four, ten) s;