Re: Window functions can be created with defaults, but they don't work - Mailing list pgsql-hackers

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
immutablestrict 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;