I wrote:
> I noticed this while poking at the variadic-aggregates issue:
> regression=# create function nth_value_def(anyelement, integer = 1) returns
> anyelement language internal window immutable strict as 'window_nth_value';
> CREATE FUNCTION
> regression=# SELECT nth_value_def(ten) OVER (PARTITION BY four), ten, four
> FROM (SELECT * FROM tenk1 WHERE unique2 < 10 ORDER BY four, ten)s;
> The connection to the server was lost. Attempting reset: Failed.
Attached is a proposed patch against HEAD that fixes this by supporting
default arguments properly for window functions. In passing, it also
allows named-argument notation in window function calls, since that's
free once the other thing works (because the same subroutine fixes up
both things).
> The reason this crashes is that the planner doesn't apply
> default-insertion to WindowFunc nodes, only to FuncExprs. We could make
> it do that, probably, but that seems to me like a feature addition.
> I think a more reasonable approach for back-patching is to fix function
> creation to disallow attaching defaults to window functions (or
> aggregates, for that matter, which would have the same issue if CREATE
> AGGREGATE had the syntax option to specify defaults). ProcedureCreate
> seems like an appropriate place, since it already contains a lot of sanity
> checks of this sort.
Having now done the patch to fix it properly, I'm more inclined to think
that maybe we should just back-patch this rather than inserting an error
check. It seems pretty low-risk.
Comments?
regards, tom lane
diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml
index e3dbc4b..16aade4 100644
*** a/doc/src/sgml/syntax.sgml
--- b/doc/src/sgml/syntax.sgml
*************** SELECT concat_lower_or_upper('Hello', 'W
*** 2527,2534 ****
<note>
<para>
! Named and mixed call notations can currently be used only with regular
! functions, not with aggregate functions or window functions.
</para>
</note>
</sect2>
--- 2527,2535 ----
<note>
<para>
! 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).
</para>
</note>
</sect2>
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 76c032c..4fa73a9 100644
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
*************** eval_const_expressions_mutator(Node *nod
*** 2251,2256 ****
--- 2251,2306 ----
*/
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 2bd24c8..ede36d1 100644
*** a/src/backend/parser/parse_func.c
--- b/src/backend/parser/parse_func.c
*************** ParseFuncOrColumn(ParseState *pstate, Li
*** 537,553 ****
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);
--- 537,542 ----
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index 7b31d13..1e6365b 100644
*** a/src/test/regress/expected/window.out
--- b/src/test/regress/expected/window.out
*************** FROM empsalary GROUP BY depname;
*** 1035,1037 ****
--- 1035,1072 ----
-- 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 6ee3696..7297e62 100644
*** a/src/test/regress/sql/window.sql
--- b/src/test/regress/sql/window.sql
*************** FROM empsalary GROUP BY depname;
*** 274,276 ****
--- 274,286 ----
-- 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;
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
