Refine error reporting for null treatment on non-window functions

Commit 4e5920e6de8 disallowed RESPECT NULLS/IGNORE NULLS on
non-window functions, but it also caused the parser to check for
that clause too early in some cases. As a result, calls such as a
nonexistent function with IGNORE NULLS no longer reported the more
helpful "function ... does not exist" error, and aggregate functions
used as window functions reported "only window functions accept ..."
instead of the more accurate aggregate-specific error.

This commit moves the RESPECT NULLS/IGNORE NULLS checks so that
helpful existing errors are preserved where appropriate. This restores
"function ... does not exist" for nonexistent functions, while still
reporting that plain functions are not window functions and that
aggregates do not accept null treatment.

Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Tatsuo Ishii <ishii@postgresql.org>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAHGQGwH7VY_0GkhycyYZ4czkPGL0uGzDyOxk3uuFOSRR7wFY3g@mail.gmail.com

diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 860767a52ee838155d074ea9ade5ca2ec879b6d2..fb306c05112e712ae002cdecdadbef9fcf2eeeab 100644 (file)
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -351,17 +351,15 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
                                         errmsg("OVER specified, but %s is not a window function nor an aggregate function",
                                                        NameListToString(funcname)),
                                         parser_errposition(pstate, location)));
+               if (ignore_nulls != NO_NULLTREATMENT)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                       /*- translator: first %s is a null treatment option, eg IGNORE NULLS */
+                                        errmsg("%s specified, but %s is not a window function",
+                                                       "RESPECT/IGNORE NULLS", NameListToString(funcname)),
+                                        parser_errposition(pstate, location)));
        }
 
-       /*
-        * NULL TREATEMENT is only allowed for window functions per spec.
-        */
-       if (fdresult != FUNCDETAIL_WINDOWFUNC && ignore_nulls != NO_NULLTREATMENT)
-               ereport(ERROR,
-                               errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                               errmsg("only window functions accept RESPECT/IGNORE NULLS"),
-                               parser_errposition(pstate, location));
-
        /*
         * So far so good, so do some fdresult-type-specific processing.
         */
@@ -528,7 +526,14 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
                                                 errmsg("%s is not an ordered-set aggregate, so it cannot have WITHIN GROUP",
                                                                NameListToString(funcname)),
                                                 parser_errposition(pstate, location)));
+
                }
+
+               if (ignore_nulls != NO_NULLTREATMENT)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                        errmsg("aggregate functions do not accept RESPECT/IGNORE NULLS"),
+                                        parser_errposition(pstate, location)));
        }
        else if (fdresult == FUNCDETAIL_WINDOWFUNC)
        {

diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index 59c3df8cf0a68ddf416f42e4c968c346d7f51502..90d9f953b81284b17736c3aae91179e31cfe5e1a 100644 (file)
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -5748,6 +5748,19 @@ WINDOW w AS (ORDER BY name ROWS BETWEEN 2 PRECEDING AND 2 FOLLOWING EXCLUDE CURR
 (10 rows)
 
 -- valid and invalid functions
+SELECT abs(1) IGNORE NULLS; -- fails
+ERROR:  RESPECT/IGNORE NULLS specified, but abs is not a window function
+LINE 1: SELECT abs(1) IGNORE NULLS;
+               ^
+SELECT no_such_window_func() IGNORE NULLS; -- fails, but not because of null treatment
+ERROR:  function no_such_window_func() does not exist
+LINE 1: SELECT no_such_window_func() IGNORE NULLS;
+               ^
+DETAIL:  There is no function of that name.
+SELECT sum(orbit) IGNORE NULLS FROM planets; -- fails
+ERROR:  aggregate functions do not accept RESPECT/IGNORE NULLS
+LINE 1: SELECT sum(orbit) IGNORE NULLS FROM planets;
+               ^
 SELECT sum(orbit) OVER () FROM planets; -- succeeds
   sum   
 --------
@@ -5764,11 +5777,11 @@ SELECT sum(orbit) OVER () FROM planets; -- succeeds
 (10 rows)
 
 SELECT sum(orbit) RESPECT NULLS OVER () FROM planets; -- fails
-ERROR:  only window functions accept RESPECT/IGNORE NULLS
+ERROR:  aggregate functions do not accept RESPECT/IGNORE NULLS
 LINE 1: SELECT sum(orbit) RESPECT NULLS OVER () FROM planets;
                ^
 SELECT sum(orbit) IGNORE NULLS OVER () FROM planets; -- fails
-ERROR:  only window functions accept RESPECT/IGNORE NULLS
+ERROR:  aggregate functions do not accept RESPECT/IGNORE NULLS
 LINE 1: SELECT sum(orbit) IGNORE NULLS OVER () FROM planets;
                ^
 SELECT row_number() OVER () FROM planets; -- succeeds

diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index 17261135dc379352670ee7722a68f6994ed95894..5ac3a486e1693c79f695c2c9dffb704782790ac3 100644 (file)
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -2099,6 +2099,9 @@ WINDOW w AS (ORDER BY name ROWS BETWEEN 2 PRECEDING AND 2 FOLLOWING EXCLUDE CURR
 ;
 
 -- valid and invalid functions
+SELECT abs(1) IGNORE NULLS; -- fails
+SELECT no_such_window_func() IGNORE NULLS; -- fails, but not because of null treatment
+SELECT sum(orbit) IGNORE NULLS FROM planets; -- fails
 SELECT sum(orbit) OVER () FROM planets; -- succeeds
 SELECT sum(orbit) RESPECT NULLS OVER () FROM planets; -- fails
 SELECT sum(orbit) IGNORE NULLS OVER () FROM planets; -- fails