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

From Tom Lane
Subject Re: Window functions can be created with defaults, but they don't work
Date
Msg-id 1083.1383690229@sss.pgh.pa.us
Whole thread Raw
In response to Re: Window functions can be created with defaults, but they don't work  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
I wrote:
> 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).

Hm, well, almost free --- turns out ruleutils.c was assuming WindowFuncs
couldn't contain named arguments.  Updated patch attached.

            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/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 04b1c4f..915fb7a 100644
*** a/src/backend/utils/adt/ruleutils.c
--- b/src/backend/utils/adt/ruleutils.c
*************** get_windowfunc_expr(WindowFunc *wfunc, d
*** 7461,7466 ****
--- 7461,7467 ----
      StringInfo    buf = context->buf;
      Oid            argtypes[FUNC_MAX_ARGS];
      int            nargs;
+     List       *argnames;
      ListCell   *l;

      if (list_length(wfunc->args) > FUNC_MAX_ARGS)
*************** get_windowfunc_expr(WindowFunc *wfunc, d
*** 7468,7485 ****
                  (errcode(ERRCODE_TOO_MANY_ARGUMENTS),
                   errmsg("too many arguments")));
      nargs = 0;
      foreach(l, wfunc->args)
      {
          Node       *arg = (Node *) lfirst(l);

!         Assert(!IsA(arg, NamedArgExpr));
          argtypes[nargs] = exprType(arg);
          nargs++;
      }

      appendStringInfo(buf, "%s(",
                       generate_function_name(wfunc->winfnoid, nargs,
!                                             NIL, argtypes,
                                              false, NULL));
      /* winstar can be set only in zero-argument aggregates */
      if (wfunc->winstar)
--- 7469,7488 ----
                  (errcode(ERRCODE_TOO_MANY_ARGUMENTS),
                   errmsg("too many arguments")));
      nargs = 0;
+     argnames = NIL;
      foreach(l, wfunc->args)
      {
          Node       *arg = (Node *) lfirst(l);

!         if (IsA(arg, NamedArgExpr))
!             argnames = lappend(argnames, ((NamedArgExpr *) arg)->name);
          argtypes[nargs] = exprType(arg);
          nargs++;
      }

      appendStringInfo(buf, "%s(",
                       generate_function_name(wfunc->winfnoid, nargs,
!                                             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 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;

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Add cassert-only checks against unlocked use of relations
Next
From: Andres Freund
Date:
Subject: Re: Add cassert-only checks against unlocked use of relations